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
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 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
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
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 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
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
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
During AOT compilation, the value of some expressions need to be known at
compile time. The compiler has the ability to statically evaluate expressions
the best it can, but there can be occurrences when an expression cannot be
evaluated statically. For instance, the evaluation could depend on a dynamic
value or syntax is used that the compiler does not understand. Alternatively,
it is possible that an expression could be statically evaluated but the
resulting value would be of an incorrect type.
In these situations, it would be helpful if the compiler could explain why it
is unable to evaluate an expression. To this extend, the static interpreter
in Ivy keeps track of a trail of `DynamicValue`s which follow the path of nodes
that were considered all the way to the node that causes an expression to be
considered dynamic. Up until this commit, this rich trail of information was
not surfaced to a developer so the compiler was of little help to explain
why static evaluation failed, resulting in situations that are hard to debug
and resolve.
This commit adds much more insight to the diagnostic that is produced for static
evaluation errors. For dynamic values, the trail of `DynamicValue` instances
is presented to the user in a meaningful way. If a value is available but not
of the correct type, the type of the resolved value is shown.
Resolves FW-2155
PR Close#37587
Commit 4213e8d5 introduced shim reference tagging into the compiler, and
changed how the `TypeCheckProgramHost` worked under the hood during the
creation of a template type-checking program. This work enabled a more
incremental flow for template type-checking, but unintentionally introduced
several regressions in performance, caused by poor incrementality during
`ts.Program` creation.
1. The `TypeCheckProgramHost` was made to rely on the `ts.CompilerHost` to
retrieve instances of `ts.SourceFile`s from the original program. If the
host does not return the original instance of such files, but instead
creates new instances, this has two negative effects: it incurs
additional parsing time, and it interferes with TypeScript's ability to
reuse information about such files.
2. During the incremental creation of a `ts.Program`, TypeScript compares
the `referencedFiles` of `ts.SourceFile` instances from the old program
with those in the new program. If these arrays differ, TypeScript cannot
fully reuse the old program. The implementation of reference tagging
introduced in 4213e8d5 restores the original `referencedFiles` array
after a `ts.Program` is created, which means that future incremental
operations involving that program will always fail this comparison,
effectively limiting the incrementality TypeScript can achieve.
Problem 1 exacerbates problem 2: if a new `ts.SourceFile` is created by the
host after shim generation has been disabled, it will have an untagged
`referencedFiles` array even if the original file's `referencedFiles` was
not restored, triggering problem 2 when creating the template type-checking
program.
To fix these issues, `referencedFiles` arrays are now restored on the old
`ts.Program` prior to the creation of a new incremental program. This allows
TypeScript to get the most out of reusing the old program's data.
Additionally, the `TypeCheckProgramHost` now uses the original `ts.Program`
to retrieve original instances of `ts.SourceFile`s where possible,
preventing issues when a host would otherwise return fresh instances.
Together, these fixes ensure that program reuse is as incremental as
possible, and tests have been added to verify this for certain scenarios.
An optimization was further added to prevent the creation of a type-checking
`ts.Program` in the first place if no type-checking is necessary.
PR Close#37641
We recently added a transformer to NGC that is responsible for downleveling Angular
decorators and constructor parameter types. The primary goal was to mitigate a
TypeScript limitation/issue that surfaces in Angular projects due to the heavy
reliance on type metadata being captured for DI. Additionally this is a pre-requisite
of making `tsickle` optional in the Angular bazel toolchain.
See: 401ef71ae5 for more context on this.
Another (less important) goal was to make sure that the CLI can re-use
this transformer for its JIT mode compilation. The CLI (as outlined in
the commit mentioned above), already has a transformer for downleveling
constructor parameters. We want to avoid this duplication and exported
the transform through the tooling-private compiler entry-point.
Early experiments in using this transformer over the current one, highlighted
that in JIT, class decorators cannot be downleveled. Angular relies on those
to be invoked immediately for JIT (so that factories etc. are generated upon loading)
The transformer we exposed, always downlevels such class decorators
though, so that would break CLI's JIT mode. We can address the CLI's
needs by adding another flag to skip class decorators. This will allow
us to continue with the goal of de-duplication.
PR Close#37545
Previously, when an entry-point was ignored via an ngcc config, ngcc
would scan sub-directories for sub-entry-points, but would not use the
correct `packagePath`. For example, if `@angular/common` was ignored, it
would look at `@angular/common/http` but incorrectly use
`.../@angular/common/http` as the `packagePath` (instead of
`.../@angular/common`). As a result, it would not retrieve the correct
ngcc config for the actual package.
This commit fixes it by ensuring the correct `packagePath` is used, even
if the primary entry-point corresponding to that path is ignored. In
order to do this, a new return value for `getEntryPointInfo()` is added:
`IGNORED_ENTRY_POINT`. This is used to differentiate between directories
that correspond to no or an incompatible entry-point and those that
correspond to an entry-point that could otherwise be valid but is
explicitly ignored. Consumers of `getEntryPointInfo()` can then use this
info to discard ignored entry-points, but still use the correct
`packagePath` when scanning their sub-directories for secondary
entry-points.
PR Close#37040
Currently Angular internally already handles `InjectionToken` as
predicates for queries. This commit exposes this as public API as
developers already relied on this functionality but currently use
workarounds to satisfy the type constraints (e.g. `as any`).
We intend to make this public as it's low-effort to support, and
it's a significant key part for the use of light-weight tokens as
described in the upcoming guide: https://github.com/angular/angular/pull/36144.
In concrete, applications might use injection tokens over classes
for both optional DI and queries, because otherwise such references
cause classes to be always retained. This was also an issue in View
Engine, but now with Ivy, this pattern became worse, as factories are
directly attached to retained classes (ultimately ending up in the
production bundle, while being unused).
More details in the light-weight token guide and in: https://github.com/angular/angular-cli/issues/16866.
Closes#21152. Related to #36144.
PR Close#37506
In v7 of Angular we removed `tsickle` from the default `ngc` pipeline.
This had the negative potential of breaking ES2015 output and SSR due
to a limitation in TypeScript.
TypeScript by default preserves type information for decorated constructor
parameters when `emitDecoratorMetadata` is enabled. For example,
consider this snippet below:
```
@Directive()
export class MyDirective {
constructor(button: MyButton) {}
}
export class MyButton {}
```
TypeScript would generate metadata for the `MyDirective` class it has
a decorator applied. This metadata would be needed in JIT mode, or
for libraries that provide `MyDirective` through NPM. The metadata would
look as followed:
```
let MyDirective = class MyDir {}
MyDirective = __decorate([
Directive(),
__metadata("design:paramtypes", [MyButton]),
], MyDirective);
let MyButton = class MyButton {}
```
Notice that TypeScript generated calls to `__decorate` and
`__metadata`. These calls are needed so that the Angular compiler
is able to determine whether `MyDirective` is actually an directive,
and what types are needed for dependency injection.
The limitation surfaces in this concrete example because `MyButton`
is declared after the `__metadata(..)` call, while `__metadata`
actually directly references `MyButton`. This is illegal though because
`MyButton` has not been declared at this point. This is due to the
so-called temporal dead zone in JavaScript. Errors like followed will
be reported at runtime when such file/code evaluates:
```
Uncaught ReferenceError: Cannot access 'MyButton' before initialization
```
As noted, this is a TypeScript limitation because ideally TypeScript
shouldn't evaluate `__metadata`/reference `MyButton` immediately.
Instead, it should defer the reference until `MyButton` is actually
declared. This limitation will not be fixed by the TypeScript team
though because it's a limitation as per current design and they will
only revisit this once the tc39 decorator proposal is finalized
(currently stage-2 at time of writing).
Given this wontfix on the TypeScript side, and our heavy reliance on
this metadata in libraries (and for JIT mode), we intend to fix this
from within the Angular compiler by downleveling decorators to static
properties that don't need to evaluate directly. For example:
```
MyDirective.ctorParameters = () => [MyButton];
```
With this snippet above, `MyButton` is not referenced directly. Only
lazily when the Angular runtime needs it. This mitigates the temporal
dead zone issue caused by a limitation in TypeScript's decorator
metadata output. See: https://github.com/microsoft/TypeScript/issues/27519.
In the past (as noted; before version 7), the Angular compiler by
default used tsickle that already performed this transformation. We
moved the transformation to the CLI for JIT and `ng-packager`, but now
we realize that we can move this all to a single place in the compiler
so that standalone ngc consumers can benefit too, and that we can
disable tsickle in our Bazel `ngc-wrapped` pipeline (that currently
still relies on tsickle to perform this decorator processing).
This transformation also has another positive side-effect of making
Angular application/library code more compatible with server-side
rendering. In principle, TypeScript would also preserve type information
for decorated class members (similar to how it did that for constructor
parameters) at runtime. This becomes an issue when your application
relies on native DOM globals for decorated class member types. e.g.
```
@Input() panelElement: HTMLElement;
```
Your application code would then reference `HTMLElement` directly
whenever the source file is loaded in NodeJS for SSR. `HTMLElement`
does not exist on the server though, so that will become an invalid
reference. One could work around this by providing global mocks for
these DOM symbols, but that doesn't match up with other places where
dependency injection is used for mocking DOM/browser specific symbols.
More context in this issue: #30586. The TL;DR here is that the Angular
compiler does not care about types for these class members, so it won't
ever reference `HTMLElement` at runtime.
Fixes#30106. Fixes#30586. Fixes#30141.
Resolves FW-2196. Resolves FW-2199.
PR Close#37382
When the compiler encounters a function call within an NgModule imports
section, it attempts to resolve it to an NgModule-annotated class by
looking at the function body and evaluating the statements there. This
evaluation can only understand simple functions which have a single
return statement as their body. If the function the user writes is more
complex than that, the compiler won't be able to understand it and
previously the PartialEvaluator would return a "DynamicValue" for
that import.
With this change, in the event the function body resolution fails the
PartialEvaluator will now attempt to use its foreign function resolvers to
determine the correct result from the function's type signtaure instead. If
the function is annotated with a correct ModuleWithProviders type, the
compiler will be able to understand the import without static analysis of
the function body.
PR Close#37126
In some versions of TypeScript, the transformation of synthetic
`$localize` tagged template literals is broken.
See https://github.com/microsoft/TypeScript/issues/38485
We now compute what the expected final output target of the
compilation will be so that we can generate ES5 compliant
`$localize` calls instead of relying upon TS to do the downleveling
for us.
This is a workaround for the TS compiler bug, which could be removed
when this is fixed. But since it only affects ES5 targeted compilations,
which is now not the norm, it has limited impact on the majority of
Angular projects. So this fix can probably be left in indefinitely.
PR Close#36989
In 420b9be1c1 all style-based sanitization code was
disabled because modern browsers no longer allow for javascript expressions within
CSS. This patch is a follow-up patch which removes all traces of style sanitization
code (both instructions and runtime logic) for the `[style]` and `[style.prop]` bindings.
PR Close#36965
ASTs for property read and method calls contain information about
the entire span of the expression, including its receiver. Use cases
like a language service and compile error messages may be more
interested in the span of the direct identifier for which the
expression is constructed (i.e. an accessed property). To support this,
this commit adds a `nameSpan` property on
- `PropertyRead`s
- `SafePropertyRead`s
- `PropertyWrite`s
- `MethodCall`s
- `SafeMethodCall`s
The `nameSpan` property already existed for `BindingPipe`s.
This commit also updates usages of these expressions' `sourceSpan`s in
Ngtsc and the langauge service to use `nameSpan`s where appropriate.
PR Close#36826
Some projects include .js source files (via the TypeScript allowJs option).
Previously, the compiler would attempt to tag these files for shims, which
caused errors as the regex used to create shim filenames assumes a .ts file.
This commit fixes the bug by filtering out non-ts files during tagging.
PR Close#36987
JoostK
Doug Parker
Charles Lyding
Alex Rickabaugh
Andrea Canciani
Andrew Kushnir
crisbeto
Pete Bacon Darwin
Paul Gschwendtner
George Kalpakas
Alan Agius
Joey Perrott
Matias Niemelä
Ayaz Hafiz