The expression `a()?.b` should expand into `(tmp = a()) === null ? null : tmp.b`, in order to avoid calling the function `a()` twice.
This commit modifies the null-safe-expansion algorithm to emit temporary assignments, and provides the reification code to actually generate the declarations, assignments, and reads.
Note also that, with our bottom-up algorithm, there are some tricky cases when a function call exists inside an indexed access, such as `f1()?.[f2()?.a]?.b`. We add some special logic to avoid generating a double-assignment to the temporary storing the result of `f2()`.
Finally, there are opportunities to reuse the same temporary in expressions like `a?.[f()]?.[f()]`. We save this for the next commit.
PR Close#50688
If a library is compiling with Angular v16.1.0, the library will break
for users that are still on Angular v16.0.x. This happens because the
`DirectiveDeclaration` or `ComponentDeclaration` types are not expecting
an extra field for `signals` metadata. This field was only added to the
generic types in `16.1.0`- so compilations fail with errors like this:
```
Error: node_modules/@angular/material/icon/index.d.ts:204:18 -
error TS2707: Generic type 'ɵɵComponentDeclaration' requires between 7 and 9 type arguments.
```
To fix this, we quickly roll back the code for inserting this metadata
field. That way, libraries remain compatible with all v16.x framework
versions.
We continue to include the `signals` metadata if `signals: true` is set.
This is not public API anyway right now- so cannot happen- but imagine
we expose some signal APIs in e.g. 16.2.x, then we'd need this metadata
and can reasonably expect signal-component library users to use a more
recent framework core version.
PR Close#50714
Angular's null-safe access operators differ from Javascript's built-in semantics, in that they short-circuit to `null` instead of `undefined`. This necessitates providing a custom transformation, instead of relying on Typescript or Javascript itseld.
The old TemplateDefinitionBuilder uses a top-down approach based on the Visitor pattern, in which it recursively extracts the left-most safe access, and hoists it to a null check at the top. See `expression_converter.ts` for details.
In this commit, we replace that approach with a new bottom-up algorithm, as part of the template pipeline. This requires an intermediate expression type to represent the not-yet-expanded ternary operators, and is split into its own pass.
Null-safe function calls are not yet implemented, since they will rely on a future temporary variable allocation pass.
Co-authored-by: Alex Rickabaugh <alxhub@users.noreply.github.com>
PR Close#50594
Refactor attribute and property binding ingestion and add an attribute extraction phase
Co-authored-by: Alex Rickabaugh <alxhub@users.noreply.github.com>
Co-authored-by: Dylan Hunn <dylhunn@users.noreply.github.com>
Only add the value to the ElementAttributes map for style and attribute kinds
Other kinds should not have their value represented in the consts array
Add missing attribute ingesiton for templates
Unify how template and element bindings are ingested
This resolves the issue of missing listener attributes on templates. In
order to avoid emitting extraneous instructions, listener ops on
templates are stripped in the attribute extraction phase instead.
Handle different binding types separately in ingest
Cleanup code and comments
Disable test that fails on new explicit error.
Previously the test was passing because ingestPropertyBinding treated
attribute bindings as normal bindings which happened to be ok for the
particular test. Now there's an explicit error that attrbiute bindings
aren't yet handled which causes the test to fail
Address feeback
PR Close#50664
Add a flag to disable specific tests when testing the template pipeline
Mark the currently failing tests
Add the template pipeline tests to CI
Update package.json
Co-authored-by: Paul Gschwendtner <paulgschwendtner@gmail.com>
PR Close#50582
Adds the necessary compiler changes to support input transform functions. The compiler output has changed in the following ways:
### Directive handler
The directive handler now extracts a reference to the input transform function and it resolves the type of its first parameter. It also asserts that the type can be referenced in the compiled output and that it doesn't clash with any pre-existing `ngAcceptInputType_` members.
### .d.ts
In the generated declaration files the compiler now inserts an `ngAcceptInputType_` member for each input with a `transform` function. The member's type corresponds to the type of the first parameter of the function, e.g.
```typescript
// foo.directive.ts
@Directive()
export class Foo {
@Input({transform: (incomingValue: string) => parseInt(incomingValue)}) value: number;
}
// foo.directive.d.ts
export class Foo {
value: number;
static ngAcceptInputType_value: string;
}
```
### Type check block
If an input has `transform` function, the TCB will use the type of its first parameter for the setter type. This uses the same infrastructure as the `ngAcceptInputType_` members.
### Directive declaration
The generated runtime directive declaration call now includes the `transform` function in the `inputs` map, if the input is being transformed. The function will be picked up by the runtime in the next commit to do the actual transformation.
```typescript
// foo.directive.ts
@Directive()
export class Foo {
@Input({transform: (incomingValue: string) => parseInt(incomingValue)}) value: number;
}
// foo.directive.js
export class Foo {
ɵdir = ɵɵdefineDirective({
inputs: {
value: ['value', 'value', incomingValue => parseInt(incomingValue)]
}
});
}
```
PR Close#50225
This commit adds the `signals: boolean` property to the internal
directive/component metadata. This does not add it to the public API
yet, as the feature has no internal support other than compiler
detection.
PR Close#49981
NgModules which import standalone components currently list those components
in their injector definitions, because we assume that any standalone
component may export providers from its own imports.
This commit adds an optimization for that emit, which attempts to statically
analyze the NgModule imports and determine which standalone components, if
any are present, do not export providers and thus can be omitted.
This analysis is imperfect, because some imported components may be declared
outside of the current compilation, or transitively import types which are
declared outside the compilation. These types are therefore _assumed_ to
carry providers and so the optimization isn't applied to them.
PR Close#49837
Adds support for marking a directive input as required. During template type checking, the compiler will verify that all required inputs have been specified and will raise a diagnostic if one or more are missing. Some specifics:
* Inputs are marked as required by passing an object literal with a `required: true` property to the `Input` decorator or into the `inputs` array.
* Required inputs imply that the directive can't work without them. This is why there's a new check that enforces that all required inputs of a host directive are exposed on the host.
* Required input diagnostics are reported through the `OutOfBandDiagnosticRecorder`, rather than generating a new structure in the TCB, because it allows us to provide a better error message.
* Currently required inputs are only supported during AOT compilation, because knowing which bindings are present during JIT can be tricky and may lead to increased bundle sizes.
Fixes#37706.
PR Close#49468
This reverts commit 13dd614cd1.
This breaks a g3 Typescript compilation tests where diagnostics are
expected for a missing input in the component.
PR Close#49467
Adds support for marking a directive input as required. During template type checking, the compiler will verify that all required inputs have been specified and will raise a diagnostic if one or more are missing. Some specifics:
* Inputs are marked as required by passing an object literal with a `required: true` property to the `Input` decorator or into the `inputs` array.
* Required inputs imply that the directive can't work without them. This is why there's a new check that enforces that all required inputs of a host directive are exposed on the host.
* Required input diagnostics are reported through the `OutOfBandDiagnosticRecorder`, rather than generating a new structure in the TCB, because it allows us to provide a better error message.
* Currently required inputs are only supported during AOT compilation, because knowing which bindings are present during JIT can be tricky and may lead to increased bundle sizes.
Fixes#37706.
PR Close#49453
This reverts commit 1a6ca68154.
This breaks tests in google3 which might be depending on private APIs. We
need to update these tests before we can land this PR.
PR Close#49449
Adds support for marking a directive input as required. During template type checking, the compiler will verify that all required inputs have been specified and will raise a diagnostic if one or more are missing. Some specifics:
* Inputs are marked as required by passing an object literal with a `required: true` property to the `Input` decorator or into the `inputs` array.
* Required inputs imply that the directive can't work without them. This is why there's a new check that enforces that all required inputs of a host directive are exposed on the host.
* Required input diagnostics are reported through the `OutOfBandDiagnosticRecorder`, rather than generating a new structure in the TCB, because it allows us to provide a better error message.
* Currently required inputs are only supported during AOT compilation, because knowing which bindings are present during JIT can be tricky and may lead to increased bundle sizes.
Fixes#37706.
PR Close#49304
Fixes that the expression converter was producing code that throws a runtime error if a non-null assertion is used as a part of a safe read, write or call.
Fixes#48742.
PR Close#48801
Allows for self-closing tags to be used for non-native tag names, e.g. `<foo [input]="bar"></foo>` can now be written as `<foo [input]="bar"/>`. Native tag names still have to have closing tags.
Fixes#39525.
PR Close#48535
This reverts commit 2d08965b1a.
The reason for revert is that we've identified some issues with implementation. The issues will get addressed soon and the fix would be re-submitted.
PR Close#47959
This commit updates the logic related to the attribute order on iframes and makes the rules more strict. There is a set of iframe attributes that may affect the behavior of an iframe, this change enforces that these attributes are applied before an `src` or `srcdoc` attributes are applied to an iframe, so that they are taken into account.
If Angular detects that some of the attributes are set after the `src` or `srcdoc`, it throws an error message, which contains the name of ann attribute that is causing the problem and the name of a Component where an iframe is located. In most cases, it should be enough to change the order of attributes in a template to move the `src` or `srcdoc` ones to the very end.
BREAKING CHANGE:
Existing iframe usages may have `src` or `srcdoc` preceding other attributes. Such usages may need to be updated to ensure compliance with the new stricter rules around iframe bindings.
PR Close#47935
This is the compile-time implementation of the `hostDirectives` feature plus a little bit of runtime code to illustrate how the newly-generated code will plug into the runtime. It works by creating a call to the new `ɵɵHostDirectivesFeature` feature whenever a directive has a `hostDirectives` field. Afterwards `ɵɵHostDirectivesFeature` will patch a new function onto the directive definition that will be invoked during directive matching.
For example, if we take the following definition:
```ts
@Directive({
hostDirectives: [HostA, {directive: HostB, inputs: ['input: alias']}]
})
class MyDir {}
```
Will compile to:
```js
MyDir.ɵdir = ɵɵdefineComponent({
features: [ɵɵHostDirectivesFeature([HostA, {
directive: HostB,
inputs: {
input: "alias"
}
}])]
});
```
The template type checking is implemented during directive matching by adding the host directives applied on the host to the array of matched directives whenever the host is matched in a template.
Relates to #8785.
PR Close#46868
Saves us some bytes by not emitting `providers` in `defineInjector`. While the amount of bytes isn't huge, I think that this change is worthwhile, because `ng generate` currently generates `providers: []` with every `NgModule` which users can forget to remove.
PR Close#46301
This commit accounts for the Babel types changes. Some properties
can now also be `undefined` so existing checks/assertions had to
be adjusted to also capture `undefined` (along with `null`).
Additionally, in preparation for a new ECMA proposal, Babel types
seem to have been updated to include private names in object property
keys. This is not necessarily the case for object expressions, but
could be for object patterns (in the future -- when implemented).
More details: https://github.com/babel/babel/pull/14304 and
https://github.com/tc39/proposal-destructuring-private.
PR Close#45967
Excludes styles that resolve to empty strings from the emitted metadata so that they don't result in empty `<style>` tags at runtime.
Fixes#31191.
PR Close#45459
In v14, the partial compilation output of components have changed in a way
that prevents older versions of Angular to compile the partial declarations
correctly.
In particular, we used to emit used directives/components in separate arrays called
`components` and `directives`, and used pipes in a property called `pipes`:
```js
TestComponent.ɵcmp = i0.ɵɵngDeclareComponent({
minVersion: "12.0.0",
version: "13.3.0",
type: TestComponent,
selector: "ng-component",
ngImport: i0,
template: ``,
isInline: true,
directives: [{ type: i1.SomeDir, selector: "[some-dir]" }],
components: [{ type: i1.SomeCmp, selector: "some-cmp" }],
pipes: { 'async': i2.AsyncPipe },
});
```
In the above example, the `version` property indicates which exact compiler
version was used to compile the component, but the `minVersion` allows older
versions of the compiler/Angular linker to "link" the partial declaration to
its final AOT compilation output.
In v14, the used directives, components and pipes are now emitted together
into a single array under the `dependencies` property:
```js
TestComponent.ɵcmp = i0.ɵɵngDeclareComponent({
minVersion: "12.0.0",
version: "13.3.0",
type: TestComponent,
selector: "ng-component",
ngImport: i0,
template: ``,
isInline: true,
dependencies: [
{ kind: "directive", type: i1.SomeDir, selector: "[some-dir]" },
{ kind: "component", type: i1.SomeCmp, selector: "some-cmp" },
{ kind: "pipe", type: i2.AsyncPipe },
],
});
```
This change has been made in support of standalone components, but it does mean
that older compiler versions can no longer link these partial declarations
as desirable as none of the components, directives and pipes would be included
in the AOT-compiled code.
By increasing the `minVersion` property, we hint to older compiler versions that
they are not capable of processing the partial declaration. This allows reporting
an error at compile time instead of resulting in runtime failures due to missing
components, directives and pipes.
PR Close#45782
Before standalone, everything that could appear in an NgModule's `imports`
was relevant to DI, and needed to be emitted in the `imports` of the
generated `InjectorDef` definition. With the introduction of standalone
types, NgModule `imports` can now contain components, directives, and pipes
which are standalone. Only standalone components need to be included in
the `imports` of the generated injector definition - directives and pipes
have no effect on DI. Having them present doesn't cause any errors in the
runtime (they're filtered out by the injector itself) but it does prevent
tree-shaking.
With this commit, the generation of `InjectorDef` now filters the `imports`
to exclude directives and pipes as much as possible. It's not _always_
possible because an expression in `imports` may pull in both a directive and
a `ModuleWithProviders` reference, and we have no way of referencing just
the MWP part of that expression. Therefore this is an optimization, not a
rule of `InjectorDef` compilation.
PR Close#45701
Previously, the NgModule handler would resolve the `imports` field as one
unit, producing an array of `Reference`s. With this refactoring, if
`imports` is a literal array, each individual element will be resolved
independently. This will allow filtering in the future at the element level,
since there will be a separate `ts.Expression` for each individual element.
PR Close#45701
This commit bundles tests for standalone components that are possible after
previous implementation commits. Most new tests are compliance tests, but
a test is also included to validate that the template type-checking system
can work with standalone components as well.
PR Close#45672
This commit propagates the `isStandalone` flag for a component, directive,
or pipe during partial compilation of a standalone declaration. This flag
allows the linker to properly process a standalone declaration that it
encounters.
PR Close#45672
Previously, the compiler tracked directives and pipes in template scopes
separately. This commit refactors the scope system to unify them into a
single data structure, disambiguated by a `kind` field.
PR Close#45672
Previously, the compiler would represent template dependencies of a
component in its component definition through separate fields (`directives`,
`pipes`).
This commit refactors the compiler/runtime interface to use a single field
(`dependencies`). The runtime component definition object still has separate
`directiveDefs` and `pipeDefs`, which are calculated from the `dependencies`
when the definition is evaluated.
This change is also reflected in partially compiled declarations. To ensure
compatibility with partially compiled code already on NPM, the linker
will still honor the old form of declaration (with separate fields).
PR Close#45672
This links back each placeholder in a message to the original Angular template span which defines its expression. This is useful for understanding where each placeholder comes from in the context of the full message.
PR Close#45606
Recent changes in `rules_nodejs` caused the test case copy file actions
to be transitioned into the `exec` configuration, resulting in much larger
file paths. These paths break on Windows with the shell argument limit, and
with the path limit, causing errors like:
```
ERROR: C:/users/circleci/ng/packages/compiler-cli/test/compliance/test_cases/BUILD.bazel:9:12: Copying file packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/value_composition/structural_directives_if_directive_def.js failed: (Exit 1): cmd.exe failed: error executing command
cd /d C:/users/circleci/_bazel_circleci/u4uoan2j/execroot/angular
SET PATH=C:\Program Files\Git\usr\bin;C:\Program Files\Git\bin;C:\Windows;C:\Windows\System32;C:\Windows\System32\WindowsPowerShell\v1.0
SET RUNFILES_MANIFEST_ONLY=1
cmd.exe /C bazel-out\x64_windows-opt-exec-2B5CBBC6\bin\packages\compiler-cli\test\compliance\test_cases\test_cases--1973427149-cmd.bat
The system cannot find the path specified
```
https://app.circleci.com/pipelines/github/angular/angular/44038/workflows/4b530cb2-f232-4e1d-b35a-e6e085151d08/jobs/1140017
PR Close#45431
When we have an event listener inside an embedded view, we generate a `restoreView` call which saves the view inside of the LFrame. The problem is that we don't clear it until it gets overwritten which can lead to memory leaks.
These changes rework the generated code in order to generate a `resetView` call which will clear the view from the LFrame.
Fixes#42848.
PR Close#43075
Angular contains an NgModule registry, which allows a user to declare
NgModules with string ids and retrieve them via those ids, using the
`getNgModuleById` API.
Previously, we attempted to structure this registration in a clever fashion
to allow for tree-shaking of registered NgModules (that is, those with ids).
This sort of worked due to the accidental alignment of behaviors from the
different tree-shakers involved. However, this trick relies on the
generation of `.ngfactory` files and how they're specifically processed in
various bundling scenarios. We intend to remove `.ngfactory` files, hence
we can no longer rely on them in this way.
The correct solution here is to recognize that `@NgModule({id})` is
inherently declaring a global side-effect, and such classes should not
really be eligible for tree-shaking in the first place. This commit removes
all the old registration machinery, and standardizes on generating a side-
effectful call to `registerNgModuleType` for NgModules that have ids.
There is some risk here that NgModules with unnecessary `id`s may not
tree-shake as a result of this change, whereas they would have in previous
circumstances. The fix here should be to remove the `id` if it's not needed.
Specifying an `id` is a request that the NgModule be retained regardless of
any other references, in case it is later looked up by string id.
PR Close#45024
The `@angular/localize` package depends on a version of Babel that is two years
old, so this commit updates to the latest version.
Some changes were made to the linker and compliance tests to account for slight
changes in source maps, along with a few code updates because of changes to
the typings of Babel.
PR Close#44931
In templates with several levels of nested nodes, it's common for several `elementStart`/`elementEnd` instructions to show up in a row which can be optimized away.
These changes add chaining support for `elementStart`, `elementEnd`, `elementContainerStart` and `elementContainerEnd` to shave off some bytes when possible.
PR Close#44994
Refs http://b/214103351.
This happens if a user writes `<span i18n>Message</span>`. This is accepted as an internationalized message, but without a description. JSCompiler will throw an error in this situation because descriptions are generally required. Now, the Angular compiler will generate a suppression annotation so JSCompiler allows the syntax. This will ease an internal migration to JSCompiler-based i18n.
PR Close#44787
An `ng-template` with an inline template (i.e. has a structural
directive) would previously not get an `undefined` `tagName` because the
logic assumed the element would be `t.Element` or `t.Content` and read
the tag name from the `name` property. For a `t.Template`, this exists
instead on the `t.tagName`. The final result would be an `tagName` of `undefined`
for the parent `t.Template`, causing failures in the indexer downstream.
This `undefined` value is actually expected in the renderer code, even
though the type does not specify this possibility. This change updates
the type of `tagName` to be `string|null` and explicitly handles the
case where there is a structural directive on an `ng-template`. You can
see how the two are differentiated in the compliance code that was
modified in this commit.
PR Close#44788
When a partially compiled component or directive is "linked" in JIT mode, the body
of its declaration is evaluated by the JavaScript runtime. If a class is referenced
in a query (e.g. `ViewQuery` or `ContentQuery`) but its definition is later in the
file, then the reference must be wrapped in a `forwardRef()` call.
Previously, query predicates were not wrapped correctly in partial declarations
causing the code to crash at runtime. In AOT mode, this code is never evaluated
but instead transformed as part of the build, so this bug did not become apparent
until Angular Material started running JIT mode tests on its distributable output.
This change fixes this problem by noting when queries are wrapped in `forwardRef()`
calls and ensuring that this gets passed through to partial compilation declarations
and then suitably stripped during linking.
See https://github.com/angular/components/pull/23882 and https://github.com/angular/components/issues/23907
PR Close#44113
When a safe method call such as `person?.getName()` is used, the
compiler would generate invalid code if the argument list also contained
a safe method call. For example, the following code:
```
person?.getName(config?.get('title').enabled)
```
would generate
```
let tmp;
ctx.person == null ? null : ctx.person.getName((tmp = tmp) == null ?
null : tmp.enabled)
```
Notice how the call to `config.get('title')` has completely disappeared,
with `(tmp = tmp)` having taken its place.
The issue occurred due to how the argument list would be converted
from expression AST to output AST twice. First, the outer safe method
call would first convert its arguments list. This resulted in a
temporary being allocated for `config.get('title')`, which was stored in
the internal `_resultMap`. Only after the argument list has been
converted would the outer safe method call realize that it should be
guarded by a safe access of `person`, entering the `convertSafeAccess`
procedure to convert itself. This would convert the argument list once
again, but this time the `_resultMap` would already contain the
temporary `tmp` for `config?.get('title')`. Consequently, the safe
method in the argument list would be emitted as `tmp`.
This commit fixes the issue by ensuring that nodes are only converted
once.
Closes#44069
PR Close#44088
Dylan Hunn