This commit introduces caching of `Symbol`s produced by the template type-
checking infrastructure, in the same way that autocompletion results are
now cached.
PR Close#39278
This commit refactors the previously introduced `getGlobalCompletions()` API
for the template type-checker in a couple ways:
* The return type is adjusted to use a `Map` instead of an array, and
separate out the component context completion position. This allows for a
cleaner integration in the language service.
* A new `CompletionEngine` class is introduced which powers autocompletion
for a single component, and can cache completion results.
* The `CompletionEngine` for each component is itself cached on the
`TemplateTypeCheckerImpl` and is invalidated when the component template
is overridden or reset.
This refactoring simplifies the `TemplateTypeCheckerImpl` class by
extracting the autocompletion logic, enables caching for better performance,
and prepares for the introduction of other autocompletion APIs.
PR Close#39278
The compiler uses a `Reference` abstraction to refer to TS nodes
that it needs to refer to from other parts of the source. Such
references keep track of any identifiers that represent the referenced
node.
Prior to this commit, the compiler (and specifically `ReferenceEmitter`
classes) assumed that the reference identifiers are always free standing.
In other words a reference identifier would be an expression like
`FooDirective` in the expression `class FooDirective {}`.
But in UMD/CommonJS source, a reference can actually refer to an "exports"
declaration of the form `exports.FooDirective = ...`.
In such cases the `FooDirective` identifier is not free-standing
since it is part of a property access, so the `ReferenceEmitter`
should take this into account when emitting an expression that
refers to such a `Reference`.
This commit changes the `LocalIdentifierStrategy` reference emitter
so that if the `node` being referenced is not a declaration itself and
is in the current file, then it should be used directly, rather than
trying to use one of its identifiers.
PR Close#39346
Previously, UMD/CommonJS class inline declarations of the form:
```ts
exports.Foo = (function() { function Foo(); return Foo; })();
```
were capturing the whole IIFE as the implementation, rather than
the inner class (i.e. `function Foo() {}` in this case). This caused
the interpreter to break when it was trying to access such an export,
since it would try to evaluate the IIFE rather than treating it as a class
declaration.
PR Close#39346
group together similar error messages as part of error code efforts
ProviderNotFound & NodeInjector grouped into throwProviderNotFoundError
Cyclic dependency errors grouped into throwCyclicDependencyError
PR Close#39251
This commit adds the basic building blocks for linking partial declarations.
In particular it provides a generic `FileLinker` class that delegates to
a set of (not yet implemented) `PartialLinker` classes.
The Babel plugin makes use of this `FileLinker` providing concrete classes
for `AstHost` and `AstFactory` that work with Babel AST. It can be created
with the following code:
```ts
const plugin = createEs2015LinkerPlugin({ /* options */ });
```
PR Close#39116
Previously, inline exports of the form `exports.foo = <implementation>;` were
being interpreted (by the ngtsc `PartialInterpeter`) as `Reference` objects.
This is not what is desired since it prevents the value of the export
from being unpacked, such as when analyzing `NgModule` declarations:
```
exports.directives = [Directive1, Directive2];
@NgImport({declarations: [exports.directives]})
class AppModule {}
```
In this example the interpreter would think that `exports.directives`
was a reference rather than an array that needs to be unpacked.
This bug was picked up by the ngcc-validation repository. See
https://github.com/angular/ngcc-validation/pull/1990 and
https://circleci.com/gh/angular/ngcc-validation/17130
PR Close#39267
Some inline declarations are of the form:
```
exports.<name> = <implementation>;
```
In this case the declaration `node` is `exports.<name>`.
When interpreting such inline declarations we actually want
to visit the `implementation` expression rather than visiting
the declaration `node`.
This commit adds `implementation?: ts.Expression` to the
`InlineDeclaration` type and updates the interpreter to visit
these expressions as described above.
PR Close#39267
When ngcc is configured to run with the `--use-program-dependencies`
flag, as is the case in the CLI's asynchronous processing, it will scan
all source files in the program, starting from the program's root files
as configured in the tsconfig. Each individual root file could
potentially rescan files that had already been scanned for an earlier
root file, causing a severe performance penalty if the number of root
files is large. This would be the case if glob patterns are used in the
"include" specification of a tsconfig file.
This commit avoids the performance penalty by keeping track of the files
that have been scanned across all root files, such that no source file
is scanned multiple times.
Fixes#39240
PR Close#39254
Previously the `node.name` property was only checked to ensure it was
defined. But that meant that it was a `ts.BindingName`, which also includes
`ts.BindingPattern`, which we do not support. But these helper methods were
forcefully casting the value to `ts.Identifier.
Now we also check that the `node.name` is actually an `ts.Identifier`.
PR Close#38959
Previously directive "queries" that relied upon a namespaced type
```ts
queries: {
'mcontent': new core.ContentChild('test2'),
}
```
caused an error to be thrown. This is now supported.
PR Close#38959
Previously, any declarations that were defined "inline" were not
recognised by the `UmdReflectionHost`.
For example, the following syntax was completely unrecognized:
```ts
var Foo_1;
exports.Foo = Foo_1 = (function() {
function Foo() {}
return Foo;
})();
exports.Foo = Foo_1 = __decorate(SomeDecorator, Foo);
```
Such inline classes were ignored and not processed by ngcc.
This lack of processing led to failures in Ivy applications that relied
on UMD formats of libraries such as `syncfusion/ej2-angular-ui-components`.
Now all known inline UMD exports are recognized and processed accordingly.
Fixes#38947
PR Close#38959
Previously these tests were checking multiple specific expression
types. The new helper function is more general and will also support
`PropertyAccessExpression` nodes for `InlineDeclaration` types.
PR Close#38959
Previously the `ConcreteDeclaration` and `InlineDeclaration` had
different properties for the underlying node type. And the `InlineDeclaration`
did not store a value that represented its declaration.
It turns out that a natural declaration node for an inline type is the
expression. For example in UMD/CommonJS this would be the `exports.<name>`
property access node.
So this expression is now used for the `node` of `InlineDeclaration` types
and the `expression` property is dropped.
To support this the codebase has been refactored to use a new `DeclarationNode`
type which is a union of `ts.Declaration|ts.Expression` instead of `ts.Declaration`
throughout.
PR Close#38959
This makes these tests more resilient to changes in the test code
structure. For example switching from
```
var SomeClass = <implementation>;
exports.SomeClass = SomeClass;
```
to
```
exports.SomeClass = <implementation>;
```
PR Close#38959
Previously `getDeclaration()` would only return the first node that matched
the name passed in and then assert the predicate on this single node.
It also only considered a subset of possible declaration types that we might
care about.
Now the function will parse the whole tree collecting an array of all the
nodes that match the name. It then filters this array based on the predicate
and only errors if the filtered array is empty.
This makes this function much more resilient to more esoteric code formats
such as UMD.
PR Close#38959
The new function does not try to restrict the kind of AST node that it
finds, leaving that to the caller. This will make it more resuable in the
UMD reflection host.
PR Close#38959
Sometimes UMD exports appear in the following form:
```
exports.MyClass = alias1 = alias2 = <<declaration>>
```
Previously the declaration of the export would have been captured
as `alias1 = alias2 = <<declaration>>`, which the `PartialInterpreter`
would have failed on, since it cannot handle assignments.
Now we skip over these aliases capturing only the `<<declaration>>`
expression.
Fixes#38947
PR Close#38959
UMD files export values by assigning them to an `exports` variable.
When evaluating expressions ngcc was failing to cope with expressions
like `exports.MyComponent`.
This commit fixes the `UmdReflectionHost.getDeclarationOfIdentifier()`
method to map the `exports` variable to the current source file.
PR Close#38959
The `SIMPLE_CLASS_FILE` contained a `ChildClass` that had an
internal aliases implementation and extended a `SuperClass` base
class. The call to `__extends` was using the wrong argument for
the child class.
PR Close#38959
This clarifies that this is specifically about statements of the form
`exports.<name> = <declaration>`, rather than a general export
statement such as `export class <ClassName> { ... }`.
PR Close#38959
There is no need to check that the `ref.node` is of any particular type
because immediately after this check the entry is tested to see if it passes
`isClassDeclarationReference()`.
The only difference is that the error that is reported is slightly different
in the case that it is a `ref` but not one of the TS node types.
Previously:
```
`Value at position ${idx} in the NgModule.${arrayName} of ${
className} is not a reference`
```
now
```
`Value at position ${idx} in the NgModule.${arrayName} of ${
className} is not a class`
```
Arguably the previous message was wrong, since this entry IS a reference
but is not a class.
PR Close#38959
Removes `ViewEncapsulation.Native` which has been deprecated for several major versions.
BREAKING CHANGES:
* `ViewEncapsulation.Native` has been removed. Use `ViewEncapsulation.ShadowDom` instead. Existing
usages will be updated automatically by `ng update`.
PR Close#38882
Expressions within ICU expressions in templates were not previously
type-checked, as they were skipped while traversing the elements
within a template. This commit enables type checking of these
expressions by actually visiting the expressions.
BREAKING CHANGE:
Expressions within ICUs are now type-checked again, fixing a regression
in Ivy. This may cause compilation failures if errors are found in
expressions that appear within an ICU. Please correct these expressions
to resolve the type-check errors.
Fixes#39064
PR Close#39072
Updates to rules_nodejs 2.2.0. This is the first major release in 7 months and includes a number of features as well
as breaking changes.
Release notes: https://github.com/bazelbuild/rules_nodejs/releases/tag/2.0.0
Features of note for angular/angular:
* stdout/stderr/exit code capture; this could be potentially be useful
* TypeScript (ts_project); a simpler tsc rule that ts_library that can be used in the repo where ts_library is too
heavy weight
Breaking changes of note for angular/angular:
* loading custom rules from npm packages: `ts_library` is no longer loaded from `@npm_bazel_typescript//:index.bzl`
(which no longer exists) but is now loaded from `@npm//@bazel/typescript:index.bzl`
* with the loading changes above, `load("@npm//:install_bazel_dependencies.bzl", "install_bazel_dependencies")` is
no longer needed in the WORKSPACE which also means that yarn_install does not need to run unless building/testing
a target that depends on @npm. In angular/angular this is a minor improvement as almost everything depends on @npm.
* @angular/bazel package is also updated in this PR to support the new load location; Angular + Bazel users that
require it for ng_package (ng_module is no longer needed in OSS with Angular 10) will need to load from
`@npm//@angular/bazel:index.bzl`. I investigated if it was possible to maintain backward compatability for the old
load location `@npm_angular_bazel` but it is not since the package itself needs to be updated to load from
`@npm//@bazel/typescript:index.bzl` instead of `@npm_bazel_typescript//:index.bzl` as it depends on ts_library
internals for ng_module.
* runfiles.resolve will now throw instead of returning undefined to match behavior of node require
Other changes in angular/angular:
* integration/bazel has been updated to use both ng_module and ts_libary with use_angular_plugin=true.
The latter is the recommended way for rules_nodejs users to compile Angular 10 with Ivy. Bazel + Angular ViewEngine is
supported with @angular/bazel <= 9.0.5 and Angular <= 8. There is still Angular ViewEngine example on rules_nodejs
https://github.com/bazelbuild/rules_nodejs/tree/stable/examples/angular_view_engine on these older versions but users
that want to update to Angular 10 and are on Bazel must switch to Ivy and at that point ts_library with
use_angular_plugin=true is more performant that ng_module. Angular example in rules_nodejs is configured this way
as well: https://github.com/bazelbuild/rules_nodejs/tree/stable/examples/angular. As an aside, we also have an
example of building Angular 10 with architect() rule directly instead of using ts_library with angular plugin:
https://github.com/bazelbuild/rules_nodejs/tree/stable/examples/angular_bazel_architect.
NB: ng_module is still required for angular/angular repository as it still builds ViewEngine & @angular/bazel
also provides the ng_package rule. ng_module can be removed in the future if ViewEngine is no longer needed in
angular repo.
* JSModuleInfo provider added to ng_module. this is for forward compat for future rules_nodejs versions.
PR Close#39182
Temporarily disable the //packages/compiler-cli/integrationtest:integrationtest
target while continuing to investigate its unknown failures
PR Close#39168
The right needs to be wrapped in parens or we cannot accurately match its
span to just the RHS. For example, the span in `e = $event /*0,10*/` is ambiguous.
It could refer to either the whole binary expression or just the RHS.
We should instead generate `e = ($event /*0,10*/)` so we know the span 0,10 matches RHS.
This is specifically needed for the TemplateTypeChecker/Language Service
when mapping template positions to items in the TCB.
PR Close#39143
This commit introduces a new API for the `TemplateTypeChecker` which allows
for autocompletion in a global expression context (for example, in a new
interpolation expression such as `{{|}}`). This API returns instances of the
type `GlobalCompletion`, which can represent either a completion result from
the template's component context or a declaration such as a local reference
or template variable. The Language Service will use this API to implement
autocompletion within templates.
PR Close#39048
Previously the value passed to `AstFactory.attachComments()` could be
`undefined` which is counterintuitive, since why attach something that
doesn't exist? Now it expects there to be a defined array. Further it no
longer returns a statement. Both these aspects of the interface were designed
to make the usage simpler but has the result of complicating the implemenation.
The `ExpressionTranslatorVisitor` now has a helper function (`attachComments()`)
to handle `leadingComments` being undefined and also returning the statement.
This keeps the usage in the translator simple, while ensuring that the `AstFactory`
API is not influenced by how it is used.
PR Close#39076
This is needed so that the Language Service can provide the module name
in the quick info for a directive/component.
To accomplish this, the compiler's `LocalModuleScope` is provided to the
`TemplateTypeCheckerImpl`. This will also allow the `TemplateTypeChecker` to
provide more completions in the future, giving it a way to determine all the
directives/pipes/etc. available to a template.
PR Close#39099
The compiler maintains an internal dependency graph of all resource
dependencies for application source files. This information can be useful
for tools that integrate the compiler and need to support file watching.
This change adds a `getResourceDependencies` method to the
`NgCompiler` class that allows compiler integrations to access resource
dependencies of files within the compilation.
PR Close#38048
Updates to rules_nodejs 2.2.0. This is the first major release in 7 months and includes a number of features as well
as breaking changes.
Release notes: https://github.com/bazelbuild/rules_nodejs/releases/tag/2.0.0
Features of note for angular/angular:
* stdout/stderr/exit code capture; this could be potentially be useful
* TypeScript (ts_project); a simpler tsc rule that ts_library that can be used in the repo where ts_library is too
heavy weight
Breaking changes of note for angular/angular:
* loading custom rules from npm packages: `ts_library` is no longer loaded from `@npm_bazel_typescript//:index.bzl`
(which no longer exists) but is now loaded from `@npm//@bazel/typescript:index.bzl`
* with the loading changes above, `load("@npm//:install_bazel_dependencies.bzl", "install_bazel_dependencies")` is
no longer needed in the WORKSPACE which also means that yarn_install does not need to run unless building/testing
a target that depends on @npm. In angular/angular this is a minor improvement as almost everything depends on @npm.
* @angular/bazel package is also updated in this PR to support the new load location; Angular + Bazel users that
require it for ng_package (ng_module is no longer needed in OSS with Angular 10) will need to load from
`@npm//@angular/bazel:index.bzl`. I investigated if it was possible to maintain backward compatability for the old
load location `@npm_angular_bazel` but it is not since the package itself needs to be updated to load from
`@npm//@bazel/typescript:index.bzl` instead of `@npm_bazel_typescript//:index.bzl` as it depends on ts_library
internals for ng_module.
* runfiles.resolve will now throw instead of returning undefined to match behavior of node require
Other changes in angular/angular:
* integration/bazel has been updated to use both ng_module and ts_libary with use_angular_plugin=true.
The latter is the recommended way for rules_nodejs users to compile Angular 10 with Ivy. Bazel + Angular ViewEngine is
supported with @angular/bazel <= 9.0.5 and Angular <= 8. There is still Angular ViewEngine example on rules_nodejs
https://github.com/bazelbuild/rules_nodejs/tree/stable/examples/angular_view_engine on these older versions but users
that want to update to Angular 10 and are on Bazel must switch to Ivy and at that point ts_library with
use_angular_plugin=true is more performant that ng_module. Angular example in rules_nodejs is configured this way
as well: https://github.com/bazelbuild/rules_nodejs/tree/stable/examples/angular. As an aside, we also have an
example of building Angular 10 with architect() rule directly instead of using ts_library with angular plugin:
https://github.com/bazelbuild/rules_nodejs/tree/stable/examples/angular_bazel_architect.
NB: ng_module is still required for angular/angular repository as it still builds ViewEngine & @angular/bazel
also provides the ng_package rule. ng_module can be removed in the future if ViewEngine is no longer needed in
angular repo.
* JSModuleInfo provider added to ng_module. this is for forward compat for future rules_nodejs versions.
@josephperrott, this touches `packages/bazel/src/external.bzl` which will make the sync to g3 non-trivial.
PR Close#37727
This commit adds the `AstHost` interface, along with implementations for
both Babel and TS.
It also implements the Babel vesion of the `AstFactory` interface, along
with a linker specific implementation of the `ImportGenerator` interface.
These classes will be used by the new "ng-linker" to transform prelinked
library code using a Babel plugin.
PR Close#38866
The `AstFactory.createFunctionDeclaration()` was allowing `null` to be
passed as the function `name` value. This is not actually possible, since
function declarations must always have a name.
PR Close#38866
The tests were assuming that newlines were `\n` characters but this is not
the case on Windows. This was fixed in #38925, but a better solution is to
configure the TS printer to always use `\n` characters for newlines.
PR Close#38866
These free standing functions rely upon the "current" `FileSystem`,
but it is safer to explicitly pass the `FileSystem` into functions or
classes that need it.
PR Close#39006
To verify the correctness of the linker output, we leverage the existing
compliance tests. The plan is to test the linker by running all compliance
tests using a full round trip of pre-linking and subsequently post-linking,
where the generated code should be identical to a full AOT compile.
This commit adds an additional Bazel target that runs the compliance
tests in partial mode. Follow-up work is required to implement the logic
for running the linker round trip.
PR Close#38938
This is a precursor to introducing the Angular linker. As an initial
step, a compiler option to configure the compilation mode is introduced.
This option is initially internal until the linker is considered ready.
PR Close#38938
* Add `templateNode` to `ElementSymbol` and `TemplateSymbol` so callers
can use the information about the attributes on the
`TmplAstElement`/`TmplAstTemplate` for directive matching
* Remove helper function `getSymbolOfVariableDeclaration` and favor
more specific handling for scenarios. The generic function did not
easily handle different scenarios for all types of variable declarations
in the TCB
PR Close#39047
This commit adds an API to `NgCompiler`, a method called
`getComponentsWithTemplateFile`. Given a filesystem path to an external
template file, it retrieves a `Set` (actually a `ReadonlySet`) of component
declarations which are using this template. In most cases, this will only be
a single component.
This information is easily determined by the compiler during analysis, but
is hard for a lot of Angular tooling (e.g. the language service) to infer
independently. Therefore, it makes sense to expose this as a compiler API.
PR Close#39002
With the introduction of incremental type checking in #36211, an
intermediate `ts.Program` for type checking is only created if there are
any templates to check. This rendered some tests ineffective at avoiding
regressions, as the intermediate `ts.Program` was required for the tests
to fail if the scenario under test would not be accounted for. This
commit adds a single component to these tests, to ensure the
intermediate `ts.Program` is in fact created.
PR Close#39011
Prior to this fix, incremental rebuilds could fail to type check due to
missing ambient types from auto-discovered declaration files in @types
directories, or type roots in general. This was caused by the
intermediary `ts.Program` that is created for template type checking,
for which a `ts.CompilerHost` was used which did not implement the
optional `directoryExists` methods. As a result, auto-discovery of types
would not be working correctly, and this would retain into the
`ts.Program` that would be created for an incremental rebuild.
This commit fixes the issue by forcing the custom `ts.CompilerHost` used
for type checking to properly delegate into the original
`ts.CompilerHost`, even for optional methods. This is accomplished using
a base class `DelegatingCompilerHost` which is typed in such a way that
newly introduced `ts.CompilerHost` methods must be accounted for.
Fixes#38979
PR Close#39011
We weren't resolving a path correctly which resulted in an error on Windows.
For reference, here's the error. Note the extra slash before `C:`:
```
Error: ENOENT: no such file or directory, scandir '/C:/bazel_output_root/yxvwd24o/external/npm/node_modules/typescript'
at Object.readdirSync (fs.js:854:3)
```
PR Close#39005
This commit updates the symbols in the TemplateTypeCheck API and methods
for retrieving them:
* Include `isComponent` and `selector` for directives so callers can determine which
attributes on an element map to the matched directives.
* Add a new `TextAttributeSymbol` and return this when requesting a symbol for a `TextAttribute`.
* When requesting a symbol for `PropertyWrite` and `MethodCall`, use the
`nameSpan` to retrieve symbols.
* Add fix to retrieve generic directives attached to elements/templates.
PR Close#38844
Prior to this change, each invocation of `loadStandardTestFiles` would
load the necessary files from disk. This function is typically called
at the top-level of a test module in order to share the result across
tests. The `//packages/compiler-cli/test/ngtsc` target has 8 modules
where this call occurs, each loading their own copy of
`node_modules/typescript` which is ~60MB in size, so the memory overhead
used to be significant. This commit loads the individual packages into
a standalone `Folder` and mounts this folder into the filesystem of
standard test files, such that all file contents are no longer
duplicated in memory.
PR Close#38909
Some compiler tests take a long time to run, even using multiple
executors. A profiling session revealed that most time is spent in
parsing source files, especially the default libraries are expensive to
parse.
The default library files are constant across all tests, so this commit
introduces a shared cache of parsed source files of the default
libraries. This achieves a significant improvement for several targets
on my machine:
//packages/compiler-cli/test/compliance: from 23s to 5s.
//packages/compiler-cli/test/ngtsc: from 115s to 11s.
Note that the number of shards for the compliance tests has been halved,
as the extra shards no longer provide any speedup.
PR Close#38909
In Ivy, template type-checking has 3 modes: basic, full, and strict. The
primary difference between basic and full modes is that basic mode only
checks the top-level template, whereas full mode descends into nested
templates (embedded views like ngIfs and ngFors). Ivy applies this approach
to all of its template type-checking, including the DOM schema checks which
validate whether an element is a valid component/directive or not.
View Engine has both the basic and the full mode, with the same distinction.
However in View Engine, DOM schema checks happen for the full template even
in the basic mode.
Ivy's behavior here is technically a "fix" as it does not make sense for
some checks to apply to the full template and others only to the top-level
view. However, since g3 relies exclusively on the basic mode of checking and
developers there are used to DOM checks applying throughout their template,
this commit re-enables the nested schema checks even in basic mode only in
g3. This is done by enabling the checks only when Closure Compiler
annotations are requested.
Outside of g3, it's recommended that applications use at least the full mode
of checking (controlled by the `fullTemplateTypeCheck` flag), and ideally
the strict mode (`strictTemplates`).
PR Close#38943
This commit refactors the `ExpressionTranslatorVisitor` so that it
is not tied directly to the TypeScript AST. Instead it uses generic
`TExpression` and `TStatement` types that are then converted
to concrete types by the `TypeScriptAstFactory`.
This paves the way for a `BabelAstFactory` that can be used to
generate Babel AST nodes instead of TypeScript, which will be
part of the new linker tool.
PR Close#38775
Previously each identifier was being imported individually, which made for a
very long import statement, but also obscurred, in the code, which identifiers
came from the compiler.
PR Close#38775
This file contains a number of classes making it long and hard to work with.
This commit splits the `ImportManager`, `Context` and `TypeTranslatorVisitor`
classes, along with associated functions and types into their own files.
PR Close#38775
When the target of the compiler is ES2015 or newer then we should
be generating `let` and `const` variable declarations rather than `var`.
PR Close#38775
The cast to `ts.Identifier` was a hack that "just happened to work".
The new approach is more robust and doesn't have to undermine
the type checker.
PR Close#38775
This commit re-enables some tests that were temporarily disabled on Windows,
as they failed on native Windows CI. The Windows filesystem emulation has
been corrected in an earlier commit, such that the original failure would
now also occur during emulation on Linux CI.
PR Close#37782
In native windows, the drive letter is a capital letter, while our Windows
filesystem emulation would use lowercase drive letters. This difference may
introduce tests to behave differently in native Windows versus emulated
Windows, potentially causing unexpected CI failures on Windows CI after a PR
has been merged.
Resolves FW-2267
PR Close#37782
The logic for computing identifiers, specifically for bound attributes
can be simplified by using the value span of the binding rather than the
source span.
PR Close#38899
In #38666 we changed how ngcc deals with type expressions, where it
would now always emit the original type expression into the generated
code as a "local" type value reference instead of synthesizing new
imports using an "imported" type value reference. This was done as a fix
to properly deal with renamed symbols, however it turns out that the
compiler has special handling for certain imported symbols, e.g.
`ChangeDetectorRef` from `@angular/core`. The "local" type value
reference prevented this special logic from being hit, resulting in
incorrect compilation of pipe factories.
This commit fixes the issue by manually inspecting the import of the
type expression, in order to return an "imported" type value reference.
By manually inspecting the import we continue to handle renamed symbols.
Fixes#38883
PR Close#38892
Common AST formats such as TS and Babel do not use a separate
node for comments, but instead attach comments to other AST nodes.
Previously this was worked around in TS by creating a `NotEmittedStatement`
AST node to attach the comment to. But Babel does not have this facility,
so it will not be a viable approach for the linker.
This commit refactors the output AST, to remove the `CommentStmt` and
`JSDocCommentStmt` nodes. Instead statements have a collection of
`leadingComments` that are rendered/attached to the final AST nodes
when being translated or printed.
PR Close#38811
This change prevents comments from a resolved node from appearing at
each location the resolved expression is used and also prevents callers
of `Scope#resolve` from accidentally modifying / adding comments to the
declaration site.
PR Close#38857
In the integration test suite of ngcc, we load a set of files from
`node_modules` into memory. This includes the `typescript` package and
`@angular` scoped packages, which account for a large number of large
files that needs to be loaded from disk. This commit moves this work
to the top-level, such that it doesn't have to be repeated in all tests.
PR Close#38840
Recent optimizations to ngcc have significantly reduced the total time
it takes to process `node_modules`, to such extend that sharding across
multiple processes has become less effective. Previously, running
ngcc asynchronously would allow for up to 8 workers to be allocated,
however these workers have to repeat work that could otherwise be shared.
Because ngcc is now able to reuse more shared computations, the overhead
of multiple workers is increased and therefore becomes less effective.
As an additional benefit, having fewer workers requires less memory and
less startup time.
To give an idea, using the following test setup:
```bash
npx @angular/cli new perf-test
cd perf-test
yarn ng add @angular/material
./node_modules/.bin/ngcc --properties es2015 module main \
--first-only --create-ivy-entry-points
```
We observe the following figures on CI:
| | 10.1.1 | PR #38840 |
| ----------------- | --------- | --------- |
| Sync | 85s | 25s |
| Async (8 workers) | 22s | 16s |
| Async (4 workers) | - | 11s |
In addition to changing the default number of workers, ngcc will now
use the environment variable `NGCC_MAX_WORKERS` that may be configured
to either reduce or increase the number of workers.
PR Close#38840
ngcc creates typically two `ts.Program` instances for each entry-point,
one for processing sources and another one for processing the typings.
The creation of these programs is somewhat expensive, as it concerns
module resolution and parsing of source files.
This commit implements several layers of caching to optimize the
creation of programs:
1. A shared module resolution cache across all entry-points within a
single invocation of ngcc. Both the sources and typings program
benefit from this cache.
2. Sharing the parsed `ts.SourceFile` for a single entry-point between
the sources and typings program.
3. Sharing parsed `ts.SourceFile`s of TypeScript's default libraries
across all entry-points within a single invocation. Some of these
default library typings are large and therefore expensive to parse,
so sharing the parsed source files across all entry-points offers
a significant performance improvement.
Using a bare CLI app created using `ng new` + `ng add @angular/material`,
the above changes offer a 3-4x improvement in ngcc's processing time
when running synchronously and ~2x improvement for asynchronous runs.
PR Close#38840
When type-checking a component, the declaring NgModule scope is used
to create a directive matcher that contains flattened directive metadata,
i.e. the metadata of a directive and its base classes. This computation
is done for all components, whereas the type-check scope is constant per
NgModule. Additionally, the flattening of metadata is constant per
directive instance so doesn't necessarily have to be recomputed for
each component.
This commit introduces a `TypeCheckScopes` class that is responsible
for flattening directives and computing the scope per NgModule. It
caches the computed results as appropriate to avoid repeated computation.
PR Close#38539
For the compilation of a component, the compiler has to prepare some
information about the directives and pipes that are used in the template.
This information includes an expression for directives/pipes, for usage
within the compilation output. For large NgModule compilation scopes
this has shown to introduce a performance hotspot, as the generation of
expressions is quite expensive. This commit reduces the performance
overhead by only generating expressions for the directives/pipes that
are actually used within the template, significantly cutting down on
the compiler's resolve phase.
PR Close#38539
Adds `TemplateTypeChecker` operation to retrieve the `Symbol` of a
`TmplAstVariable` or `TmplAstReference` in a template.
Sometimes we need to traverse an intermediate variable declaration to arrive at
the correct `ts.Symbol`. For example, loop variables are declared using an intermediate:
```
<div *ngFor="let user of users">
{{user.name}}
</div>
```
Getting the symbol of user here (from the expression) is tricky, because the TCB looks like:
```
var _t0 = ...; // type of NgForOf
var _t1: any; // context of embedded view for NgForOf structural directive
if (NgForOf.ngTemplateContextGuard(_t0, _t1)) {
// _t1 is now NgForOfContext<...>
var _t2 = _t1.$implicit; // let user = '$implicit'
_t2.name; // user.name expression
}
```
Just getting the `ts.Expression` for the `AST` node `PropRead(ImplicitReceiver, 'user')`
via the sourcemaps will yield the `_t2` expression. This function recognizes that `_t2`
is a variable declared locally in the TCB, and actually fetch the `ts.Symbol` of its initializer.
These special handlings show the versatility of the `Symbol`
interface defined in the API. With this, when we encounter a template variable,
we can provide the declaration node, as well as specific information
about the variable instance, such as the `ts.Type` and `ts.Symbol`.
PR Close#38618
Adds support to the `TemplateTypeChecker` to get a `Symbol` of an AST
expression in a component template.
Not all expressions will have `ts.Symbol`s (e.g. there is no `ts.Symbol`
associated with the expression `a + b`, but there are for both the a and b
nodes individually).
PR Close#38618
Adds support to the `TemplateTypeChecker` for retrieving a `Symbol` for
`TmplAstTemplate` and `TmplAstElement` nodes in a component template.
PR Close#38618
Specifically, this commit adds support for retrieving a `Symbol` from a
`TmplAstBoundEvent` or `TmplAstBoundAttribute`. Other template nodes
will be supported in following commits.
PR Close#38618
The statements generated in the TCB are optimized for performance and producing diagnostics.
These optimizations can result in generating a TCB that does not have all the information
needed by the `TemplateTypeChecker` for retrieving `Symbol`s. For example, as an optimization,
the TCB will not generate variable declaration statements for directives that have no
references, inputs, or outputs. However, the `TemplateTypeChecker` always needs these
statements to be present in order to provide `ts.Symbol`s and `ts.Type`s for the directives.
This commit adds logic to the TCB generation to ensure the required
information is available in a form that the `TemplateTypeChecker` can
consume. It also adds an option to the `NgCompiler` that makes this
generation configurable.
PR Close#38618
This commit defines the interfaces which outline the information the
`TemplateTypeChecker` can return when requesting a Symbol for an item in the
`TemplateAst`.
Rather than providing the `ts.Symbol`, `ts.Type`, etc.
information in several separate functions, the `TemplateTypeChecker` can
instead provide all the useful information it knows about a particular
node in the `TemplateAst` and allow the callers to determine what to do
with it.
PR Close#38618
When type-checking a component, the declaring NgModule scope is used
to create a directive matcher that contains flattened directive metadata,
i.e. the metadata of a directive and its base classes. This computation
is done for all components, whereas the type-check scope is constant per
NgModule. Additionally, the flattening of metadata is constant per
directive instance so doesn't necessarily have to be recomputed for
each component.
This commit introduces a `TypeCheckScopes` class that is responsible
for flattening directives and computing the scope per NgModule. It
caches the computed results as appropriate to avoid repeated computation.
PR Close#38539
For the compilation of a component, the compiler has to prepare some
information about the directives and pipes that are used in the template.
This information includes an expression for directives/pipes, for usage
within the compilation output. For large NgModule compilation scopes
this has shown to introduce a performance hotspot, as the generation of
expressions is quite expensive. This commit reduces the performance
overhead by only generating expressions for the directives/pipes that
are actually used within the template, significantly cutting down on
the compiler's resolve phase.
PR Close#38539
The type-to-value conversion could previously crash if a symbol was
resolved that does not have any declarations, e.g. because it's imported
from a missing module. This would typically result in a semantic
TypeScript diagnostic and halt further compilation, therefore not
reaching the type-to-value conversion logic. In Bazel however, it turns
out that Angular semantic diagnostics are requested even if there are
semantic TypeScript errors in the program, so it would then reach the
type-to-value conversation and crash.
This commit fixes the unsafe access and adds a test that ignores the
TypeScript semantic error, effectively replicating the situation as
experienced under Bazel.
Fixes#38670
PR Close#38684
Previously, localized strings had very limited or incorrect source-mapping
information available.
Now the i18n AST nodes and related output AST nodes include source-span
information about message-parts and placeholders - including closing tag
placeholders.
This information is then used when generating the final localized string
ASTs to ensure that the correct source-mapping is rendered.
See #38588 (comment)
PR Close#38645
Previously this interface was mostly stored in compiler-cli, but it
contains some properties that would be useful for compiling the
"declare component" prelink code.
This commit moves some of the interface over to the compiler
package so that it can be referenced there without creating a
circular dependency between the compiler and compiler-cli.
PR Close#38594
The `R3TargetBinder` accepts an interface for directive metadata which
declares types for `input` and `output` objects. These types convey the
mapping between the property names for an input or output and the
corresponding property name on the component class. Due to
`R3TargetBinder`'s requirements, this mapping was specified with property
names as keys and field names as values.
However, because of duck typing, this interface was accidentally satisifed
by the opposite mapping, of field names to property names, that was produced
in other parts of the compiler. This form more naturally represents the data
model for inputs.
Rather than accept the field -> property mapping and invert it, this commit
introduces a new abstraction for such mappings which is bidirectional,
eliminating the ambiguous plain object type. This mapping uses new,
unambiguous terminology ("class property name" and "binding property name")
and can be used to satisfy both the needs of the binder as well as those of
the template type-checker (field -> property).
A new test ensures that the input/output metadata produced by the compiler
during analysis is directly compatible with the binder via this unambiguous
new interface.
PR Close#38685
If a type has been renamed when it was exported, we need to
reference the external public alias name rather than the internal
original name for the type. Otherwise we will try to import the
type by its internal name, which is not publicly accessible.
Fixes#38238
PR Close#38666
A recent change to `@angular/localize` brought in the `AbsoluteFsPath` type
from the `@angular/compiler-cli`. But this brought along with it a reference
to NodeJS typings - specifically the `FileSystem` interface refers to the
`Buffer` type from NodeJS.
This affects compilation of `@angular/localize` code that will be run in
the browser - for example projects that reference `loadTranslations()`.
The compilation breaks if the NodeJS typings are not included in the build.
Clearly it is not desirable to have these typings included when the project
is not targeting NodeJS.
This commit replaces references to the NodeJS `Buffer` type with `Uint8Array`,
which is available across all platforms and is actually the super-class of
`Buffer`.
Fixes#38692
PR Close#38700
Previously, the compiler was not able to display template parsing errors as
true `ts.Diagnostic`s that point inside the template. Instead, it would
throw an actual `Error`, and "crash" with a stack trace containing the
template errors.
Not only is this a poor user experience, but it causes the Language Service
to also crash as the user is editing a template (in actuality the LS has to
work around this bug).
With this commit, such parsing errors are converted to true template
diagnostics with appropriate span information to be displayed contextually
along with all other diagnostics. This majorly improves the user experience
and unblocks the Language Service from having to deal with the compiler
"crashing" to report errors.
PR Close#38576
The template type-checking engine includes utilities for creating
`ts.Diagnostic`s for component templates. Previously only the template type-
checker itself created such diagnostics. However, the template parser also
produces errors which should be represented as template diagnostics.
This commit prepares for that conversion by extracting the machinery for
producing template diagnostics into its own sub-package, so that other parts
of the compiler can depend on it without depending on the entire template
type-checker.
PR Close#38576
Previously, the `sourceSpan` and `startSourceSpan` were the same
object, which meant that you had the following situation:
```
element = <div>some content</div>
sourceSpan = <div>
startSourceSpan = <div>
endSourceSpan = </div>
```
This made `sourceSpan` redundant and meant that if you
wanted a span for the whole element including its content
and closing tag, it had to be computed.
Now `sourceSpan` is separated from `startSourceSpan`
resulting in:
```
element = <div>some content</div>
sourceSpan = <div>some content</div>
startSourceSpan = <div>
endSourceSpan = </div>
```
PR Close#38581
Previously the lexer was responsible for deciding whether an "inline"
template should also have its line-endings normalized.
Now this decision is made higher up in the call stack to allow more
flexibility in the parser/lexer.
PR Close#38581
The HTML parser gets an element's namespace either from the tag name
(e.g. `<svg:rect>`) or from its parent element `<svg><rect></svg>`) which
breaks down when an element is inside of an SVG `foreignElement`,
because foreign elements allow nodes from a different namespace to be
inserted into an SVG.
These changes add another flag to the tag definitions which tells child
nodes whether to try to inherit their namespaces from their parents.
It also adds a definition for `foreignObject` with the new flag,
allowing elements placed inside it to infer their namespaces instead.
Fixes#37218.
PR Close#38477
With Typescript 4, `ts.updateIdentifier` is no longer available.
Calling `ts.updateIdentifier` used to return the same node when
`typeArguments` was `undefined` because `node.typeArguments`
was also `undefined`.
Relevant TS code:
```js
function updateIdentifier(node, typeArguments) {
return node.typeArguments !== typeArguments
? updateNode(createIdentifier(ts.idText(node), typeArguments), node)
: node;
}
```
PR Close#38076
Prior to this change, the unary + and - operators would be parsed as `x - 0`
and `0 - x` respectively. The runtime semantics of these expressions are
equivalent, however they may introduce inaccurate template type checking
errors as the literal type is lost, for example:
```ts
@Component({
template: `<button [disabled]="isAdjacent(-1)"></button>`
})
export class Example {
isAdjacent(direction: -1 | 1): boolean { return false; }
}
```
would incorrectly report a type-check error:
> error TS2345: Argument of type 'number' is not assignable to parameter
of type '-1 | 1'.
Additionally, the translated expression for the unary + operator would be
considered as arithmetic expression with an incompatible left-hand side:
> error TS2362: The left-hand side of an arithmetic operation must be of
type 'any', 'number', 'bigint' or an enum type.
To resolve this issues, the implicit transformation should be avoided.
This commit adds a new unary AST node to represent these expressions,
allowing for more accurate type-checking.
Fixes#20845Fixes#36178
PR Close#37918
We had a couple of places where we were assuming that if a particular
symbol has a value, then it will exist at runtime. This is true in most cases,
but it breaks down for `const` enums.
Fixes#38513.
PR Close#38542
This commit adds a `getTemplateOfComponent` method to the
`TemplateTypeChecker` API, which retrieves the actual nodes parsed and used
by the compiler for template type-checking. This is advantageous for the
language service, which may need to query other APIs in
`TemplateTypeChecker` that require the same nodes used to bind the template
while generating the TCB.
Fixes#38352
PR Close#38355
Similarly to the change we landed in the `@angular/core` reflection
capabilities, we need to make sure that ngcc can detect pass-through
delegate constructors for classes using downleveled ES2015 output.
More details can be found in the preceding commit, and in the issue
outlining the problem: #38453.
Fixes#38453.
PR Close#38463
This commit updates the code to move generated i18n statements into the `consts` field of
ComponentDef to avoid invoking `$localize` function before component initialization (to better
support runtime translations) and also avoid problems with lazy-loading when i18n defs may not
be present in a chunk where it's referenced.
Prior to this change the i18n statements were generated at the top leve:
```
var I18N_0;
if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
var MSG_X = goog.getMsg(“…”);
I18N_0 = MSG_X;
} else {
I18N_0 = $localize('...');
}
defineComponent({
// ...
template: function App_Template(rf, ctx) {
i0.ɵɵi18n(2, I18N_0);
}
});
```
This commit updates the logic to generate the following code instead:
```
defineComponent({
// ...
consts: function() {
var I18N_0;
if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
var MSG_X = goog.getMsg(“…”);
I18N_0 = MSG_X;
} else {
I18N_0 = $localize('...');
}
return [
I18N_0
];
},
template: function App_Template(rf, ctx) {
i0.ɵɵi18n(2, 0);
}
});
```
Also note that i18n template instructions now refer to the `consts` array using an index
(similar to other template instructions).
PR Close#38404
For a template that contains for example `<span *ngIf="first"></span>`
there's no need to render the `NgIf` guard expression, as the child
scope does not have any type-checking statements, so any narrowing
effect of the guard is not applicable.
This seems like a minor improvement, however it reduces the number of
flow-node antecedents that TypeScript needs to keep into account for
such cases, resulting in an overall reduction of type-checking time.
PR Close#38418
The template type-checker would always generate a directive declaration
even if its type was never used. For example, directives without any
input nor output bindings nor exportAs references don't need the
directive to be declared, as its type would never be used.
This commit makes the `TcbOp`s that are responsible for declaring a
directive as optional, such that they are only executed when requested
from another operation.
PR Close#38418
The template type-checker would generate a statement with a call
expression for all DOM elements in a template of the form:
```
const _t1 = document.createElement("div");
```
Profiling has shown that this is a particularly expensive call to
perform type inference on, as TypeScript needs to perform signature
selection of `Document.createElement` and resolve the exact type from
the `HTMLElementTagNameMap`. However, it can be observed that the
statement by itself does not contribute anything to the type-checking
result if `_t1` is not actually used anywhere, which is only rarely the
case---it requires that the element is referenced by its name from
somewhere else in the template. Consequently, the type-checker can skip
generating this statement altogether for most DOM elements.
The effect of this optimization is significant in several phases:
1. Less type-check code to generate
2. Less type-check code to emit and parse again
3. No expensive type inference to perform for the call expression
The effect on phase 3 is the most significant here, as type-checking is
not currently incremental in the sense that only phases 1 and 2 can
be reused from a prior compilation. The actual type-checking of all
templates in phase 3 needs to be repeated on each incremental
compilation, so any performance gains we achieve here are very
beneficial.
PR Close#38418
The compiler does not currently report errors when there's an `@Input()`
for a `private`, `protected`, or `readonly` directive/component class member.
This change adds an option to enable reporting errors when a template
attempts to bind to one of these restricted input fields.
PR Close#38249
Prior to this change, the template type checker would always use a
type-constructor to instantiate a directive. This type-constructor call
serves two purposes:
1. Infer any generic types for the directive instance from the inputs
that are passed in.
2. Type check the inputs that are passed into the directive's inputs.
The first purpose is only relevant when the directive actually has any
generic types and using a type-constructor for these cases inhibits
a type-check performance penalty, as a type-constructor's signature is
quite complex and needs to be generated for each directive.
This commit refactors the generated type-check blocks to only generate
a type-constructor call for directives that have generic types. Type
checking of inputs is achieved by generating individual statements for
all inputs, using assignments into the directive's fields.
Even if a type-constructor is used for type-inference of generic types
will the input checking also be achieved using the individual assignment
statements. This is done to support the rework of the language service,
which will start to extract symbol information from the type-check
blocks.
As a future optimization, it may be possible to reduce the number of
inputs passed into a type-constructor to only those inputs that
contribute the the type-inference of the generics. As this is not a
necessity at the moment this is left as follow-up work.
Closes#38185
PR Close#38249
"Quote expressions" are expressions that start with an identifier followed by a
comma, allowing arbitrary syntax to follow. These kinds of expressions would
throw a an error in the template type checker, which would make them hard to
track down. As quote expressions are not generally used at all, the error would
typically occur for URLs that would inadvertently occur in a binding:
```html
<a [href]="https://example.com"></a>
```
This commit lets such bindings be inferred as the `any` type.
Fixes#36568
Resolves FW-2051
PR Close#37917
In TypeScript 3.8 support was added for type-only imports, which only brings in
the symbol as a type, not their value. The Angular compiler did not yet take
the type-only keyword into account when representing symbols in type positions
as value expressions. The class metadata that the compiler emits would include
the value expression for its parameter types, generating actual imports as
necessary. For type-only imports this should not be done, as it introduces an
actual import of the module that was originally just a type-only import.
This commit lets the compiler deal with type-only imports specially, preventing
a value expression from being created.
Fixes#37900
PR Close#37912
When using the safe navigation operator in a binding expression, a temporary
variable may be used for storing the result of a side-effectful call.
For example, the following template uses a pipe and a safe property access:
```html
<app-person-view [enabled]="enabled" [firstName]="(person$ | async)?.name"></app-person-view>
```
The result of the pipe evaluation is stored in a temporary to be able to check
whether it is present. The temporary variable needs to be declared in a separate
statement and this would also cause the full expression itself to be pulled out
into a separate statement. This would compile into the following
pseudo-code instructions:
```js
var temp = null;
var firstName = (temp = pipe('async', ctx.person$)) == null ? null : temp.name;
property('enabled', ctx.enabled)('firstName', firstName);
```
Notice that the pipe evaluation happens before evaluating the `enabled` binding,
such that the runtime's internal binding index would correspond with `enabled`,
not `firstName`. This introduces a problem when the pipe uses `WrappedValue` to
force a change to be detected, as the runtime would then mark the binding slot
corresponding with `enabled` as dirty, instead of `firstName`. This results
in the `enabled` binding to be updated, triggering setters and affecting how
`OnChanges` is called.
In the pseudo-code above, the intermediate `firstName` variable is not strictly
necessary---it only improved readability a bit---and emitting it inline with
the binding itself avoids the out-of-order execution of the pipe:
```js
var temp = null;
property('enabled', ctx.enabled)
('firstName', (temp = pipe('async', ctx.person$)) == null ? null : temp.name);
```
This commit introduces a new `BindingForm` that results in the above code to be
generated and adds compiler and acceptance tests to verify the proper behavior.
Fixes#37194
PR Close#37911
When we were outputting class members for `setClassMetadata` calls,
we were using the string representation of the member name. This can
lead to us generating invalid code when the name contains dashes and
is quoted (e.g. `@Output() 'has-dashes' = new EventEmitter()`), because
the quotes will be stripped for the string representation.
These changes fix the issue by using the original name AST node that was
used for the declaration and which knows whether it's supposed to be
quoted or not.
Fixes#38311.
PR Close#38387
For attribute bindings that target a directive's input, the template
type checker is able to verify that the type of the input expression is
compatible with the directive's declaration for said input. This
checking adheres to the `strictNullChecks` flag as configured in the
TypeScript compilation, such that errors are reported for expressions
that include `undefined` or `null` in their type if the input's
declaration does not include those types.
There was a bug with this level of type-checking for directives that
also declare coercion members, where binding an expression that includes
the `undefined` type to a directive's input that does not include the
`undefined` type would not be reported as error.
This commit fixes the bug by changing the type-constructor in type-check
code to use an intersection type of regular inputs and coerced inputs,
instead of a union type. The union type would inadvertently allow
`undefined` types to be assigned into the regular inputs, as that would
still satisfy the characteristics of a union type.
As a result of this change, you may start to see build failures if
`strictTemplates` is enabled and `strictInputTypes` is not disabled.
These errors are legitimate and some action is required to achieve a
successful build:
1. Update the templates for which an error is reported and introduce the
non-null assertion operator at the end of the expression. This
removes the `undefined` type from the expression's type, making it
appear as a valid assignment.
2. Disable `strictNullInputTypes` in the compiler options. This will
implicitly add the non-null assertion operators similar to option 1,
but all templates in the compilation are affected.
3. Update the directive's input declaration to include the `undefined`
type, if the directive is not implemented in an external library.
PR Close#38273
Roll forward of #38147.
This allows Closure compiler to tree shake unused constructor calls to `NgModuleFactory`, which is otherwise considered
side-effectful. The Angular compiler generates factory objects which are exported but typically not used, as they are
only needed for compatibility with View Engine. This results in top-level constructor calls, such as:
```typescript
export const FooNgFactory = new NgModuleFactory(Foo);
```
`NgModuleFactory` has a side-effecting constructor, so this statement cannot be tree shaken, even if `FooNgFactory` is
never imported. The `NgModuleFactory` continues to reference its associated `NgModule` and prevents the module and all
its unused dependencies from being tree shaken, making Closure builds significantly larger than necessary.
The fix here is to wrap `NgModuleFactory` constructor with `noSideEffects(() => /* ... */)`, which tricks the Closure
compiler into assuming that the invoked function has no side effects. This allows it to tree-shake unused
`NgModuleFactory()` constructors when they aren't imported. Since the factory can be removed, the module can also be
removed (if nothing else references it), thus tree shaking unused dependencies as expected.
The one notable edge case is for lazy loaded modules. Internally, lazy loading is done as a side effect when the lazy
script is evaluated. For Angular, this side effect is registering the `NgModule`. In Ivy this is done by the
`NgModuleFactory` constructor, so lazy loaded modules **cannot** have their top-level `NgModuleFactory` constructor
call tree shaken. We handle this case by looking for the `id` field on `@NgModule` annotations. All lazy loaded modules
include an `id`. When this `id` is found, the `NgModuleFactory` is generated **without** with `noSideEffects()` call,
so Closure will not tree shake it and the module will lazy-load correctly.
PR Close#38320
This introduces a new `ModuleInfo` interface to represent some of the statically analyzed data from an `NgModule`. This
gets passed into transforms to give them more context around a given `NgModule` in the compilation.
PR Close#38320
The `TscPlugin` interface using a type of `ts.CompilerHost&Partial<UnifiedModulesHost>` for the `host` parameter
of the `wrapHost` method. However, prior to this change, the interface implementing `NgTscPlugin` class used a
type of `ts.CompilerHost&UnifiedModulesHost` for the parameter. This change corrects the inconsistency and
allows `UnifiedModulesHost` members to be optional when using the `NgtscPlugin`.
PR Close#38004
Currently the `getInheritedFactory` function is implemented to allow
closure to remove the call if the base factory is unused. However, this
method does not work with terser. By adding the PURE annotation,
terser will also be able to remove the call when unused.
PR Close#38291
This reverts commit 7f8c2225f2.
This commit caused test failures internally, which were traced back to the
optimizer removing NgModuleFactory constructor calls when those calls caused
side-effectful registration of NgModules by their ids.
PR Close#38303
This commit disables one TypeChecker test (added as a part of
https://github.com/angular/angular/pull/38105) which make assertions about the filename while
running on Windows.
Such assertions are currently suffering from a case sensitivity issue.
PR Close#38294
This allows Closure compiler to tree shake unused constructor calls to `NgModuleFactory`, which is otherwise considered
side-effectful. The Angular compiler generates factory objects which are exported but typically not used, as they are
only needed for compatibility with View Engine. This results in top-level constructor calls, such as:
```typescript
export const FooNgFactory = new NgModuleFactory(Foo);
```
`NgModuleFactory` has a side-effecting constructor, so this statement cannot be tree shaken, even if `FooNgFactory` is
never imported. The `NgModuleFactory` continues to reference its associated `NgModule` and prevents the module and all
its unused dependencies from being tree shaken. This effectively prevents all components from being tree shaken, making
Closure builds significantly larger than they should be.
The fix here is to wrap `NgModuleFactory` constructor with `noSideEffects(() => /* ... */)`, which tricks the Closure
compiler into assuming that the invoked function has no side effects. This allows it to tree-shake unused
`NgModuleFactory()` constructors when they aren't imported. Since the factory can be removed, the module can also be
removed (if nothing else references it), thus tree shaking unused components as expected.
PR Close#38147
Large strings constants are now wrapped in a function which is called whenever used. This works around a unique
limitation of Closure, where it will **always** inline string literals at **every** usage, regardless of how large the
string literal is or how many times it is used.The workaround is to use a function rather than a string literal.
Closure has differently inlining semantics for functions, where it will check the length of the function and the number
of times it is used before choosing to inline it. By using a function, `ngtsc` makes Closure more conservative about
inlining large strings, and avoids blowing up the bundle size.This optimization is only used if the constant is a large
string. A wrapping function is not included for other use cases, since it would just increase the bundle size and add
unnecessary runtime performance overhead.
PR Close#38253
This commit adds a method `getDiagnosticsForComponent` to the
`TemplateTypeChecker`, which does the minimum amount of work to retrieve
diagnostics for a single component.
With the normal `ReusedProgramStrategy` this offers virtually no improvement
over the standard `getDiagnosticsForFile` operation, but if the
`TypeCheckingProgramStrategy` supports separate shims for each component,
this operation can yield a faster turnaround for components that are
declared in files with many other components.
PR Close#38105
Previously, a stable template id was implemented for each component in a
file. This commit adds this id to each `TemplateDiagnostic` generated from
the template type-checker, so it can potentially be used for filtration.
PR Close#38105
This commit adds an `overrideComponentTemplate` operation to the template
type-checker. This operation changes the template used during template
type-checking operations.
Overriding a template causes any previous work for it to be discarded, and
the template type-checking engine will regenerate the TCB for that template
on the next request.
This operation can be used by a consumer such as the language service to
get rapid feedback or diagnostics as the user is editing a template file,
without the need for a full incremental build iteration.
Closes#38058
PR Close#38105
Previously, the `TemplateTypeChecker` abstraction allowed fetching
diagnostics for a single file, but under the hood would generate type
checking code for the entire program to satisfy the request.
With this commit, an `OptimizeFor` hint is passed to `getDiagnosticsForFile`
which indicates whether the user intends to request diagnostics for the
whole program or is truly interested in just the single file. If the latter,
the `TemplateTypeChecker` can perform only the work needed to produce
diagnostics for just that file, thus returning answers more efficiently.
PR Close#38105
The template type-checking engine relies on the abstraction interface
`TypeCheckingProgramStrategy` to create updated `ts.Program`s for
template type-checking. The basic API is that the type-checking engine
requests changes to certain files in the program, and the strategy provides
an updated `ts.Program`.
Typically, such changes are made to 'ngtypecheck' shim files, but certain
conditions can cause template type-checking to require "inline" operations,
which change user .ts files instead. The strategy used by 'ngc' (the
`ReusedProgramStrategy`) supports these kinds of updates, but other clients
such as the language service might not always support modifying user files.
To accommodate this, the `TypeCheckingProgramStrategy` interface was
modified to include a `supportsInlineOperations` flag. If an implementation
specifies `false` for inline support, the template type-checking system will
return diagnostics on components which would otherwise require inline
operations.
Closes#38059
PR Close#38105
This commit significantly refactors the 'typecheck' package to introduce a
new abstraction, the `TemplateTypeChecker`. To achieve this:
* a 'typecheck:api' package is introduced, containing common interfaces that
consumers of the template type-checking infrastructure can depend on
without incurring a dependency on the template type-checking machinery as
a whole.
* interfaces for `TemplateTypeChecker` and `TypeCheckContext` are introduced
which contain the abstract operations supported by the implementation
classes `TemplateTypeCheckerImpl` and `TypeCheckContextImpl` respectively.
* the `TemplateTypeChecker` interface supports diagnostics on a whole
program basis to start with, but the implementation is purposefully
designed to support incremental diagnostics at a per-file or per-component
level.
* `TemplateTypeChecker` supports direct access to the type check block of a
component.
* the testing utility is refactored to be a lot more useful, and new tests
are added for the new abstraction.
PR Close#38105
Previously in the template type-checking engine, it was assumed that every
input file would have an associated type-checking shim. The type check block
code for all components in the input file would be generated into this shim.
This is fine for whole-program type checking operations, but to support the
language service's requirements for low latency, it would be ideal to be
able to check a single component in isolation, especially if the component
is declared along with many others in a single file.
This commit removes the assumption that the file/shim mapping is 1:1, and
introduces the concept of component-to-shim mapping. Any
`TypeCheckingProgramStrategy` must provide such a mapping.
To achieve this:
* type checking record information is now split into file-level data as
well as per-shim data.
* components are now assigned a stable `TemplateId` which is unique to the
file in which they're declared.
PR Close#38105
When the `NgIf` directive is used in a template, its context variables
can be used to capture the bound value. This is sometimes used in
complex expressions, where the resulting value is captured in a
context variable. There's two syntax forms available:
1. Binding to `NgIfContext.ngIf` using the `as` syntax:
```html
<span *ngIf="enabled && user as u">{{u.name}}</span>
```
2. Binding to `NgIfContext.$implicit` using the `let` syntax:
```html
<span *ngIf="enabled && user; let u">{{u.name}}</span>
```
Because of the semantics of `ngIf`, it is known that the captured
context variable is truthy, however the template type checker
would not consider them as such and still report errors when
`strict` is enabled.
This commit updates `NgIf`'s context guard to make the types of the
context variables truthy, avoiding the issue.
Based on https://github.com/angular/angular/pull/35125
PR Close#36627
The current implementation of the TypeScriptReflectionHost does not account for members that
are string literals, i.e. `class A { 'string-literal-prop': string; }`
PR Close#38226
Prior to this commit, duplicated styles defined in multiple components in the same file were not
shared between components, thus causing extra payload size. This commit updates compiler logic to
use `ConstantPool` for the styles (while generating the `styles` array on component def), which
enables styles sharing when needed (when duplicates styles are present).
Resolves#38204.
PR Close#38213
This commit splits the transformation into 2 separate steps: Ivy compilation and actual transformation
of corresponding TS nodes. This is needed to have all `o.Expression`s generated before any TS transforms
happen. This allows `ConstantPool` to properly identify expressions that can be shared across multiple
components declared in the same file.
Resolves#38203.
PR Close#38213
This commit updates synthetic host property and listener instruction names to better align with other instructions.
The `ɵɵupdateSyntheticHostBinding` instruction was renamed to `ɵɵsyntheticHostProperty` (to match the `ɵɵhostProperty`
instruction name) and `ɵɵcomponentHostSyntheticListener` was renamed to `ɵɵsyntheticHostListener` since this
instruction is generated for both Components and Directives (so 'component' is removed from the name).
This PR is a followup after PR #35568.
PR Close#37145
Fixes the following issues related to how we validate properties during JIT:
- The invalid property warning was printing `null` as the node name
for `ng-content`. The problem is that when generating a template from
`ng-content` we weren't capturing the node name.
- We weren't running property validation on `ng-container` at all.
This used to be supported on ViewEngine and seems like an oversight.
In the process of making these changes, I found and cleaned up a
few places where we were passing in `LView` unnecessarily.
PR Close#37773
In CLI v10 there was a move to use the new solution-style tsconfig
which became available in TS 3.9.
The result of this is that the standard tsconfig.json no longer contains
important information such as "paths" mappings, which ngcc might need to
correctly compute dependencies.
ngcc (and ngc and tsc) infer the path to tsconfig.json if not given an
explicit tsconfig file-path. But now that means it infers the solution
tsconfig rather than one that contains the useful information it used to
get.
This commit logs a warning in this case to inform the developer
that they might not have meant to load this tsconfig and offer
alternative options.
Fixes#36386
PR Close#38003
The `fs.relative()` method assumed that the file-system is a single tree,
which is not the case in Windows, where you can have multiple drives,
e.g. `C:`, `D:` etc.
This commit changes `fs.relative()` so that it no longer forces the result
to be a `PathSegment` and then flows that refactoring through the rest of
the compiler-cli (and ngcc). The main difference is that now, in some cases,
we needed to check whether the result is "rooted", i.e an `AbsoluteFsPath`,
rather than a `PathSegment`, before using it.
Fixes#36777
PR Close#37959
Builds on top of #34655 to support more cases that could be using a pipe inside host bindings (e.g. ternary expressions or function calls).
Fixes#37610.
PR Close#37883
The `ng_module` rule supports the generation of flat module bundles. In
View Engine, information about this flat module bundle is exposed
as a Bazel provider. This is helpful as other rules like `ng_package`
could rely on this information to determine entry-points for the APF.
With Ivy this currently does not work because the flat module
information is not exposed in the provider. The reason for this is
unclear. We should also provide this information in Ivy so that rules
like `ng_package` can also determine the correct entry-points when a
package is built specifically with `--config=ivy`.
PR Close#36971
The ngtsc testing packages for file_system and logging were missing from the bazel deps rules, which means that they were not included in the releases
PR Close#37977
Currently when the `plural` or `select` keywords in an ICU contain trailing spaces (e.g. `{count, select , ...}`), these spaces are also included into the key names in ICU vars (e.g. "VAR_SELECT "). These trailing spaces are not desirable, since they will later be converted into `_` symbols while normalizing placeholder names, thus causing mismatches at runtime (i.e. placeholder will not be replaced with the correct value). This commit updates the code to trim these spaces while generating an object with placeholders, to make sure the runtime logic can replace these placeholders with the right values.
PR Close#37866
Incremental compilation allows for the output state of one compilation to be
reused as input to the next compilation. This involves retaining references
to instances from prior compilations, which must be done carefully to avoid
memory leaks.
This commit fixes such a leak with a complicated retention chain:
* `TrackedIncrementalBuildStrategy` unnecessarily hangs on to the previous
`IncrementalDriver` (state of the previous compilation) once the current
compilation completes.
In general this is unnecessary, but should be safe as long as the chain
only goes back one level - if the `IncrementalDriver` doesn't retain any
previous `TrackedIncrementalBuildStrategy` instances. However, this does
happen:
* `NgCompiler` indirectly causes retention of previous `NgCompiler`
instances (and thus previous `TrackedIncrementalBuildStrategy` instances)
through accidental capture of the `this` context in a closure created in
its constructor. This closure is wrapped in a `ts.ModuleResolutionCache`
used to create a `ModuleResolver` class, which is passed to the program's
`TraitCompiler` on construction.
* The `IncrementalDriver` retains a reference to the `TraitCompiler` of the
previous compilation, completing the reference chain.
The final retention chain thus looks like:
* `TrackedIncrementalBuildStrategy` of current program
* `.previous`: `IncrementalDriver` of previous program
* `.lastGood.traitCompiler`: `TraitCompiler`
* `.handlers[..].moduleResolver.moduleResolutionCache`: cache
* (via `getCanonicalFileName` closure): `NgCompiler`
* `.incrementalStrategy`: `TrackedIncrementalBuildStrategy` of previous
program.
The closure link is the "real" leak here. `NgCompiler` is creating a closure
for `getCanonicalFileName`, delegating to its
`this.adapter.getCanonicalFileName`, for the purposes of creating a
`ts.ModuleResolutionCache`. The fact that the closure references
`NgCompiler` thus eventually causes previous `NgCompiler` iterations to be
retained. This is also potentially problematic due to the shared nature of
`ts.ModuleResolutionCache`, which is potentially retained across multiple
compilations intentionally.
This commit fixes the first two links in the retention chain: the build
strategy is patched to not retain a `previous` pointer, and the `NgCompiler`
is patched to not create a closure in the first place, but instead pass a
bound function. This ensures that the `NgCompiler` does not retain previous
instances of itself in the first place, even if the build strategy does
end up retaining the previous incremental state unnecessarily.
The third link (`IncrementalDriver` unnecessarily retaining the whole
`TraitCompiler`) is not addressed in this commit as it's a more
architectural problem that will require some refactoring. However, the leak
potential of this retention is eliminated thanks to fixing the first two
issues.
PR Close#37835
When ngcc creates an entry-point program, the `allowJs` option is enabled
in order to operate on the JavaScript source files of the entry-point.
A side-effect of this approach is that external modules that don't ship
declaration files will also have their JavaScript source files loaded
into the program, as the `allowJs` flag allows for them to be imported.
This may pose an issue in certain edge cases, where ngcc would inadvertently
operate on these external modules. This can introduce all sorts of undesirable
behavior and incompatibilities, e.g. the reflection host that is selected for
the entry-point's format could be incompatible with that of the external
module's JavaScript bundles.
To avoid these kinds of issues, module resolution that would resolve to
a JavaScript file located outside of the package will instead be rejected,
as if the file would not exist. This would have been the behavior when
`allowJs` is set to false, which is the case in typical Angular compilations.
Fixes#37508
PR Close#37596
Changes `isWithinPackage` to take an `AbsoluteFsPath` instead of `ts.SourceFile`,
to allow for an upcoming change to use it when no `ts.SourceFile` is available,
but just a path.
PR Close#37596
Previously an error thrown in the `analyzeFn` would cause
the ngcc process to exit immediately without removing the
lockfile, and potentially before the unlocker process had been
successfully spawned resulting in the lockfile being orphaned
and left behind.
Now we catch these errors and remove the lockfile as needed.
PR Close#37739
This commit disables all diagnostic tests for DynamicValue diagnostics which
make assertions about the diagnostic filename while running tests on Windows.
Such assertions are currently suffering from a case sensitivity issue.
PR Close#37763
Several partial_evaluator tests in the diagnostics_spec check assert
correctness of diagnostic filenames. Previously these assertions compared
a resolved (`absoluteFrom`) filename with the TypeScript `ts.SourceFile`'s
`fileName` string, which caused the tests to fail on Windows because the
drive letter case differed.
This commit changes the assertions to use `absoluteFromSourceFile` instead
of the `fileName` string, resulting in an apples-to-apples comparison of
canonicalized paths.
PR Close#37758
This commit introduces a dedicated `DynamicValue` kind to indicate that a value
cannot be evaluated statically as the function body is not just a single return
statement. This allows more accurate reporting of why a function call failed
to be evaluated, i.e. we now include a reference to the function declaration
and have a tailor-made diagnostic message.
PR Close#37587
Alex Rickabaugh
Pete Bacon Darwin
twerske
Andrew Kushnir
JoostK
Kristiyan Kostadinov
Greg Magolan
Joey Perrott
Andrew Scott
Charles Lyding
Alan Agius
Paul Gschwendtner
Ahn
Andrew Scott
Doug Parker
Andrea Canciani
Olivier Combe
Keen Yee Liau