It's possible for the user to create a host attrbiute binding with a
name that makes it _look_ like a class binding `{['class.foo']: ''}`, we
were previously treating these as actual class property bindings. This
change fixes the logic so that only true property bindings cam be
converted to class property bindings.
Note: A user who added an attribute like the above almost certainly
intended to create an actual class property binding. It would be nice if
we could add a diagnostic to warn them about this.
PR Close#53626
Further refine the template pipeline's behavior w.r.t. duplicate values
in the consts array to better align its behavior with TDB. In particular
this means allowing duplicate values for classes and styles.
PR Close#53596
Adds a test for handling of duplicate bindings. Fow now we replicate the
TDB behavior in template pipeline, which is: For style and class text
attributes, only keep the last one. For all other text attributes, add
all of the values to the consts array.
PR Close#53596
The for loop tracking function doesn't allow references to local template variables, aside from `$index` and the item which are passed in as parameters. We enforce this by rewriting all variable references to the components scope.
The problem is that the logic that rewrites the references first walks the view tree and then checks if the variable is `$index` or the item. This is problematic in nested for loops, because it'll find the `$index` of the parent.
These changes resolve the issue by checking for `$index` and the item first.
Fixes#53600.
PR Close#53604
Changes template pipeline to be less aggressive in const collecting
attrs, to match the behavior of template definition builder. There is
nothing wrong with the more aggressive const collection, and in fact it
would be good to re-enable it later, but for now this makes it easier to
transition from TDB to template pipeline.
Also adds a test to verify that sensitive iframe attributes are properly
validated.
PR Close#53580
TemplateDefinitionBuilder is apparently more careful about when it attempts to split namespaces in attribute values. However, we are doing this on style attributes, which might start with a single `:`. Rather than refactor our logic to only try to split namespaces in some cases, we can just add an option to make namespace splitting fail gracefully. We only use this option for attributes, not elements.
Note also: the compiled code for this, while "correct" is absolutely insane. Maybe we should consider fixing this, as a matter of principle.
PR Close#53574
Some elements may have multiple bindings with the same name. We should accept and emit them all, as long as they have different kinds.
Co-authored-by: Miles Malerba <mmalerba@users.noreply.github.com>
PR Close#53574
The template pipeline was previously not reserving a variable slot for the result of the `deferWhen` instruction, which caused the `defer when` feature to crash at runtime.
PR Close#53574
When an element is self-closing, it will cause an `element` instruction to be emitted (instead of `elementStart`/`elementEnd`). In that case, we should use map whole source span for the instruction, not just the starting span.
PR Close#53574
The template pipeline was producing slightly different names than TemplateDefinitionBuilder for defer deps functions. I have added a workaround in the name of backwards compatibility, to avoid suffixing the const pool function names.
PR Close#53574
Previously when we found an ICU that was the only translatable content
in its i18n block, we assigned the block's i18n context to the ICU.
However, we neglected to set the contextKind to inidcate that the
context was associated with an ICU. As of this change we now set the
correct contextKind.
This change also refactors the context creation to explicitly separate
creation of contexts for attributes, root i18n blocks, child i18n
blocks, and ICUs. This allows us to more easily ensure that contexts are
shared appropriately between i18n blocks and ICUs.
Finally, this change also refactors the i18n message extraction pahse to
simplify how contexts are converted to i18n messages. This
simplification should make it easier to merge i18n contexts and i18n
messages into a single op in a future refactor.
PR Close#53557
Whenever an input of a directive changes, the semantic symbol should
reflect this change for the type check API. This is important because
signal inputs require special output in the type checking blocks- hence
we need to ensure that such type checking blocks are re-generated
properly.
Test verify that incremental type-checking builds work as expected now.
PR Close#53521
Whenever a signal input is captured in a type check block, we will
insert an import. This will change the import graph so that the full
TypeScript program cannot be structurally re-used.
We can fix this trivially by ensuring the import graph remains stable,
by always generating an import to e.g. `@angular/core`. This fixes the
issue nicely for type-check block files. A test verifies this.
For inline code, such as TCB inline or the type constructors inline,
this fix is not applicable because we would change user-input source files,
adding new edges that would not exist for subsequent builds- causing the
program to be not re-used completely. One idea was to rely on the
existing edge that can be assumed to exist for directive code files.
This is true technically, but in practice TS does not deduplicate
imports- so our new namespace import when referencing our symbols will
invalidate the re-use. We will address this in a follow-up. There are a
couple of options, such as working with the TS team, updating the
existing edge, or inlining our helpers as well.
PR Close#53521
`o.WrappedNodeExpr` can show up in some cases, when a host binding's value is inside a TS expression.
It's an open question whether we will need to support all of the TS expression types as a result.
PR Close#53478
For some reason, the parser reuses the same field to store the animation phase and the event target. We were incorrectly interpreting the presence of any value on that field as an animation phase, leading us to incorrectly emit synthetic listener instructions for listeners on events with targets. This bug is now fixes.
PR Close#53478
`$any` should be interpreted as a cast, not as a context read of a variable called `$any`. This already worked in template compilations, but the relevant phase was not enabled for host bindings.
PR Close#53478
Fixes that the compiler was throwing an error if an ambient type is used inside of an input `transform` function. The problem was that the reference emitter was trying to write a reference to the ambient type's source file which isn't necessary.
Fixes#51424.
PR Close#51474
Adds support for sanitizing host bindings. Since the tag name of the
element the host binding is being set on isn't always known, we have to
consider multiple possible security contexts.
This commit also adds additional tests to help verify correct behavior
of the sanitization logic for different edge cases.
PR Close#53513
Use the DomElementSchemaRegistry to determine the correct security
context for static attributes, and pass it along during ingestion. Then
during the resolve sanitizers phase, use the security context to
determine if a trusted value function is needed
PR Close#53473
Consider the case:
```
<button *ngIf="true" [@anim]="field"></button>
```
Only the inner `button` should recieve a `property` instruction for the animation binding. We were previously emitting one for the implicit `ng-template` as well, and collecting it into the consts for the `ng-template`. Both of these issues are now fixed.
PR Close#53457
The behavior of explicit bindings on `ng-template`s was untested, and we differed from `TemplateDefinitionBuilder` significantly. We now have much more similar behavior, although not 100% identical.
For example, consider this templarte:
```
<ng-template l="l1" [p]="p1" [attr.a]="a1" [class.c]="c1"></ng-template>
```
It's not clear what a class binding on an `ng-template` would actually do. Nonetheless, it's well-defined behavior in TemplateDefinitionBuilder, which emits `property` instructions for all three bindings, and people actually do this in google3.
Note that some of these bindings don't really make much sense, but we have to support them for compatibility purposes.
See comments for an in-depth explanation of all the logic.
Also, add a test to exercise the problematic case.
PR Close#53457
The Template Pipeline has had a number of tricky bugs involving bindings on structural elements.
Consider this template:
```
<div *ngIf="true" [class.bar]="field"></div>
```
We were incorrectly emitting `ɵɵclassProp` on *both* the template's view, and the inner view. The solution is to just emit an extracted attribute on the enclosing template, so it still shows up in the const array, but does not affect the update block.
We will refactor binding ingestion soon, but this commit improves our correctness before any big refactor.
PR Close#53457
These patches are no longer necessary with the current state of the
type packages and the code within the repository. The types are now
included in the already required babel.d.ts file for the relevant
babel packages (currently: `@babel/core` and `@babel/generator`).
PR Close#53441
@for does not use actual TemplateOps, but instead has a similar
RepeaterCreateOp. This commit adds support for this op to the relevant
i18n phases.
PR Close#53440
This test actually passes, template pipeline just orders the translated
messages and consts array differently. Since the order isn't important,
we just fork off an alternate golden file for template pipeline.
PR Close#53459
I discovered this failure while looking at presubmit results. We appear to still have ordering issues, when more update ops are present.
PR Close#53405
While running a g3 presubmit, I discovered two related novel failure modes:
1. Simple case: this new test uses an `ngFor` structural directive, which binds a context variable. That variable is immediately used in an attribute binding. It looks like we generate an extra attribute instruction, which might result in an invalid property read at runtime.
2. Complex case: this is another attribute binding, this time on a structural element, inside of an `ng-template`. Not sure what's going on here.
PR Close#53405
Previously, binding ops only knew whether they applied to a structural template (and even this was actually very misleading!).
Now, binding ops have full information about what kind of template they apply to, if any (e.g. plain template, structural template, etc). Additionally, each binding knows whether it `IsStructuralTemplateAttribute`, which is a property of the binding rather than the template target.
In the future, we should refactor this to unify the various flags that can describe binding types, as well as the flags that describe template targets, into a single and comprehensive field on binding ops.
PR Close#53405
Previously, we created i18n contexts for i18n attributes in ingest. This turned out to be the wrong approach, because we don't always want to produce i18n messages for all i18n attributes! In fact, several kinds of i18n attributes on elements with structural directives should not produce their own messages.
This commit also contains related refactors to fix one such structural directives test.
PR Close#53405
When a binding is present on an element with a structural directive, that binding is parsed onto *both* the synthetic `ng-template`, as well as the inner element. However, we do not want to create different i18n messages for both bindings; we only want to generate a new i18n message for the inner, "real" element.
PR Close#53405
Listener instructions should not be inside the i18n block. In order to avoid this, we ingest bindings on an element before starting the i18n block.
We previously missed this case because almost all bindings result in *update* instructions, which don't need to be ordered relative to i18nStart/i18nEnd create instructions. However, listeners are the only kind of binding that gets ingested into the create block.
PR Close#53405
Previously, our i18n slot moving process was buggy. Specifically, it was not resilient to cases in which a create op consumed a slot, but no update ops depended on that slot.
The new algorithm fixes this issue, and is also easier to understand.
PR Close#53405
I18n expressions logically have both a target and an owner:
- For i18n text expressions, the owner is the i18nStart instruction. The target is initially the same, but later moves to be the last slot consumer in the i18n block.
- For i18n attribute expressions, the owner is the I18nAttributes config instruction, whereas the target is the ElementCreate that hosts the attribute.
This refactor makes the code clearer in quite a few plases.
Additionally, we now perform a lot of the i18n processing earlier. For example, re-targeting and re-ordering of i18n expressions happens *before* apply instructions are generated. As a result, the re-ordering logic is a lot simpler.
These changes also have consequences on i18n const collection, along with a couple other minor changes.
PR Close#53376
These changes add an option to the `extendedDiagnostics` field that allows the check from #53190 to be disabled. This is a follow-up based on a recent discussion.
PR Close#53311
Add support for i18n attributes:
- Generate i18n contexts from i18n attributes, and extract the eventual messages into the constant pool.
- Emit I18nAttributes config instructions when needed.
- Use the generated i18n variable in the appropriate places, including extracted attribute instructions, as well as I18nAttributes config arrays.
PR Close#53341
Currently we generate the following TCB for a `@for` loop:
```ts
// @for (item of items; track item) {...}
for (const item of this.items) {
var _t1 = item;
// Do things with `_t1`
}
```
This is problematic if the item name is the same as a global variable (e.g. `document`), because when the TCB has references to that variable (e.g. `document.createElement`), it'll find the loop initializer instead of the global variable.
These changes fix the issue by generating the following instead:
```ts
for (const _t1 of this.items) {
// Do things with `_t1`
}
```
Fixes#53293.
PR Close#53319
Previously we recorded separate param values for a strucural directive
and the element tag it goes on. We then later attempted to combine those
into a single value. However in some cases this merging logic matched
the directive with the wrong tag.
This change implements an alternate approach where we match the
directive to its element tag from the start, while we're traversing the
ops. This should be a more robust solution.
PR Close#53327
We previously failed to populate the attributes property on projection
ops, this commit populates it and later strips out the "select"
attribute.
PR Close#53327
Previously we failed to reset the sub-template index counter when we
exited a root block. This caused following sibling blocks to start
counting at the wrong index.
PR Close#53327
It is possible for ICUs to be nested inside other ICUs. This change
adjusts our ingestion logic to create extra interpolation ops for the
nested ICUs during ingestion.
PR Close#53300
We previously had an assertion that every placeholder in the i18n AST
had a corresponding param in the output. However, there are some cases
such as interpolations nested inside ICUs where this assertion is not
true. This change simply removes the asserion.
PR Close#53300
ICUs may share a placeholder, and in that case they need special
post-processing. This change adds logic to cover this possibility. In
particular, we set the param to a special placeholder value and then
pass an array containing the sub-message variables as a post-processing
param.
PR Close#53300
When we re-assign the slot dependencies for the i18nExprs, we should
move them down below the other ops that target their same slot. This
keeps the behavior consistent with TDB
PR Close#53300
This commit fixes an issue where having an expression with nullish coalescing in styling host bindings leads to JS errors due to the fact that a declaration for a temporary variable was not included into the generated code.
Resolves#53295.
PR Close#53305
As part of this fix, I realized that child i18n blocks don't need their
own context. Instead, we can just add their params directly to the
context for their root block, and forgo the step of merging the contexts.
PR Close#53209
ICU sub-messages should be recorded as belonging to the message for the
root i18n block they are part of. This ensures that they still get
emitted even if they are nested in a child template.
PR Close#53209
This is a follow-up to the fix from #52414. It adds a diagnostic that will tell users when a control flow is preventing its direct descendants from being projected into a specific component slot.
PR Close#53190
Adds support for inheriting host directives from the parent class. This is consistent with how we inherit other features like host bindings.
Fixes#51203.
PR Close#52992
This is a follow-up to the fix from #52414. It adds a diagnostic that will tell users when a control flow is preventing its direct descendants from being projected into a specific component slot.
PR Close#52726
The `$first`, `$last`, `$even` and `$odd` variables in `@for` loops aren't defined on the template context of the loop, but are computed based on `$index` and `$count` (e.g. `$first` is defined as `$index === 0`). We do this calculation by looking up `$index` and `$count` when one of the variables is used.
The problem is that all `@for` loop variables are available implicitly which means that when a nested loop tries to rewrite a reference to an outer loop computed variable, it finds its own `$index` and `$count` first and it doesn't look up the ones on the parent at all. This means that the calculated values will be incorrect at runtime.
These changes work around the issue by defining nested-level-specific variable names that can be used for lookups (e.g. `$index` at level `2` will also be available as `ɵ$index_2`). This isn't the most elegant solution, however the `TemplatDefitinionBuilder` wasn't set up to handle shadowed variables like this and it doesn't make sense to refactor it given the upcoming template pipeline.
Fixes#52917.
PR Close#52931
Reworks the `repeater` instruction to go through `advance`, instead of passing in the index directly. This ensures that lifecycle hooks run at the right time and that we don't throw "changed after checked" errors when we shouldn't be.
Fixes#52885.
PR Close#52935
In some cases ICU expression placeholders may have trailing spaces that
need to be trimmed when matching the placeholder to its corresponding
text binding.
PR Close#52698
We were previously counting the i18n expression index and deciding when
to apply i18n expressions based on the i18n context. These should be
done based on the i18n block instead.
PR Close#52698
The previous commit added support for interpolated text in ICUs, but it
made the assumption that the interpolation would be a single variable
read expression.
To properly support all kinds of interpolation expressions, this commit
refactors how ICUs are ingested to allow us to re-use the same logic we
use for bound text outside of ICUs.
To accomplish this, the `IcuOp` creation op has been removed in favor of
a pair of ops: `IcuStartOp` and `IcuEndOp`, that mark the beginning and
end of the ICU. Now, instead of inserting an `IcuUpdateOp` in the update
IR, we call `ingestBoundText` and use the presence of the surrounding
`IcuStartOp` and `IcuEndOp` to match the interpolation with the ICU.
PR Close#52698
Previously ICUs were assumed to only generate a single i18n expression
per ICU. However, it is possible for ICUs to contain text interpolations
which requires additional expressions. This commit adds support for
multiple expressions per ICU.
PR Close#52698
ICUs that contain element tags need extra parameters for the i18n
message. These are in addition to the element slot params that are
already added to the parent i18n block's params. In this commit we add a
new phase to fill in these placeholders.
PR Close#52698
Previously the template pipeline sorted i18n message params before
adding the sub-message placeholders. Now its sorts after all
placeholders are added.
Both the template pipeline and TemplateDefinitionBuilder previously
failed to sort the post-processing params. They both now sort these as
well. This is safe to change in TemplateDefinitionBuilder, as it does
not change anything about the functionality, it simply ensures that
params map in the output has the keys ordered in a way that can be
easily reproduced in the template pipeline.
PR Close#52698
Fixes that all implicit variables in `@for` loops were inferred to be numbers, even though most are actually boolean.
Note that I also had to work around a weird TypeScript behavior in `tsDeclareVariable` where usually we declare variables in the following format:
```
var _t1: <type> = null!;
```
This works in most cases, but if the type is a `boolean`, TypeScript infers the variable as `never`, instead of `boolean`. I've worked around it by adding an `as boolean` to the initializer.
Fixes#52730.
PR Close#52732
Adds the private `_enableBlockSyntax` flag that can be used by the language service to disable blocks on apps that aren't on Angular v17.
PR Close#52683
Now that two-way bindings work correctly with implicit receivers, we can fix the corresponing source map tests. The main issue was that we were not properly mapping `elementEnd` for elements with no closing tag (self-closing elements).
PR Close#52479
Some two-way bindings tests were not working properly, because we could not ingest the implicit receiver required to write to the `ngModelChanges` property. Now, we properly resolve that implicit receiver to the root component context.
Also, add some tests, both for the simple case, and the case where the listener is inside a nested view.
PR Close#52479
Some `defer` blocks have external dependencies on other components or directives. These dependencies need to be extracted into deps functions, which either return local deps, or use a dynamic import for non-local deps. Template Pipeline can now generate these functions.
PR Close#52479
When an `ng-template` has local refs, such as `<ng-template #foo>`, we must emit a `ɵɵtemplateRefExtractor` argument to the template creation functino. The template pipeline now supports this.
PR Close#52479
`TemplateDefinitionBuilder` is somewhat unreliable about extracting constant attributes (e.g. `[attr.foo]="'one'"`). It never extracts const non-string expressions, and usually, but not always, extracts const string expressions. Template pipeline consistently extracts const strings, and we add new goldens for a couple such cases.
PR Close#52479
Some defer triggers, such as `hover`, expect a local reference as an argument. For example, `@defer (on hover(target))` waits until the user hovers over the target.
However, these defer conditions also have a nullary form, in which the trigger is implicitly the first element in the placeholder block. We now support that case in template pipeline.
PR Close#52479
We already supported `defer on` conditions, which become instructions in the create mode block.
Now, we also support `defer when` conditions, where are very similar, with the notable difference that they go in the update block (because a user-supplied condition must be re-evaluated on each update.)
PR Close#52479
Previously we supported ICUs where the ICU itself represetned the entire
translated message. This change allows ICUs to act as a sub-message
inside other translated messages.
PR Close#52503
Discovered this while validating #52414 against Angular Material. We were projecting `<ng-template>` nodes at the root of `@if` and `@for` with the `ng-template` tag name which enables directive matching and applies the directive to the control flow node.
These changes fix the issue by never passing along the `ng-template` tag name.
PR Close#52515
TypeScript JsDoc parsing, by default, treats occurences of Angular decorators (e.g. `@Component`) in JsDoc comments as JsDoc tags. This commit escapes these decorator strings by copying the raw JS doc onto a dummy symbol in a new SourceFile to make TypeScript re-parse the comment.
PR Close#52481
With the directive-based control flow users were able to conditionally project content using the `*` syntax. E.g. `<div *ngIf="expr" projectMe></div>` will be projected into `<ng-content select="[projectMe]"/>`, because the attributes and tag name from the `div` are copied to the template via the template creation instruction. With `@if` and `@for` that is not the case, because the conditional is placed *around* elements, rather than *on* them. The result is that content projection won't work in the same way if a user converts from `*ngIf` to `@if`.
These changes aim to cover the most common case by doing the same copying when a control flow node has *one and only one* root element or template node.
This approach comes with some caveats:
1. As soon as any other node is added to the root, the copying behavior won't work anymore. A diagnostic will be added to flag cases like this and to explain how to work around it.
2. If `preserveWhitespaces` is enabled, it's very likely that indentation will break this workaround, because it'll include an additional text node as the first child. We can work around it here, but in a discussion it was decided not to, because the user explicitly opted into preserving the whitespace and we would have to drop it from the generated code. The diagnostic mentioned point #1 will flag such cases to users.
Fixes#52277.
PR Close#52414
Prior to this change, the transform function would be referenced with a potentially
relative import into an external declaration file. Such imports are not portable
and should not be created in this context. This commit addresses the issue by threading
though the originally used module specifier by means of the `Reference` type.
Fixes#52324
PR Close#52437
This commit fixes an issue where using literal types in the arguments of an input coercion
function could result in emitting invalid output, due to an assumption that TypeScript makes
when emitting literal types. Specifically, it takes the literal's text from its containing
source file, but this breaks when the literal type node has been transplanted into a
different source file. This issue has surfaced in the type-check code generator and is
already being addressed there, so this commit moves the relevant `TypeEmitter` class
from the `typecheck` module to the `translator` module, such that it can be reused for
emitting types in the type translator.
Fixes#51672
PR Close#52437
The previous commits provided the scaffolding for `defer on`. In this commit, we build on that work, adding triggers for `immediate`, `timer`, `hover`, and `viewport`.
PR Close#52387
Previously, we supported a `HasConst` trait, allowing an op to be const collected automatically. However, that approach had the shortcoming that each op could only collect a single constant.
Instead, we now provide a `ConstCollectedExpr`, which collects constants at the expression level, allowing ops to have multiple collectible consts.
Then, we use this new abstraction to support the `defer on` conditions.
PR Close#52387
Previously, we had an "empty shell" implementation of defer conditions, and we used separate ops to represent secondary defer blocks.
Now, we have a real scaffolding for supporting the various defer conditions, and the secondary defer block information has been refactored onto the main defer op.
Additionally, to enable this, we refactor the way that using slot indices works. Instead of having a trait that causes users of slot indices to be linked to the allocated slot, we share a single `SlotHandle` object by reference. This allows an op to use slot information for more than one Xref at a time, and eliminates a layer of indirection.
Co-authored-by: Alex Rickabaugh <alxhub@users.noreply.github.com>
PR Close#52387
Enables a handful of i18n tests that are currently skipped, but pass if
enabled. Some of them require alternate golden files because of
inconsequential differences in the cost array order.
PR Close#52390
This commit expands docs extraction for classes and interfaces to include inherited members. This relies on the type checker to get the _resolved_ members of the type so that the extractor doesn't need to reason about inheritance rules, which can get tricky (especially with regards to method overloads).
PR Close#52389
This commit adds decorators to the extracted API docs. It makes some
very hard-coded assumptions about the pattern used to declare decorators
that's extremely specific to what the framework does today.
PR Close#52389
Previously, we would emit *two* pipe creation instructions for each pipe in a switch case. This is because we were visiting both the transformed and raw versions of the pipe bindings.
Now, we clear the raw case expressions array after generating the transformed test expression.
Also, we introduce some new goldens, because our pipe creation order is harmlessly different.
PR Close#52289
We roughly attempt to match TemplateDefinitionBuilder's pipe creation order, by placing pipe creation instructions after their target elements. However, we cannot fully emulate the "inside-out" ordering TemplateDefinitionBuilder uses when multiple pipes apply to one element, because TemplateDefinitionBuilder creates the pipes as expressions are visited, from the leaves up. Our order is perfectly adequate though.
We also add a non-compatibility-mode ordering, which just appends them to the end of the create block. This is better because it allows for more chaining opportunities.
PR Close#52289
Singleton property interpolation instructions consume only one variable, but are still emitted as an interpolation instruction (they cannot be collapsed because `propertyInterpolate` implicitly stringifies its argument.)
PR Close#52289
We were incorrectly emiting a extracted constant pool index for the final argument of the projection instruction. It actually takes an array literal.
(N.B.: This means we re-create the array every time! We should probably modify the runtime to use a const index for this.)
Additionally, we alter the projection op to not extend the element op base type.
PR Close#52289
The correct order of attributes and properties is:
1. Interpolated properties
2. Interpolated attributes
3. Non-interpolated properties
4. Non-interpolated attributes
This includes an additional nuance: singleton attribute interpolations, such as `[attr.foo]="{{bar}}"`, will be "collaped" into a simple `attribute` instruction. However, this is *not* the case for singleton property interpolations! The ordering phase must take this nuance into account to match the TemplateDefinitionBuilder order.
After the project lands, it might be nice to also collapse singleton property interpolations.
PR Close#52289
Previously, we ran the ordering phase near the end of the compilation. However, this meant that phases like slot assignment and variable offset assignment would happen first, and then the nice, monotonically-increasing orders would be scrambled by the reordering.
It's much more intelligible to order first, and then perform these assignments. However, to make this happen, some modifications to the ordering phase are required. In particular, we can no longer rely on `advance` instructions to break up orderable groups.
PR Close#52289
Many instructions consume variable slots, which are used to persist data between update runs. For top-level instructions, the offset into the variable data array is implicitly advanced, because those instructions always run.
However, instructions in non-top-level expressions cannot be assumed to run every time, because they might be conditionally executed. Therefore, they cannot implicitly advance the offset into the variable data, and must be given an explicitly assigned variable offset.
TemplateDefinitionBuilder assigned offsets top-to-bottom for all instructions *except* pure functions. Pure functions would be assigned offsets lazily, on a second pass.
Template Pipeline can now imitate this behavior, when in compatibility mode: pure functions are assigned offsets on a second pass.
This also makes the "variadic var offsets" phase unnecessary -- the new approach is more general and correct.
PR Close#52289
Previously, inside an event listener, template pipeline would always save the context from restoring a view, e.g.
```
const restored_ctx = r0.ɵɵrestoreView(s);
```
This is usually correct! However, consider the case of a listener in the template's root view. The appropriate context will already be available via closure capture, and we can just use it (as `ctx`).
Now, the context resolution phase understands that we don't need to use the restored view's saved context if we would have access to it by closure.
Note: we also create a new golden, because the const array is in a harmlessly different order.
PR Close#52289
Previously, the template pipeline did not handle "empty" reads gracefully: it would emit syntactically invalid reads of empty properties. Now we read `$implicit`.
This allows us to enable a test that relies on `$implicit`. However, we also have to create another golden, because our variable inlining is more aggressive.
PR Close#52289
We currently allow elements to be collapsed around pipe creation instructions. TemplateDefinitionBuilder disallows this, but only sometimes. Collapsing in this case is actually less generated code, and it's OK to allow it.
PR Close#52289
The template pipeline can now generate track functions, and extract them into the constant pool (or optimize them if needed). Additionally, context variables such as `$index` can be used inside track functions and for loop bodies.
PR Close#52001
Add support for `repeaterCreate` and `repeater` instructions. Correctly count decls and vars, and support primary and empty blocks.
`track` functions are not yet extracted.
PR Close#52001
In #52110 the compiler was changed to produce `if` statements when type checking `@switch` in order to avoid a bug in the TypeScript compiler. In order to avoid duplicate diagnostics, the main `@switch` expression was ignored in each of the `@case` comparisons. This appears to have caused a regression where comparing incompatible types wasn't being reported anymore.
These changes resolve the issue by wrapping the expression in parentheses which allows the compiler to report comparison diagnostics while ignoring diagnostics in the expression itself.
Fixes#52315.
PR Close#52322
Now the method `getConstructorDependencies` no longer needs to do any post analysis, and can rely on the reflection host's result to generate ctor params. This will automatically include invalid factories which fix the issue.
PR Close#52215
ICUs can be used outside of an i18n block. In this case the ICU should
be automatically wrapped in a new i18n block. This commit adds a new
phase to handle wrapping these bare ICUs.
PR Close#52250
ICU params in i18n messages are now resolved in the post-processing call
rather than in the initial message creation. This matches the output
generated by TemplateDefinitionBuilder.
PR Close#52250
ICUs are now ingested by adding ops to both the creation and update IR.
Both of these ops are ultimately removed before reification, but they
are needed to coordinate and link data between the creation and update
ops. This is done in a new ICU extraction phase that removes both ICU
ops and adds an i18nExpr op to the update IR.
PR Close#52250
This commit extracts the API reference info for generic parameters for
classes, methods, interfaces, and functions. It includes any constraints
and the default type if present.
PR Close#52204
Placing a structural directive on an element with an `i18n` attribute
was generating too many i18n blocks. This was due to both the element
and the template generating their own i18n block. To fix the issue, we
no longer generate top-level i18n blocks for structural directive
templates.
PR Close#52202
Structural directives on an ng-template (e.g. <ng-template *ngIf>) were
being assigned the wrong tag name ('ng-template' instead of null).
PR Close#52202
Fixes handling of placeholders for self-closing tags. Self-closing tags
set a combined value for the start tag placeholder, rather than separate
values for the start and close placeholders.
This commit also enables a number of now passing tests. For some of
these tests I had create a separate golden file due to the different
ordering of the const array. In the template pipeline, i18n and
attribute const collection happen in different pahses and we therefore
get a different order than TemplateDefinitionBuilder, which collected
everything in one pass. The order should not affect the overall behavior.
PR Close#52195
The way we were propagating params up to parent i18n ops didn't account
for the fact that a parent and child could both have a value for the
same placeholder. In order to properly merge the value for these cases,
we need to propagate the params up *before* serialization. Therefore I
removed the standalone param propagation phase and folded the logic into
the placeholder resolution phase.
PR Close#52195
Currently the compiler allocates a variable slot to the `@for` loop expression which ends up unused since we don't store the result on the `LView`.
PR Close#52158
A new flag added to the component's debug info to determine whether to throw runtime error (in dev mode) if component is being rendered without its NgModule. This flag is only set for non-standalone components.
PR Close#52061
Orphan component is an anti-pattern in Angular where a component is rendered while the NgModule declaring it is not installed. It is not easy to capture this scenario, specially in compile time. But it is possible to capture a special case in runtime where the component is being rendered without its NgModule even loaded into the browser. This change adds a flag in cli compiler option to enable such checking, and throwing a runtime exception if it happens. Note that such check is only done in dev mode.
Currently the check requires some generated code that is behind ngJitMode flag (i.e., call to ɵɵsetNgModuleScope), and the new flag can be set only if JIT mode is enabled (i.e., supportJitMode=true) otherwise an error will be thrown.
The orphan component is a main blocker for rolling out local compilation in g3. This option is needed for identifying and isolating such cases.
PR Close#52061
Based on recent discussions, these changes remove the Windows CI check because it has been too flaky for too long. Furthermore, we've concluded that the simulated file system in the compiler tests already catches the same set of bugs as running the tests on a real Windows system.
PR Close#52140
This commit adds support for extracting type alises. It currently
extracts the raw written type from the source without performing any
resolution, such as for resolving `typeof` queries, as current Angular
public APIs do not rely on this.
PR Close#52118
Fixes that the new block syntax was generating instructions in the wrong order which meant that pipes were being declared too early. This meant that if the block is first in the template, any pipes used in it won't be able to inject things like `ChangeDetectorRef`.
These changes update the compiler and add a bunch of tests to ensure that pipes work as expected.
Fixes#52102.
PR Close#52112
Since expressions in event listener are added inside of a callback, type narrowing won't apply to them anymore. These changes add the logic to create a guard expression that will re-narrow the expression in the callback.
Fixes#52052.
PR Close#52069
Since expressions in event listener are added inside of a callback, type narrowing won't apply to them anymore. These changes add the logic to create a guard expression that will re-narrow the expression in the callback.
Fixes#52052.
PR Close#52069
This commit updates `@defer` logic related to handling `after` and `minimum` parameters tree-shakable.
If `after` or `minimum` was used on a `@loading` or `@placeholder` blocks, compiler generates an extra argument for the `ɵɵdefer` instruction. This extra argument is a reference to a function that brings timer-related code.
PR Close#52042
A new statement will be generated for components which will attach some useful debug info to them to be used in runtime error handling. Currently this only happens in full and local compilation modes.
PR Close#51919
The current implementation assumes a qualified name consists of just two identifier, e.g., Foo.Bar. However it can be more nested, like Foo.Bar.Baz.XX.YY. While such nested patterns are quite uncommon and devs mostly just use two identifier here, the TS compiler seems to throw error if we make such assumption and it broke quite a lot of targets in g3 when compiled in local mode. So here we handle this nested property of qualified names.
PR Close#51947
The custom logic in the generate advance phase for i18n expressions did
not work in all cases. Instead we add a new phase to update the
expression's target op, and then allow the standard advance generation
code to determine the number of advance instructions needed.
Co-authored-by: Dylan Hunn <dylhunn@users.noreply.github.com>
PR Close#51988
This commit adds support for extracting function overloads. Interestingly, this worked in an earlier version when the code was extracting all statements in every source file, but the existing compiler API for extracting all exported declarations from an entry-point only returns the first function declaration in cases when there are overloads.
This also marks abstract classes as abstract, required inputs as required, and filters out Angular-private APIs.
PR Close#52040
We type check `@switch` blocks by generating identical TS `switch` statements in the TCB, however TS currently has a bug where parenthesized `switch` block expressions don't narrow their types. Since we use parenthesized expressions to wrap AST nodes for diagnostics, this will bug will affect all Angular-generated `switch` statements.
These changes work around the issue by generating `if`/`else if`/`else` statements that represent the `switch`.
Some alternatives that were considered:
1. Moving the `switch` expression to a constant - this is fairly simple to implement, but it won't fully resolve the narrowing issue since the same constant will have to be used in expressions inside the different cases.
2. Removing the outer-most parenthesis from the switch expression - this works and allows us to continue using switch statements, but because we use parenthesized expressions to map diagnostics to their template locations, I wasn't sure if it won't lead to worse template dignostics.
Fixes#52077.
PR Close#52110
Fixes that the compiler wasn't picking up pipes used inside defer block triggers as dependencies. We had implemented the `visitDeferredTrigger` visitor method, but it wasn't being called, because we weren't going through the `visitAll` method of the deferred block. We don't use `visitAll`, because child nodes have to be processed differently than the connected blocks and triggers.
Fixes#52068.
PR Close#52071
The `_enabledBlockTypes` config option was removed recently, since we've enabled @-syntax by default. This commit removes `_enabledBlockTypes` references from the `compiler-cli` test cases.
PR Close#52066
This adds API doc extraction for interfaces, largely using the same code paths for classes. The primary difference between classes and interfaces is that classes have member _declarations_ while interfaces have member _signatures_. This largely doesn't matter for the purposes of extraction, but the types are distinct with no common base types, so we have to do a fair amount of type unioning and aliasing.
PR Close#52006
A couple tests were already passing, and just needed to be enabled. This includes tests pertaining to:
* ng-template
* host binding styling slots
* and host animation bindings
* some literal tests (which were missing some $foo$ escaped names)
We add pipeline-specific versions of the following tests, and enable them:
* A local refs test. The consts for the element attributes and the consts for local reference are collected in the reverse order, but the emitted template is functionally the same.
* A safe accesstest. Consider the expression `$any(val)?.foo`. `TemplateDefinitionBuilder` extracts a temporary variable: `($tmp_0_0$ = $ctx$.val) == null ? null : $tmp_0_0$.foo`. It presumably does this because it considers the `$any(...)` to be a function call. However, this is not a real call, so Template Pipeline safely ignores it and declines to generate a temporary.
* Another local refs test. AttributeMarker.Template is emitted at the end of the const array (instead of the middle)
PR Close#51950
Consider an `ng-template` which is generated as a result of a structural directive:
```
<div *ngFor="let inner of items"
(click)="onClick(inner)"
[title]="getTitle()"
>
```
This should logically expand into something like the following:
```
<ng-template [ngForOf]="..." >
<div (click)="..." [title]="..."></div>
</ng-template>
```
Note that the `(click)` handler and the `[title]` property are only present on the inner div, *not* on the enclosing generated `ng-template`.
Previously, Template Pipeline would place these bindings on *both* the tempate and the inner element.
However, we can't just remove them completely, because these bindings should still be matchable on the generated `ng-template` (which is very surprising, but nonetheless true).
We resolve this issue with two improvements:
(1) The ingestion step is now much smarter about determining not only if a binding is on a template element, but whether it actually targets that template element.
(2) We use `ExtractedAttributeOp` directly, rather than going through `BindingOp`, to cause the `ng-template` to still receive these bindings in its `consts` array for matching purposes.
PR Close#51950
For components, the parser already extracts the `important` property (and it is later disregarded). However, because host bindings use a totally separate parsing code path, this was never happing for host bindings.
Here, we add some code to the host style parsing phase to drop the `!important` suffix.
We could solve this category of problems for good by parsing host bindings with the same code as template bindings.
PR Close#51950
Previously, we always generated temporary variable declarations at the beginning of each view's update block. This is wrong, for two reasons:
1. Temporaries can be used in the create block
2. When listeners use temporaries, we should declare them inside the listener.
Now, we always place temporaries at the beginning of the enclosing OpList, and recursively try to generate them when we find a listener.
PR Close#51950
