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
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
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
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
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
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 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
This reverts commit 7f8c2225f2cba3dfcf3ec23e0fe08b7d2e3497e8.
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
