Due to the fact that Tsickle runs between analyze and transform phases in Angular, Tsickle may transform nodes (add comments with type annotations for Closure) that we captured during the analyze phase. As a result, some patterns where a function is returned from another function may trigger automatic semicolon insertion, which breaks the code (makes functions return `undefined` instead of a function). In order to avoid the problem, this commit updates the code to wrap all functions in some expression ("privders" and "viewProviders") in parentheses. More info can be found in Tsickle source code here: https://github.com/angular/tsickle/blob/d7974262571c8a17d684e5ba07680e1b1993afdd/src/jsdoc_transformer.ts#L1021
PR Close#33609
The template type checker generates code to check directive inputs and
outputs, whose name may contain characters that can not be used as
identifier in TypeScript. Prior to this change, such names would be
emitted into the generated code as is, resulting in invalid code and
unexpected template type check errors.
This commit fixes the bug by representing the potentially invalid names
as string literal instead of raw identifier.
Fixes#33590
PR Close#33741
This commit transforms the setClassMetadata calls generated by ngtsc from:
```typescript
/*@__PURE__*/ setClassMetadata(...);
```
to:
```typescript
/*@__PURE__*/ (function() {
setClassMetadata(...);
})();
```
Without the IIFE, terser won't remove these function calls because the
function calls have arguments that themselves are function calls or other
impure expressions. In order to make the whole block be DCE-ed by terser,
we wrap it into IIFE and mark the IIFE as pure.
It should be noted that this change doesn't have any impact on CLI* with
build-optimizer, which removes the whole setClassMetadata block within
the webpack loader, so terser or webpack itself don't get to see it at
all. This is done to prevent cross-chunk retention issues caused by
webpack's internal module registry.
* actually we do expect a short-term size regression while
https://github.com/angular/angular-cli/pull/16228
is merged and released in the next rc of the CLI. But long term this
change does nothing to CLI + build-optimizer configuration and is done
primarly to correct the seemingly correct but non-function PURE annotation
that builds not using build-optimizer could rely on.
PR Close#33337
NgModules in Ivy have a definition which contains various different bits
of metadata about the module. In particular, this metadata falls into two
categories:
* metadata required to use the module at runtime (for bootstrapping, etc)
in AOT-only applications.
* metadata required to depend on the module from a JIT-compiled app.
The latter metadata consists of the module's declarations, imports, and
exports. To support JIT usage, this metadata must be included in the
generated code, especially if that code is shipped to NPM. However, because
this metadata preserves the entire NgModule graph (references to all
directives and components in the app), it needs to be removed during
optimization for AOT-only builds.
Previously, this was done with a clever design:
1. The extra metadata was added by a function called `setNgModuleScope`.
A call to this function was generated after each NgModule.
2. This function call was marked as "pure" with a comment and used
`noSideEffects` internally, which causes optimizers to remove it.
The effect was that in dev mode or test mode (which use JIT), no optimizer
runs and the full NgModule metadata was available at runtime. But in
production (presumably AOT) builds, the optimizer runs and removes the JIT-
specific metadata.
However, there are cases where apps that want to use JIT in production, and
still make an optimized build. In this case, the JIT-specific metadata would
be erroneously removed. This commit solves that problem by adding an
`ngJitMode` global variable which guards all `setNgModuleScope` calls. An
optimizer can be configured to statically define this global to be `false`
for AOT-only builds, causing the extra metadata to be stripped.
A configuration for Terser used by the CLI is provided in `tooling.ts` which
sets `ngJitMode` to `false` when building AOT apps.
PR Close#33671
The Ivy template type-checker is capable of inferring the type of a
structural directive (such as NgForOf<T>). Previously, this was done with
fullTemplateTypeCheck: true, even if strictTemplates was false. View Engine
previously did not do this inference, and so this causes breakages if the
type of the template context is not what the user expected.
In particular, consider the template:
```html
<div *ngFor="let user of users as all">
{{user.index}} out of {{all.length}}
</div>
```
As long as `users` is an array, this seems reasonable, because it appears
that `all` is an alias for the `users` array. However, this is misleading.
In reality, `NgForOf` is rendered with a template context that contains
both a `$implicit` value (for the loop variable `user`) as well as a
`ngForOf` value, which is the actual value assigned to `all`. The type of
`NgForOf`'s template context is `NgForContext<T>`, which declares `ngForOf`'s
type to be `NgIterable<T>`, which does not have a `length` property (due to
its incorporation of the `Iterable` type).
This commit stops the template type-checker from inferring template context
types unless strictTemplates is set (and strictInputTypes is not disabled).
Fixes#33527.
PR Close#33537
Previously, the ngtsc compiler attempted to reuse analysis work from the
previous program during an incremental build. To do this, it had to prove
that the work was safe to reuse - that no changes made to the new program
would invalidate the previous analysis.
The implementation of this had a significant design flaw: if the previous
program had errors, the previous analysis would be missing significant
information, and the dependency graph extracted from it would not be
sufficient to determine which files should be re-analyzed to fill in the
gaps. This often meant that the build output after an error was resolved
would be wholly incorrect.
This commit switches ngtsc to take a simpler approach to incremental
rebuilds. Instead of attempting to reuse prior analysis work, the entire
program is re-analyzed with each compilation. This is actually not as
expensive as one might imagine - analysis is a fairly small part of overall
compilation time.
Based on the dependency graph extracted during this analysis, the compiler
then can make accurate decisions on whether to emit specific files. A new
suite of tests is added to validate behavior in the presence of source code
level errors.
This new approach is dramatically simpler than the previous algorithm, and
should always produce correct results for a semantically correct program.s
Fixes#32388Fixes#32214
PR Close#33862
Originally, QueryList implemented Iterable and provided a Symbol.iterator
on its prototype. This caused issues with tree-shaking, so QueryList was
refactored and the Symbol.iterator added in its constructor instead. As
part of this change, QueryList no longer implemented Iterable directly.
Unfortunately, this meant that QueryList was no longer assignable to
Iterable or, consequently, NgIterable. NgIterable is used for NgFor's input,
so this meant that QueryList was not usable (in a type sense) for NgFor
iteration. View Engine's template type checking would not catch this, but
Ivy's did.
As a fix, this commit adds the declaration (but not the implementation) of
the Symbol.iterator function back to QueryList. This has no runtime effect,
so it doesn't affect tree-shaking of QueryList, but it ensures that
QueryList is assignable to NgIterable and thus usable with NgFor.
Fixes#29842
PR Close#33536
Previously, the compiler assumed that all TS files logically within a
project existed under one or more "root directories". If the TS compiler
option `rootDir` or `rootDirs` was set, they would dictate the root
directories in use, otherwise the current directory was used.
Unfortunately this assumption was unfounded - it's common for projects
without explicit `rootDirs` to import from files outside the current
working directory. In such cases the `LogicalProjectStrategy` would attempt
to generate imports into those files, and fail. This would lead to no
`ReferenceEmitStrategy` being able to generate an import, and end in a
compiler assertion failure.
This commit introduces a new strategy to use when there are no `rootDirs`
explicitly present, the `RelativePathStrategy`. It uses simpler, filesystem-
relative paths to generate imports, even to files above the current working
directory.
Fixes#33659Fixes#33562
PR Close#33828
This commit adds the ability to change directories using the compiler's
internal filesystem abstraction. This is a prerequisite for writing tests
which are sensitive to the current working directory.
In addition to supporting the `chdir()` operation, this commit also fixes
`getDefaultLibLocation()` for mock filesystems to not assume `node_modules`
is in the current directory, but to resolve it similarly to how Node does
by progressively looking higher in the directory tree.
PR Close#33828
Adds support for chaining of `styleProp`, `classProp` and `stylePropInterpolateX` instructions whenever possible which should help generate less code. Note that one complication here is for `stylePropInterpolateX` instructions where we have to break into multiple chains if there are other styling instructions inbetween the interpolations which helps maintain the execution order.
PR Close#33837
Since i18n messages are mapped to `$localize` tagged template strings,
the "raw" version must be properly escaped. Otherwise TS will throw an
error such as:
```
Error: Debug Failure. False expression: Expected argument 'text' to be the normalized (i.e. 'cooked') version of argument 'rawText'.
```
This commit ensures that we properly escape these raw strings before creating
TS AST nodes from them.
PR Close#33820
The `:` char is used as a metadata marker in `$localize` messages.
If this char appears in the metadata it must be escaped, as `\:`.
Previously, although the `:` char was being escaped, the TS AST
being generated was not correct and so it was being output double
escaped, which meant that it appeared in the rendered message.
As of TS 3.6.2 the "raw" string can be specified when creating tagged
template AST nodes, so it is possible to correct this.
PR Close#33820
The following files are consumed only by the language service and do not
have to be in compiler-cli:
1. expression_diagnostics.ts
2. expression_type.ts
3. typescript_symbols.ts
4. symbols.ts
PR Close#33809
Previously, due to a bug a `Context` with `isStatement: false` could be
returned in places where a `Context` with `isStatement: true` was
requested. As a result, some statements would be unnecessarily wrapped
in parenthesis.
This commit fixes the bug in `Context#withStatementMode` to always
return a `Context` with the correct `isStatement` value. Note that this
does not have any impact on the generated code other than avoiding some
superfluous parenthesis on certain statements.
PR Close#33514
Currently if a consumer does something like the following, the object literal will be shared across the two elements and any instances of the component template. The same applies to array literals:
```
<div [someDirective]="{}"></div>
<div [someDirective]="{}"></div>
```
These changes make it so that we generate a pure function even if an object is constant so that each instance gets its own object.
Note that the original design for this fix included moving the pure function factories into the `consts` array. In the process of doing so I realized that pure function are also used inside of directive host bindings which means that we don't have access to the `consts`.
These changes also:
* Fix an issue that meant that the `pureFunction0` instruction could only be run during creation mode.
* Make the `getConstant` utility slightly more convenient to use. This isn't strictly required for these changes to work, but I had made it as a part of a larger refactor that I ended up reverting.
PR Close#33705
During incremental compilations, ngtsc needs to know which metadata
from a previous compilation can be reused, versus which metadata has to
be recomputed as some dependency was updated. Changes to
directives/components should cause the NgModule in which they are
declared to be recompiled, as the NgModule's compilation is dependent
on its directives/components.
When a dependent source file of a directive/component is updated,
however, a more subtle dependency should also cause to NgModule's source
file to be invalidated. During the reconciliation of state from a
previous compilation into the new program, the component's source file
is invalidated because one of its dependency has changed, ergo the
NgModule needs to be invalidated as well. Up until now, this implicit
dependency was not imposed on the NgModule. Additionally, any change to
a dependent file may influence the module scope to change, so all
components within the module must be invalidated as well.
This commit fixes the bug by introducing additional file dependencies,
as to ensure a proper rebuild of the module scope and its components.
Fixes#32416
PR Close#33522
When the Angular compiler is operated through the ngc binary in watch
mode, changing a template in an external file would not cause the
component to be recompiled if Ivy is enabled.
There was a problem with how a cached compiler host was present that was
unaware of the changed resources, therefore failing to trigger a
recompilation of a component whenever its template changes. This commit
fixes the issue by ensuring that information about modified resources is
correctly available to the cached compiler host.
Fixes#32869
PR Close#33551
Chains multiple listener instructions on a particular element into a single call which results in less generated code. Also handles listeners on templates, host listeners and synthetic host listeners.
PR Close#33720
When the Angular compiler is operated through the ngc binary in watch
mode, changing a template in an external file would not cause the
component to be recompiled if Ivy is enabled.
There was a problem with how a cached compiler host was present that was
unaware of the changed resources, therefore failing to trigger a
recompilation of a component whenever its template changes. This commit
fixes the issue by ensuring that information about modified resources is
correctly available to the cached compiler host.
Fixes#32869
PR Close#33551
This commit moves nested i18n section detection to an earlier stage where we convert HTML AST to Ivy AST. This also gives a chance to produce better diagnistic message for nested i18n sections, that also includes a file name and location.
PR Close#33583
These exports are no longer used by the CLI since 7.1.0. Since major versions of the CLI are now locked to major versions of the framework, a CLI user will not be able to use FW 9.0+ on an outdated version (<7.1.0) of the CLI that uses these old APIs.
PR Close#33242
Previously the compiler would crash if a pipe was encountered which did not
match any pipe in the scope of a template.
This commit introduces a new diagnostic error for unknown pipes instead.
PR Close#33454
Previously the template binder would crash when encountering an unknown
localref (# reference) such as `<div #ref="foo">` when no directive has
`exportAs: "foo"`.
With this commit, the compiler instead generates a template diagnostic error
informing the user about the invalid reference.
PR Close#33454
Previously declarations that were imported via a namespace import
were given the same `bestGuessOwningModule` as the context
where they were imported to. This causes problems with resolving
`ModuleWithProviders` that have a type that has been imported in
this way, causing errors like:
```
ERROR in Symbol UIRouterModule declared in
.../@uirouter/angular/uiRouterNgModule.d.ts
is not exported from
.../@uirouter/angular/uirouter-angular.d.ts
(import into .../src/app/child.module.ts)
```
This commit modifies the `TypescriptReflectionHost.getDirectImportOfIdentifier()`
method so that it also understands how to attach the correct `viaModule` to
the identifier of the namespace import.
Resolves#32166
PR Close#33495
Now that we've replaced `ngBaseDef` with an abstract directive definition, there are a lot more cases where we generate a directive definition without a selector. These changes make it so that we don't generate the `selectors` array if it's going to be empty.
PR Close#33431
Removes `ngBaseDef` from the compiler and any runtime code that was still referring to it. In the cases where we'd previously generate a base def we now generate a definition for an abstract directive.
PR Close#33264
Andrew Kushnir
JoostK
Alex Rickabaugh
Kristiyan Kostadinov
Pete Bacon Darwin
Misko Hevery
Keen Yee Liau
George Kalpakas
Andrew Scott
Charles Lyding