Commit Graph

1848 Commits

Author SHA1 Message Date
JoostK f0688b4d18 perf(ngcc): introduce cache for sharing data across entry-points (#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
2020-09-15 11:23:04 -07:00
JoostK 297c060ae7 perf(compiler-cli): optimize computation of type-check scope information (#38539)
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
2020-09-14 11:54:40 -07:00
JoostK 077f51685a perf(compiler-cli): only emit directive/pipe references that are used (#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
2020-09-14 11:54:37 -07:00
Andrew Scott 2d52c80332 test(compiler): Add back tests for renamed inputs and outputs (#38798)
#38685 corrected the confusion between field and property names so the consumer can
now be determined correctly.

PR Close #38798
2020-09-10 14:33:10 -07:00
Andrew Scott 19598b47ca feat(compiler-cli): add ability to get symbol of reference or variable (#38618)
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
2020-09-10 12:40:50 -07:00
Andrew Scott f56ece4fdc feat(compiler-cli): Add ability to get `Symbol` of AST expression in component template (#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
2020-09-10 12:40:47 -07:00
Andrew Scott cf2e8b99a8 feat(compiler-cli): Add ability to get `Symbol` of `Template`s and `Element`s in component template (#38618)
Adds support to the `TemplateTypeChecker` for retrieving a `Symbol` for
`TmplAstTemplate` and `TmplAstElement` nodes in a component template.

PR Close #38618
2020-09-10 12:40:44 -07:00
Andrew Scott c4556db9f5 feat(compiler-cli): `TemplateTypeChecker` operation to get `Symbol` from a template node (#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
2020-09-10 12:40:41 -07:00
Andrew Scott a46e0e48a3 refactor(compiler-cli): Adjust output of TCB to support `TemplateTypeChecker` Symbol retrieval (#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
2020-09-10 12:40:38 -07:00
Andrew Scott 9e77bd3087 feat(compiler-cli): define interfaces to be used for TemplateTypeChecker (#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
2020-09-10 12:40:35 -07:00
Andrew Scott 18f84a0328 Revert "perf(compiler-cli): only emit directive/pipe references that are used (#38539)" (#38765)
This reverts commit 4faac78e32.
internal failure:
https://test.corp.google.com/ui#id=OCL:329948619:BASE:329967516:1599160428139:d63165ae

PR Close #38765
2020-09-09 12:21:22 -07:00
Andrew Scott b0ca3cd0c4 Revert "perf(compiler-cli): optimize computation of type-check scope information (#38539)" (#38765)
This reverts commit ba95b79a21.
internal failure:
https://test.corp.google.com/ui#id=OCL:329948619:BASE:329967516:1599160428139:d63165ae

PR Close #38765
2020-09-09 12:21:22 -07:00
JoostK ba95b79a21 perf(compiler-cli): optimize computation of type-check scope information (#38539)
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
2020-09-08 14:50:38 -07:00
JoostK 4faac78e32 perf(compiler-cli): only emit directive/pipe references that are used (#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
2020-09-08 14:50:38 -07:00
JoostK a32a317ea1 fix(compiler-cli): ensure that a declaration is available in type-to-value conversion (#38684)
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
2020-09-08 14:06:25 -07:00
Pete Bacon Darwin 7e0b3fd953 fix(compiler-cli): compute source-mappings for localized strings (#38645)
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
2020-09-08 13:17:21 -07:00
Pete Bacon Darwin 687477279b refactor(compiler): move `ParsedTemplate` interface to compiler (#38594)
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
2020-09-08 11:43:25 -07:00
Alex Rickabaugh 4007422cc6 fix(compiler): correct confusion between field and property names (#38685)
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
2020-09-08 11:43:02 -07:00
Pete Bacon Darwin 7869de6136 fix(ngcc): use aliased exported types correctly (#38666)
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
2020-09-08 11:41:21 -07:00
Pete Bacon Darwin 2c4a98a285 fix(localize): do not expose NodeJS typings in $localize runtime code (#38700)
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
2020-09-08 11:40:58 -07:00
Alex Rickabaugh c90eb5450d refactor(compiler-cli): make template parsing errors into diagnostics (#38576)
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
2020-09-03 14:02:35 -07:00
Alex Rickabaugh 3e97435f1c refactor(compiler-cli): split out template diagnostics package (#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
2020-09-03 14:02:31 -07:00
Pete Bacon Darwin 1d8c5d88cd refactor(compiler): `element.sourceSpan` should span the `outerHTML` (#38581)
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
2020-09-02 14:47:31 -07:00
Pete Bacon Darwin 86e11f1110 refactor(compiler): move the line-ending handling decision (#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
2020-09-02 14:47:25 -07:00
crisbeto f5a148b1b7 fix(compiler): incorrectly inferring namespace for HTML nodes inside SVG (#38477)
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
2020-08-31 13:25:38 -07:00
Alan Agius 281b647f15 refactor(compiler-cli): remove usage of `ts.updateIdentifier` (#38076)
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
2020-08-24 13:07:02 -07:00
Alan Agius 0fc44e0436 feat(compiler-cli): add support for TypeScript 4.0 (#38076)
With this change we add support for TypeScript 4.0

PR Close #38076
2020-08-24 13:06:59 -07:00
JoostK 874792dc43 feat(compiler): support unary operators for more accurate type checking (#37918)
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 #20845
Fixes #36178

PR Close #37918
2020-08-21 12:25:53 -07:00
crisbeto e7da4040d6 fix(compiler-cli): adding references to const enums in runtime code (#38542)
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
2020-08-21 12:23:21 -07:00
Alex Rickabaugh 0b54c0c6b4 refactor(compiler-cli): add getTemplateOfComponent to TemplateTypeChecker (#38355)
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
2020-08-19 14:07:03 -07:00
Joey Perrott e472f5f688 refactor(ngcc): update yargs and typings for yargs (#38470)
Updating yargs and typings for the updated yargs module.

PR Close #38470
2020-08-17 15:30:33 -07:00
Paul Gschwendtner 3b9c802dee fix(ngcc): detect synthesized delegate constructors for downleveled ES2015 classes (#38463)
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
2020-08-17 10:55:40 -07:00
Andrew Kushnir cb05c0102f fix(core): move generated i18n statements to the `consts` field of ComponentDef (#38404)
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
2020-08-17 10:13:57 -07:00
Andrew Kushnir 5f90b64328 refactor(compiler): i18n compiler tests refactoring (#38404)
This commit refactors i18n compiler tests to avoid code duplication and simplify further maintenance and updates.

PR Close #38404
2020-08-17 10:13:55 -07:00
Ahn d5f819ebc1 style(compiler-cli): remove unused constant (#38441)
Remove unused constant allDiagnostics

PR Close #38441
2020-08-13 13:32:41 -07:00
JoostK 1388c1761f perf(compiler-cli): don't emit template guards when child scope is empty (#38418)
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
2020-08-13 13:28:46 -07:00
JoostK fb8f4b4d72 perf(compiler-cli): only generate directive declarations when used (#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
2020-08-13 13:28:44 -07:00
JoostK f42e6ce917 perf(compiler-cli): only generate type-check code for referenced DOM elements (#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
2020-08-13 13:28:42 -07:00
Andrew Scott 71138f6004 feat(compiler-cli): Add compiler option to report errors when assigning to restricted input fields (#38249)
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
2020-08-11 09:55:48 -07:00
JoostK fa0104017a refactor(compiler-cli): only use type constructors for directives with generic types (#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
2020-08-11 09:55:48 -07:00
JoostK 80b67e02b7 fix(compiler-cli): infer quote expressions as any type in type checker (#37917)
"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
2020-08-11 09:54:53 -07:00
JoostK 18098d38b8 fix(compiler-cli): avoid creating value expressions for symbols from type-only imports (#37912)
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
2020-08-11 09:53:25 -07:00
JoostK 9514fd9080 fix(compiler): evaluate safe navigation expressions in correct binding order (#37911)
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
2020-08-11 09:51:10 -07:00
crisbeto 6da9e5851a fix(compiler-cli): preserve quotes in class member names (#38387)
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
2020-08-10 15:26:45 -07:00
JoostK 7525f3afc1 fix(compiler-cli): type-check inputs that include undefined when there's coercion members (#38273)
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
2020-08-06 15:21:02 -07:00
Doug Parker dca4443a8e fix(compiler-cli): mark eager `NgModuleFactory` construction as not side effectful (#38320)
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
2020-08-06 09:02:16 -07:00
Doug Parker 2a745c8df8 refactor(compiler): add `ModuleInfo` interface (#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
2020-08-06 09:02:16 -07:00
Doug Parker a18f82b458 refactor(core): add `noSideEffects()` as private export (#38320)
This is to enable the compiler to generate `noSideEffects()` calls. This is a private export, gated by `ɵ`.

PR Close #38320
2020-08-06 09:02:16 -07:00
Charles Lyding ba175be41f fix(compiler-cli): match wrapHost parameter types within plugin interface (#38004)
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
2020-08-05 10:54:07 -07:00
Charles Lyding 6f6102d8ad fix(compiler): add PURE annotation to getInheritedFactory calls (#38291)
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
2020-07-30 16:53:52 -07:00