This commit changes the TypeScript reflection host to:
* inspect / process ES private fields. e.g. `#someField` — those are
ignored right now and we would want to check them to issue
diagnostics.
* determine an access level of a class member. E.g. a member may be
public, may be private, may be ES private, or public readonly. This
can then be used in various checks later.
PR Close#55070
Updates the function that parses initializer APIs to check any `Expression`, instead of expecting a class member. This will be useful for the upcoming changes.
PR Close#55070
This commit updates the logic for preserving file overview comments
to be more reliable and less dependent on previous transforms.
Previously, with the old import manager, we had a utility called
`addImport` that always separated import statements and non-import
statements. This meant that the non-emitted statement from Tsickle
for the synthetic file-overview comments no longer lived at the
beginning of the file.
`addImports` tried to overcome this by adding another new non-emitted
statement *before* all imports. This then was later used by the
transform (or was assumed!) to attach the synthetic file overview
comments if the original tsickle AST Node is no longer at the top.
This logic can be improved, because the import manager shouldn't need to
bother about this fileoverview non-emitted statement, and the logic for
re-attaching the fileoverview comment should be local. This commit fixes
this and makes it a local transform.
PR Close#54983
This commit adds some unit tests verifying the import generation in TCB
files and inline blocks. We don't seem to have any unit tests for these
in general. This commit adds some, verifying some characteristics we
would like to guarantee.
PR Close#54983
To ease review and to allow for both instances to co-exist, `ImportManagerV2`
was introduced. This commit renames it to `ImportManager` now that we
deleted the older one.
PR Close#54983
Enables the incremental type-checking test that we never enabled when we
landed signal inputs. Now that we fixed incremental re-use by re-using
the existing user imports for inline type check blocks, the test is
passing and can be enabled.
PR Close#54983
Updates the type-check block generation code (also for inline type check
blocks) to use the new import manager.
This is now a requirement because the translator utilities from the
reference emit environment expect an import manager that follows the
new contract established via `ImportGenerator<TFile, TExpression>`.
For type check files, we can simply print new imports as we don't expect
existing imports to be updated. That is because type check files do not
have any _original_ source files (or in practice— those are empty).
For type check blocks inline, or constructors, imports _may_ be re-used.
This is great as it helps fixing some incrementality bugs that we were
seeing in the type check code. That is, sometimes the type check block
code may generate imports conditionally for e.g. `TemplateRef`, or
animations. Those then **prevent** incremental re-use if TCB code
switches between those continously. We tried to account for that with
signal inputs by always pre-generating such imports. This fixed the
issue for type-check files, but for inline type check blocks this is
different as we would introduce new imports in user code that would then
be changed back in subsequential edit iterations. See:
https://github.com/angular/angular/pull/53521#pullrequestreview-1778130879.
In practice, the assumption was that we would be fine since user code is
most likely containing imports to `@angular/core` already. That is a
true assumption, but unfortunately it doesn't help with incremental
re-use because TypeScript's structural change detection does not dedupe
and expects 1:1 exact imports from their old source files.
https://github.com/microsoft/TypeScript/pull/56845
To improve incremental re-use for the type check integration, we should
re-use original source file imports when possible. This commit enables
this.
To update imports and execute inline operations, we are now uisng
`magic-string` (which is then bundled) as it simplifies the string
manipulatuons.
PR Close#54983
This commit switches ngtsc's JS and DTS transform to use the new import
manager. This is a drop-in replacement as we've updated the translator
helpers in the previous commit to align with the new API suggested by
the `ImportManagerV2` (to be renamed then).
PR Close#54983
`ImportGenerator` is the abstraction used by the translator functions to
insert imports for `ExternalExpr` in an AST-agnostic way.
This was built specifically for the linker which does not use any of the
complex import managers- but rather re-uses `ngImport` or uses
`ngImport.Bla`.
This commit also switches the linker AST-agnostic generator to follow
the new signatures. This was rather trivial.
PR Close#54983
This commit introduces a new implementation of `ImportManager` that has
numerous benefits:
- It allows efficient re-use of original source file imports.
* either fully re-using original imports if matching
* updating existing import declarations to include new symbols.
- It allows efficient re-use of previous generated imports.
- The manager can be used for schematics and migrations.
The implementation is a rework of the import manager that we originally
built for schematics in Angular Material, but this commit improved it
to be more flexible, more readable, and "correct".
In follow-ups we can use this for schematics/migrations.
PR Close#54983
Previously, `RouterLinkActive` would only add or remove the active classes when
its active state changed. This means that if you accidentally add one of
the active classes to the static class attribute, it won't get removed
until the link becomes active and then deactives (because the class is
added at creation time and never removed until the `RouterLinkActive`
state changes from active to inactive).
fixes#54978
PR Close#54982
This update enhances the encoding handling of request bodies to generate the necessary cache key for transfer cache functionality.
Closes#54956
PR Close#54980
Previously we assumed that if a `for` loop tracking function is in the form of `someMethod($index, $item)`, it will be pure so we didn't pass the parameter to bind the context to it. This appears to be risky, because we don't know if the method is trying to access `this`.
These changes play it safe by always binding method-based tracking functions.
Fixes#53628.
PR Close#54960
In some cases the hydration mismatch is nested within a component.
As the devTool only reports issues on the component level, we need to mark the component node rather than the actual mismatched node.
PR Close#54671
This commit updates the name of an internal function argument that represents an embedded view injector. Also it introduces a new config option that allows passing an environment injector to be used in the underlying LView. There are no changes to the behavior, just some initial cleanup for upcoming changes.
PR Close#54903
We are already testing the JIT transforms via integration tests, but
this commit adds dedicated unit tests for the transform behavior for
proper test coverage (planned follow-up).
PR Close#54841
Move the initialization of class field `DelegatingPerfRecorder` into the constructor.
This fixes the error : `TypeError: Cannot read properties of undefined (reading 'eventCount')`
This is blocking the roll-out of public class.
PR Close#54834
The logic in `isCssClassMatching` is only interested in two areas in the attributes:
implicit attributes and the `AttributeMarker.Classes` area, with the first area only
of interest for projection matching, not directive matching. This commit splits these
two searches to make this more apparent.
PR Close#54800
This commit resolves a regression that was introduced when the compiler switched from
`TemplateDefinitionBuilder` (TDB) to the template pipeline (TP) compiler. The TP compiler
has changed the output of
```html
if (false) { <div class="test"></div> }
```
from
```ts
defineComponent({
consts: [['class', 'test'], [AttributeMarker.Classes, 'test']],
template: function(rf) {
if (rf & 1) {
ɵɵtemplate(0, App_Conditional_0_Template, 2, 0, "div", 0)
}
}
});
```
to
```ts
defineComponent({
consts: [[AttributeMarker.Classes, 'test']],
template: function(rf) {
if (rf & 1) {
ɵɵtemplate(0, App_Conditional_0_Template, 2, 0, "div", 0)
}
}
});
```
The last argument to the `ɵɵtemplate` instruction (0 in both compilation outputs) corresponds with
the index in `consts` of the element's attribute's, and we observe how TP has allocated only a single
attribute array for the `div`, where there used to be two `consts` entries with TDB. Consequently,
the `ɵɵtemplate` instruction is now effectively referencing a different attributes array, where the
distinction between the `"class"` attribute vs. the `AttributeMarker.Classes` distinction affects
the behavior: TP's emit causes the runtime to incorrectly match a directive with `selector: '.foo'` to
be instantiated on the `ɵɵtemplate` instruction as if it corresponds with a structural directive!
Instead of changing TP to align with TDB's emit, this commit updates the runtime instead. This uncovered
an inconsistency in selector matching for class names, where there used to be two paths dealing with
class matching:
1. The first check was commented to be a special-case for class matching, implemented in `isCssClassMatching`.
2. The second path was part of the main selector matching algorithm, where `findAttrIndexInNode` was being used
to find the start position in `tNode.attrs` to match the selector's value against.
The second path only considers `AttributeMarker.Classes` values if matching for content projection, OR of the
`TNode` is not an inline template. The special-case in path 1 however does not make that distinction, so it
would consider the `AttributeMarker.Classes` binding as a selector match, incorrectly causing a directive to
match on the `ɵɵtemplate` itself.
The second path was also buggy for class bindings, as the return value of `classIndexOf` was incorrectly
negated: it considered a matching class attribute as non-matching and vice-versa. This bug was not observable
because of another issue, where the class-handling in part 2 was never relevant because of the special-case
in part 1.
This commit separates path 1 entirely from path 2 and removes the buggy class-matching logic in part 2, as
that is entirely handled by path 1 anyway. `isCssClassMatching` is updated to exclude class bindings from
being matched for inline templates.
Fixes#54798
PR Close#54800
Add an internal API to enable and use i18n hydration for testing and development. This helps ensure that we don't accidentally break the current behavior until we are completely ready to roll out i18n support.
PR Close#54784
Currently if an `(output)` listener fails, it will be handled gracefully
by Angular and reported to the `ErrorHandler`.
For programmatic subscriptions with `OutputEmitterRef`, this is not the case.
Instead, as soon as any subscription is failing, all other subsequent
subscription callbacks are not firing anymore.
This commit intends to make this more consistent by gracefully
reporting errors from `OutputEmitterRef#emit` to `ErrorHandler`,
allowing for listener execution to continue.
PR Close#54821
Ensures that all of the functions intended to be run in initializers are in an injection context. This is a stop-gap until we have a compiler diagnostic for it.
PR Close#54761
This commits assert that the repeater instruction gets a reference
to a tracking function. This change will allow us to better track
occurences of https://github.com/angular/angular/issues/53628 -
in certain situations a reference to a tracking function might be
undefiened.
We are not fixing the underlying issue here, just getting better
visibility.
PR Close#54814
Speeds up the retrieval of `DestroyRef` in `EventEmitter` because
`try/catch` is expensive if there is no injection context.
We saw a script time regression in Cloud.
The goldens had to be updated because `getInjectImplementation` is now
referenced. `inject` also references the underlying field, but directly.
This is super minimal overhead of a function exposing the internal
field.
PR Close#54748
This commit updates HTML sanitization logic to avoid infinite loops in case clobbered elements contain fields like `nextSibling` or `parentNode`. Those fields are used for DOM traversal and this update makes sure that those calls return valid results.
Also this commit fixes an issue when clobbering `nodeName` causes JS exceptions.
PR Close#54425
An i18n message effectively acts as a dynamic template: two elements with contiguous instruction indices won't necessarily be contiguous in the DOM.
For that reason, we need to maintain a mapping from instruction index to a physical DOM node in order to hydrate views with i18n, pointing to where hydration for that view should begin.
PR Close#54750
Updates the instruction generation for two-way bindings to only emit the `twoWayBindingSet` call when writing to template variables. Since template variables are constants, it's only allowed to write to them when they're signals. Non-signal values are flagged during template type checking.
Fixes#54670.
PR Close#54714
We have a diagnostic that reports writes to template variables which worked both for regular event bindings and two-way bindings, however the latter was broken by #54154 because two-way bindings no longer had a `PropertyWrite` AST.
These changes fix the diagnostic and expand it to allow two-way bindings to template variables that are signals.
PR Close#54714
Moves the check which ensures that there are no writes to template variables into the `TemplateSemanticsChecker` to prepare for the upcoming changes.
PR Close#54714
Introduces a new `TemplateSemanticsChecker` that will be used to flag semantic errors in the user's template. Currently we do some of this in the type check block, but the problem is that it doesn't have access to the template type checker which prevents us from properly checking cases like #54670. This pass is also distinct from the extended template checks, because we don't want users to be able to turn the checks off and we want them to run even if `strictTemplates` are disabled.
PR Close#54714
The `runCallbackOnce` closure is declared not to have any parameters itself, so it is
compatible as `queueMicrotask` callback without the extra closure. This reduces the call
stack by a frame and avoids the extra closure allocation.
PR Close#54801
Paul Gschwendtner