Previously, defer deps fns names were only prefixed with the component name, meaning that distinct deps fns in the same component would produce a name collision. Now, we take into account the entire template function name when naming inner deps fns.
PR Close#54060
This commit introduces three additional diagnostics for queries:
- If a query (either using decorator or signal-based) is declared on a
static class member, a diagnostic is raised.
- If a signal-based query is mixed with a query decorator, a diagnostic
is raised. Similar to signal inputs.
- If a singal-based query is also declared in the directive/component
class decorator metadata, a diagnostic is raised.
PR Close#54019
Collapses multiple sibling query advance statements into single
query advance invocations. This will help reducing generated code
for directives/components with many queries.
PR Close#54019
Previously, if an ICU was inside a nested i18n root, it would use the nested root to calculate whether it should be applied. Now, we use the root i18n block.
PR Close#54026
This commit adds compliance tests to ensure that the generated output of
signal-based queries matches our expectation.
Note: collapsing query advance instructions is not implemented yet.
PR Close#53978
Adds a compiler integration test for recognizing signal-based queries,
and emitting the expected output. Concrete output will be verified via
the compliance tests.
PR Close#53978
This commit introduces the compiler output generation for signal-based
queries. Signal-based queries will have new creation-mode instructions
and update instructions to advance the current query indices in the
global shared context.
An output like the following is the expected output for signal-based
queries:
```
i0.ɵɵdefineComponent({
viewQuery: function App_Query(rf, ctx) {
if (rf & 1) {
i0.ɵɵviewQuery(ctx.d, _c0, 5);
i0.ɵɵviewQuerySignal(ctx.ds1, _c0, 5);
i0.ɵɵviewQuerySignal(ctx.ds2, _c0, 5);
}
if (rf & 2) {
let _t;
// only change-detected queries need explicit refresh
i0.ɵɵqueryRefresh(_t = i0.ɵɵloadQuery()) && (ctx.d = _t.first);
// we bump up current query index by 2 positions since there are 2 signal-based queries
i0.ɵɵqueryAdvance(2);
}
…
},
…
});
```
Note: For now, the collapsing of multiple advance instructions is not
implemented. This will be a follow-up.
Note 2: A couple of query helpers are now in their own file. This makes
it easier to focus on query-specific compiler code. The new function is
called `createQueryCreateCall`, which is a modified variant of the
existing function that previously only generated query parameters.
PR Close#53978
At the moment local compilation breaks for host directives because the current logic relies on global static analysis. This change creates a local version by cutting the diagnostics and copying the directive identifier as it is to the generated code without attempting to statically resolve it.
PR Close#53877
At the moment when unified host is selected (through option `_useHostForImportGeneration`) the compiler always generates alias reexports. Such reexports are mainly generated to satisfy strict dependency condition for generated files. Such condition is no longer the case for G3. At the same time, these alias reexports make it impossible to mix locally compiled targets with globally compiled targets. More precisely, a globally compiled target may not be able to consume a locally compiled target as its dependency since the former may import from the alias reexports which do not exist in the latter due to local compilation mode. So, to make global-local compilation interop possible, it is required to be able to turn off alias reexport generation.
PR Close#53937
This commit adds extra logic to produce a diagnostic in case `@Component.deferredImports` contain types from imports that also bring eager symbols. This would result in retaining a regular import and generating a dynamic import, which would not allow to defer-load dependencies.
PR Close#53899
Currently when the extended type check fails due to an import reference
that cannot be generated, the fatal diagnostic is not caught and
not properly exposed as a `ts.Diagnostic` that can be gracefully
handled. This is inconsistent to non-extended type checking diagnostics.
This is problematic because Angular CLI applications currently fail in
obscure ways because:
- the CLI does not expect `getDiagnosticsForFile` to actually throw
runtime errors.
- the CLI does not seem to properly print these errors given the
parallel workers and build excection, and those errors are
especially hard to debug because there is no `stack` for
`FatalDiagnosticError`'s.
Example: `MyDir` is not exported and the type check block cannot reference it.
PR Close#53896
This allows us to ensure signal inputs and a potential JIT transform
remain single file compilation compatible. The consequences are that
options need to be statically analyzable more strictly, compared to
loosened restrictions with static interpretation where e.g. `alias`
can be defined through a shared variable.
PR Close#53872
Currently when someone declares a signal or non-signal input on a static
class member, the compiler will not yield any diagnostic. We can detect
these mistakes and report a diagnostic to help our users.
PR Close#53808
This commit addds two diagnostics for two scenarios where signal inputs
are declared incorrectly:
- a signal input is also annotated with `@Input` in the TypeScript
sources.
- a signal input is also declared in the `inputs` option of
`@Directive/`@Component`.
PR Close#53808
This commit adds a transform for supporting input signals in JIT
environments. The transform will be wired up for Angular CLI
applications automatically. An integration test verifies that this fixes
unit testing with signal inputs.
The transform basically will take the signal input metadata and
transform it into `@Input` decorators that can provide static
information to the Angular JIT runtime when the directive/component
definition is compiled.
PR Close#53808
Adds signal input compliance tests, ensuring linking works as expected,
partial output is generated properly, types are inferred properly, and
that the full, or linked output matches our expected runtime structure.
PR Close#53808
This commit changes the `HasTransform` flag to be only concerned with
decorator inputs. This allows us to automatically detect signal input
transforms without reliance on the flag, resulting in less complexity in
the compiler (as outlined in the design doc) and various other places,
while it also allows us to simplify JIT support for signal inputs
because there would be no need to capture the "hasTransform" state in
the decorator so that JIT can generate the according input flags.
`isSignal` will still persist as an input flag to allow for monomorphic
and highly efficient distinguishing at runtime, whether an input is
signal based or not. JIT transform will also need to propagate this
information to the runtime somehow.
PR Close#53808
In some cases, the input files for a partial output generation
compliance tests may be invalid and lead to compilation errors.
The golden partial would be silently generated with the remaining
test cases. Instead of hiding errors, we will now print these and
cause the script to fail properly.
Note that the error logging is pretty minimalistic, but it's sufficient.
PR Close#53808
We generate `advance` instructions before most update instructions and the majority of `advance` calls are advancing by one. We can save some bytes for the most common case by omitting the parameter for `advance(1)` altogether.
PR Close#53845
Instead of computing the bit input flags at compile-time and inling
the final bit flag number, we will use the `InputFlags` enum directly.
This is a little more code in the compiler side, but will allow us to
have better debuggable development code, and also prevents problems
where runtime flag bitmasks differ from the compiler flag bitmasks.
This is in practice a noop for optimized applications as the enum values
would be inlined anyway. This matches existing compiler emit for e.g.
change detection strategy, or view encapsulation enums.
PR Close#53571
The linker compliance tests were disabled with a Babel update and
nobody realized for quite a while, via
https://github.com/angular/angular/pull/49914.
As we've came across this lost coverage, which also is quite
impactful as all libraries depend on linked output- I've took initiative
to debug the root cause as there was no follow-up.
https://github.com/angular/angular/issues/51647.
It turned out to be a highly complex issue that is non-trivial to fix,
but at least we should try to resurrect the significant portion of test
coverage by still running the linker tests- avoiding regressions, or
unexpected issues (like with defer being developed). We can work on
re-enabling and fixing source-maps separately.
Tracked via https://github.com/angular/angular/issues/51647.
PR Close#53571
The linker compliance tests did not run for a while. There were a couple
of new tests that were not passing as this wasn't flagged on CI. This commit fixes this.
Fortunately there was no problematic code that did indicate issues with linking.
In the follow-up commit, we fix the compliance test infrastructure to
re-enable linker testing..
One clear issue is still that the defer blocks are not handled properly
in linked output- hence making defer not actually "lazy" for compiled
libraries. This needs to be handled separately by the framework team.
PR Close#53571
This commit adds a final test for input signals, integrating all major
parts:
* type-checking
* compiler detection
* compiler emit
* API signature tests
PR Close#53571
This commit introduces a new enum for capturing additional metadata
about inputs. Called `InputFlags`. These will be built up at compile
time and then propagated into the runtime logic, in a way that does
not require additional lookup dictionaries data structures, or
additional memory allocations for "common inputs" that do not have any flags.
The flags will incorporate information on whether an input is signal
based. This can then be used to avoid megamorphic accesses when such
input is set- as we'd not need to check the input field value. This also
avoids cases where an input signal may be used as initial value for an
input (as we'd not incorrectly detect the input as a signal input then).
The new metadata emit will be useful for incorporating additional
metadata for inputs, such as whether they are required etc (although
required inputs are a build-time only construct right now- but this is a
good illustration of why input flags can be useful). An alternative
could have been to have an additional boolean entry for signal inputs,
but allocating a number with more flexible input flags seems more future
proof and more reasonable andreadable.
More information on the megamorphic access when updating an input
signal
https://docs.google.com/document/d/1FpnFruviKb6BFTQfMAP2AMEqEB0FI7z-3mT_qm7lzX8/edit.
PR Close#53571
In #52931, Kristiyan fixed a TemplateDefinitionBuilder bug in which derived alias variables in for loops (`$even`, `$first`, etc) were referring to the wrong level of nested `@for` block. (These variables are unique because they become inlined expressions, and are not "real" context variables.) He fixed this by appending level information to the generated alias name.
Template Pipeline actually suffered from the same bug. We fix it in a very similar way -- in particular, whenever these derived context variables are used, we make them depend on versions of `$index` and `$count` that have been suffixed with the xref of the enclosing repeater.
I have added a few more pipeline goldens, because we are not quite as clever as TDB about only generating the duplicate suffixed index and count variables when inside nested loops. This is fine, since in the long run, we want to refactor it more fundamentally.
I have also added a TODO to fix this more rigorously. In particular, it would be nice if we had proper support for shadowed variables, as well as unlimited levels of variables depending on one another.
PR Close#53662
Template pipeline previously mangled CSS property names like
`--camelCase` when used in host style bindings. Note: It still *does*
mangle these names in static style attrs, both in host bindings and on
elements. This is clearly wrong, but is consistent with what TDB does
today.
PR Close#53665
Currently compiling input transform in local mode breaks, since compiler does static analysis for the transform function, and this cannot be done in local mode if the function is imported from another compilation unit. In this fix the static analysis is ditched in local mode.
PR Close#53645
It's possible for attributes to have a namespace, we need to handle this
possiblity for both attribute instructions and attributes extracted to
the consts array.
PR Close#53646
The way we were handling ICU placeholders was not compatible with using
interpolations on attributes of elements inside the ICU. This change
refactors the handling of ICU placeholders and unifies the way
expression and tag placeholders work inside ICUs.
The new approach modifies the ingest logic to add the placeholder on to
the TextOp rather than the TextInterpolationOp. This is because, in
ICUs, we may need multiple i18n expressions created from the
interpolation expressions to roll up into the same placeholder. ICUs
essentially do the interpolation at compile time, combining the static
strings with special placeholder strings that represent the expression
values.
PR Close#53643
Consider a case when an explicit `this` read is inside a template with a context that also provides the variable name being read:
```
<ng-template let-a>{{this.a}}</ng-template>
```
Clearly, `this.a` should refer to the class property `a`. However, in today's Angular, `this.a` will refer to `let-a` on the template context.
Amazingly, both TemplateDefinitionBuilder and the Typecheck block have the same bug, and are consistent with each other! This is because `ImplicitReceiver` extends `ThisReceiver` in the parser AST, which is an insane gotcha.
In this commit, I patch the template pipeline to emulate this behavior as well.
To actually fix this nastiness, we have to:
- Update `ingest.ts` in the Template Pipeline (see the corresponding comment)
- Check `type_check_block.ts` in the Typecheck block code (see the corresponding comment)
- Turn off legacy TemplateDefinitionBuilder
- Fix g3, and release in a major version
PR Close#53594
`ng-content` elements, and thus their corresponding projection instructions, can have many attributes on them. Some of these attributes may result in special behavior. For example, `ngProjectAs` and `i18n-foo` both result in special const collection, into the approprate BindingKind slot in the const array. Additionally, `i18n-foo` needs to recieve all the additional i18n attribute processing.
We solve this by subjecting `ng-content` attributes to all the same pipeline logic that applies to attributes on elements, and then allow the element const collection phase to collect them.
PR Close#53594
For regular templates, any listener will have its name const collected into the bindings section of the element consts.
In contrast, host bindings omit listener names from their hostAttrs. This is a strange and inconsistent behavior, so we hide it behind a compatiblity mode flag.
PR Close#53594
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
