Commit Graph

1804 Commits

Author SHA1 Message Date
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
Alex Rickabaugh 3a525d196b Revert "fix(compiler): mark `NgModuleFactory` construction as not side effectful (#38147)" (#38303)
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
2020-07-30 12:19:35 -07:00
Andrew Kushnir 1eebb7f189 test(compiler-cli): disable one TypeChecker test on Windows due to path sensitivity issue (#38294)
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
2020-07-29 17:49:45 -07:00
Doug Parker 7f8c2225f2 fix(compiler): mark `NgModuleFactory` construction as not side effectful (#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. 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
2020-07-29 13:32:08 -07:00
Doug Parker 887c350f9d refactor(compiler): wrap large strings in function (#38253)
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
2020-07-29 13:31:03 -07:00
Alex Rickabaugh d8c07b83c3 refactor(compiler-cli): support type-checking a single component (#38105)
This commit adds a method `getDiagnosticsForComponent` to the
`TemplateTypeChecker`, which does the minimum amount of work to retrieve
diagnostics for a single component.

With the normal `ReusedProgramStrategy` this offers virtually no improvement
over the standard `getDiagnosticsForFile` operation, but if the
`TypeCheckingProgramStrategy` supports separate shims for each component,
this operation can yield a faster turnaround for components that are
declared in files with many other components.

PR Close #38105
2020-07-29 10:31:21 -07:00
Alex Rickabaugh 8f73169979 refactor(compiler-cli): add `TemplateId` to template diagnostics (#38105)
Previously, a stable template id was implemented for each component in a
file. This commit adds this id to each `TemplateDiagnostic` generated from
the template type-checker, so it can potentially be used for filtration.

PR Close #38105
2020-07-29 10:31:20 -07:00
Alex Rickabaugh 3c0424e7e0 refactor(compiler-cli): allow overriding templates in the type checker (#38105)
This commit adds an `overrideComponentTemplate` operation to the template
type-checker. This operation changes the template used during template
type-checking operations.

Overriding a template causes any previous work for it to be discarded, and
the template type-checking engine will regenerate the TCB for that template
on the next request.

This operation can be used by a consumer such as the language service to
get rapid feedback or diagnostics as the user is editing a template file,
without the need for a full incremental build iteration.

Closes #38058

PR Close #38105
2020-07-29 10:31:20 -07:00
Alex Rickabaugh de14b2c670 refactor(compiler-cli): efficient single-file type checking diagnostics (#38105)
Previously, the `TemplateTypeChecker` abstraction allowed fetching
diagnostics for a single file, but under the hood would generate type
checking code for the entire program to satisfy the request.

With this commit, an `OptimizeFor` hint is passed to `getDiagnosticsForFile`
which indicates whether the user intends to request diagnostics for the
whole program or is truly interested in just the single file. If the latter,
the `TemplateTypeChecker` can perform only the work needed to produce
diagnostics for just that file, thus returning answers more efficiently.

PR Close #38105
2020-07-29 10:31:20 -07:00
Alex Rickabaugh c573d91285 refactor(compiler-cli): allow program strategies to opt out of inlining (#38105)
The template type-checking engine relies on the abstraction interface
`TypeCheckingProgramStrategy` to create updated `ts.Program`s for
template type-checking. The basic API is that the type-checking engine
requests changes to certain files in the program, and the strategy provides
an updated `ts.Program`.

Typically, such changes are made to 'ngtypecheck' shim files, but certain
conditions can cause template type-checking to require "inline" operations,
which change user .ts files instead. The strategy used by 'ngc' (the
`ReusedProgramStrategy`) supports these kinds of updates, but other clients
such as the language service might not always support modifying user files.

To accommodate this, the `TypeCheckingProgramStrategy` interface was
modified to include a `supportsInlineOperations` flag. If an implementation
specifies `false` for inline support, the template type-checking system will
return diagnostics on components which would otherwise require inline
operations.

Closes #38059

PR Close #38105
2020-07-29 10:31:20 -07:00
Alex Rickabaugh 16c7441c2f refactor(compiler-cli): introduce the TemplateTypeChecker abstraction (#38105)
This commit significantly refactors the 'typecheck' package to introduce a
new abstraction, the `TemplateTypeChecker`. To achieve this:

* a 'typecheck:api' package is introduced, containing common interfaces that
  consumers of the template type-checking infrastructure can depend on
  without incurring a dependency on the template type-checking machinery as
  a whole.
* interfaces for `TemplateTypeChecker` and `TypeCheckContext` are introduced
  which contain the abstract operations supported by the implementation
  classes `TemplateTypeCheckerImpl` and `TypeCheckContextImpl` respectively.
* the `TemplateTypeChecker` interface supports diagnostics on a whole
  program basis to start with, but the implementation is purposefully
  designed to support incremental diagnostics at a per-file or per-component
  level.
* `TemplateTypeChecker` supports direct access to the type check block of a
  component.
* the testing utility is refactored to be a lot more useful, and new tests
  are added for the new abstraction.

PR Close #38105
2020-07-29 10:31:20 -07:00
Alex Rickabaugh 736f6337b2 refactor(compiler-cli): make file/shim split 1:n instead of 1:1 (#38105)
Previously in the template type-checking engine, it was assumed that every
input file would have an associated type-checking shim. The type check block
code for all components in the input file would be generated into this shim.

This is fine for whole-program type checking operations, but to support the
language service's requirements for low latency, it would be ideal to be
able to check a single component in isolation, especially if the component
is declared along with many others in a single file.

This commit removes the assumption that the file/shim mapping is 1:1, and
introduces the concept of component-to-shim mapping. Any
`TypeCheckingProgramStrategy` must provide such a mapping.

To achieve this:

 * type checking record information is now split into file-level data as
   well as per-shim data.
 * components are now assigned a stable `TemplateId` which is unique to the
   file in which they're declared.

PR Close #38105
2020-07-29 10:31:20 -07:00
Andrea Canciani 9c8bc4a239 fix(common): narrow `NgIf` context variables in template type checker (#36627)
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
2020-07-29 10:30:44 -07:00
Andrew Scott 65cc0c8bd6 fix(compiler-cli): Add support for string literal class members (#38226)
The current implementation of the TypeScriptReflectionHost does not account for members that
are string literals, i.e. `class A { 'string-literal-prop': string; }`

PR Close #38226
2020-07-27 15:26:27 -07:00
Andrew Kushnir 8e5969bb52 fix(compiler): share identical stylesheets between components in the same file (#38213)
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
2020-07-27 10:04:30 -07:00
Andrew Kushnir 2fdc18b42c refactor(compiler): separate compilation and transform phases (#38213)
This commit splits the transformation into 2 separate steps: Ivy compilation and actual transformation
of corresponding TS nodes. This is needed to have all `o.Expression`s generated before any TS transforms
happen. This allows `ConstantPool` to properly identify expressions that can be shared across multiple
components declared in the same file.

Resolves #38203.

PR Close #38213
2020-07-27 10:04:30 -07:00
Andrew Kushnir d72b1e44c6 refactor(core): rename synthetic host property and listener instructions (#37145)
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
2020-07-21 09:09:24 -07:00
crisbeto bf641e1b4b fix(core): incorrectly validating properties on ng-content and ng-container (#37773)
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
2020-07-15 12:39:39 -07:00
Pete Bacon Darwin b358495a6c fix(ngcc): report a warning if ngcc tries to use a solution-style tsconfig (#38003)
In CLI v10 there was a move to use the new solution-style tsconfig
which became available in TS 3.9.

The result of this is that the standard tsconfig.json no longer contains
important information such as "paths" mappings, which ngcc might need to
correctly compute dependencies.

ngcc (and ngc and tsc) infer the path to tsconfig.json if not given an
explicit tsconfig file-path. But now that means it infers the solution
tsconfig rather than one that contains the useful information it used to
get.

This commit logs a warning in this case to inform the developer
that they might not have meant to load this tsconfig and offer
alternative options.

Fixes #36386

PR Close #38003
2020-07-14 13:21:31 -07:00
Pete Bacon Darwin 6b311552f0 fix(compiler-cli): ensure file_system handles mixed Windows drives (#37959)
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
2020-07-13 12:05:21 -07:00
crisbeto 9322b9a060 fix(compiler): check more cases for pipe usage inside host bindings (#37883)
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
2020-07-10 11:00:20 -07:00
Paul Gschwendtner 1550663b9e fix(bazel): ng_module rule does not expose flat module information in Ivy (#36971)
The `ng_module` rule supports the generation of flat module bundles. In
View Engine, information about this flat module bundle is exposed
as a Bazel provider. This is helpful as other rules like `ng_package`
could rely on this information to determine entry-points for the APF.

With Ivy this currently does not work because the flat module
information is not exposed in the provider. The reason for this is
unclear. We should also provide this information in Ivy so that rules
like `ng_package` can also determine the correct entry-points when a
package is built specifically with `--config=ivy`.

PR Close #36971
2020-07-09 22:11:17 +00:00
Olivier Combe 6eb868b63a build(compiler-cli): fix bazel deps rules for ngtsc testing packages (#37977)
The ngtsc testing packages for file_system and logging were missing from the bazel deps rules, which means that they were not included in the releases

PR Close #37977
2020-07-08 12:05:22 -07:00
Andrew Kushnir aed6b131bb fix(core): handle spaces after `select` and `plural` ICU keywords (#37866)
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
2020-07-06 13:55:47 -07:00
Alex Rickabaugh 71956250dd perf(compiler-cli): fix memory leak in retained incremental state (#37835)
Incremental compilation allows for the output state of one compilation to be
reused as input to the next compilation. This involves retaining references
to instances from prior compilations, which must be done carefully to avoid
memory leaks.

This commit fixes such a leak with a complicated retention chain:

* `TrackedIncrementalBuildStrategy` unnecessarily hangs on to the previous
  `IncrementalDriver` (state of the previous compilation) once the current
  compilation completes.

  In general this is unnecessary, but should be safe as long as the chain
  only goes back one level - if the `IncrementalDriver` doesn't retain any
  previous `TrackedIncrementalBuildStrategy` instances. However, this does
  happen:

* `NgCompiler` indirectly causes retention of previous `NgCompiler`
  instances (and thus previous `TrackedIncrementalBuildStrategy` instances)
  through accidental capture of the `this` context in a closure created in
  its constructor. This closure is wrapped in a `ts.ModuleResolutionCache`
  used to create a `ModuleResolver` class, which is passed to the program's
  `TraitCompiler` on construction.

* The `IncrementalDriver` retains a reference to the `TraitCompiler` of the
  previous compilation, completing the reference chain.

The final retention chain thus looks like:

* `TrackedIncrementalBuildStrategy` of current program
* `.previous`: `IncrementalDriver` of previous program
* `.lastGood.traitCompiler`: `TraitCompiler`
* `.handlers[..].moduleResolver.moduleResolutionCache`: cache
* (via `getCanonicalFileName` closure): `NgCompiler`
* `.incrementalStrategy`: `TrackedIncrementalBuildStrategy` of previous
  program.

The closure link is the "real" leak here. `NgCompiler` is creating a closure
for `getCanonicalFileName`, delegating to its
`this.adapter.getCanonicalFileName`, for the purposes of creating a
`ts.ModuleResolutionCache`. The fact that the closure references
`NgCompiler` thus eventually causes previous `NgCompiler` iterations to be
retained. This is also potentially problematic due to the shared nature of
`ts.ModuleResolutionCache`, which is potentially retained across multiple
compilations intentionally.

This commit fixes the first two links in the retention chain: the build
strategy is patched to not retain a `previous` pointer, and the `NgCompiler`
is patched to not create a closure in the first place, but instead pass a
bound function. This ensures that the `NgCompiler` does not retain previous
instances of itself in the first place, even if the build strategy does
end up retaining the previous incremental state unnecessarily.

The third link (`IncrementalDriver` unnecessarily retaining the whole
`TraitCompiler`) is not addressed in this commit as it's a more
architectural problem that will require some refactoring. However, the leak
potential of this retention is eliminated thanks to fixing the first two
issues.

PR Close #37835
2020-06-29 16:34:51 -07:00
JoostK a87951a28f fix(ngcc): prevent including JavaScript sources outside of the package (#37596)
When ngcc creates an entry-point program, the `allowJs` option is enabled
in order to operate on the JavaScript source files of the entry-point.
A side-effect of this approach is that external modules that don't ship
declaration files will also have their JavaScript source files loaded
into the program, as the `allowJs` flag allows for them to be imported.
This may pose an issue in certain edge cases, where ngcc would inadvertently
operate on these external modules. This can introduce all sorts of undesirable
behavior and incompatibilities, e.g. the reflection host that is selected for
the entry-point's format could be incompatible with that of the external
module's JavaScript bundles.

To avoid these kinds of issues, module resolution that would resolve to
a JavaScript file located outside of the package will instead be rejected,
as if the file would not exist. This would have been the behavior when
`allowJs` is set to false, which is the case in typical Angular compilations.

Fixes #37508

PR Close #37596
2020-06-29 12:21:22 -07:00
JoostK 6b565ba8f2 refactor(ngcc): let `isWithinPackage` operate on paths instead of source files (#37596)
Changes `isWithinPackage` to take an `AbsoluteFsPath` instead of `ts.SourceFile`,
to allow for an upcoming change to use it when no `ts.SourceFile` is available,
but just a path.

PR Close #37596
2020-06-29 12:21:22 -07:00
Pete Bacon Darwin 78a44768e0 fix(ngcc): ensure lockfile is removed when analyzeFn fails (#37739)
Previously an error thrown in the `analyzeFn` would cause
the ngcc process to exit immediately without removing the
lockfile, and potentially before the unlocker process had been
successfully spawned resulting in the lockfile being orphaned
and left behind.

Now we catch these errors and remove the lockfile as needed.

PR Close #37739
2020-06-29 10:29:11 -07:00
Keen Yee Liau e0eeb4afcb refactor(compiler-cli): Remove any cast for CompilerHost (#37079)
This commit removes the FIXME for casting CompilerHost to any since
google3 is now already on TS 3.8.

PR Close #37079
2020-06-26 11:08:17 -07:00
Alex Rickabaugh 2cbc429291 test(compiler-cli): disable DynamicValue diagnostic tests on Windows (#37763)
This commit disables all diagnostic tests for DynamicValue diagnostics which
make assertions about the diagnostic filename while running tests on Windows.

Such assertions are currently suffering from a case sensitivity issue.

PR Close #37763
2020-06-25 17:04:57 -07:00
Alex Rickabaugh 8572e885b4 test(compiler-cli): fix assertion of diagnostic filename on Windows (#37758)
Several partial_evaluator tests in the diagnostics_spec check assert
correctness of diagnostic filenames. Previously these assertions compared
a resolved (`absoluteFrom`) filename with the TypeScript `ts.SourceFile`'s
`fileName` string, which caused the tests to fail on Windows because the
drive letter case differed.

This commit changes the assertions to use `absoluteFromSourceFile` instead
of the `fileName` string, resulting in an apples-to-apples comparison of
canonicalized paths.

PR Close #37758
2020-06-25 15:40:43 -07:00
JoostK ce879fc416 refactor(compiler-cli): more accurate reporting of complex function call (#37587)
This commit introduces a dedicated `DynamicValue` kind to indicate that a value
cannot be evaluated statically as the function body is not just a single return
statement. This allows more accurate reporting of why a function call failed
to be evaluated, i.e. we now include a reference to the function declaration
and have a tailor-made diagnostic message.

PR Close #37587
2020-06-25 14:16:35 -07:00
JoostK 712f1bd0b7 feat(compiler-cli): explain why an expression cannot be used in AOT compilations (#37587)
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
2020-06-25 14:16:35 -07:00
JoostK d2fb552116 refactor(compiler-cli): create diagnostics using `ts.DiagnosticRelatedInformation` (#37587)
Previously, an anonymous type was used for creating a diagnostic with related
information. The anonymous type would then be translated into the necessary
`ts.DiagnosticRelatedInformation` shape within `makeDiagnostic`. This commit
switches the `makeDiagnostic` signature over to taking `ts.DiagnosticRelatedInformation`
directly and introduces `makeRelatedInformation` to easily create such objects.
This is done to aid in making upcoming work more readable.

PR Close #37587
2020-06-25 14:16:35 -07:00
Alex Rickabaugh 5103d908c8 perf(compiler-cli): fix regressions in incremental program reuse (#37641)
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
2020-06-25 14:12:20 -07:00
Pete Bacon Darwin 9318e23e64 perf(ngcc): use `EntryPointManifest` to speed up noop `ProgramBaseEntryPointFinder` (#37665)
Previously the `ProgramBasedEntryPointFinder` was parsing all the
entry-points referenced by the program for dependencies even if all the
entry-points had been processed already.

Now this entry-point finder will re-use the `EntryPointManifest` to load
the entry-point dependencies when possible which avoids having to parse
them all again, on every invocation of ngcc.

Previously the `EntryPointManifest` was only used in the
`DirectoryWalkerEntryPointFinder`, which also contained the logic for
computing the contents of the manifest. This logic has been factored out
into an `EntryPointCollector` class. Both the `ProgramBasedEntryPointFinder`
and `DirectoryWalkerEntryPointFinder` now use the `EntryPointManifest` and
the `EntryPointCollector`.

The result of this change is that there is a small cost on the first run of
ngcc to compute and store the manifest - the processing takes 102% of the
processing time before this PR. But on subsequent runs there is a
significant benefit on subsequent runs - the processing takes around 50%
of the processing time before this PR.

PR Close #37665
2020-06-25 14:11:03 -07:00
Pete Bacon Darwin 07a07e34bc fix(compiler-cli): only read source-map comment from last line (#32912)
Source-maps can be linked to from a source-file by a comment at
the end of the file.

Previously the `SourceFileLoader` would read
the first comment that matched `//# sourceMappingURL=` but
this is not valid since some bundlers may include embedded
source-files that contain such a comment.

Now we only look for this comment in the last non-empty line
in the file.

PR Close #32912
2020-06-25 14:10:03 -07:00
Pete Bacon Darwin dda3f49952 refactor(compiler): add source-map spans to localized strings (#32912)
Previously localized strings were not mapped to their original
source location, so it was not possible to back-trace them
in tools like the i18n message extractor.

PR Close #32912
2020-06-25 14:10:03 -07:00
Pete Bacon Darwin decd95e7f0 fix(compiler-cli): ensure source-maps can handle webpack:// protocol (#32912)
Webpack and other build tools sometimes inline the contents of the
source files in their generated source-maps, and at the same time
change the paths to be prefixed with a protocol, such as `webpack://`.

This can confuse tools that need to read these paths, so now it is
possible to provide a mapping to where these files originated.

PR Close #32912
2020-06-25 14:10:03 -07:00
Pete Bacon Darwin 6abb8d0d91 feat(compiler-cli): add `SourceFile.getOriginalLocation()` to sourcemaps package (#32912)
This method will allow us to find the original location given a
generated location, which is useful in fine grained work with
source-mapping. E.g. in `$localize` tooling.

PR Close #32912
2020-06-25 14:10:03 -07:00
Pete Bacon Darwin bedc0451a0 docs(ngcc): add additional next steps to an error (#37672)
The file-writing error in the this commit can also be the result
of the ngcc process dying in the middle of writing files.

This commit improves the error message to offer a resolution
in case this is the reason for the error.

Fixes #36393

PR Close #37672
2020-06-25 11:37:43 -07:00
Pete Bacon Darwin 5c40fd65fa refactor(compiler-cli): tidy up file_system BUILD.bazel file_system (#37114)
* Re-order the `load` and `package` statements
* Make `srcs` glob more generic
* Remove unnecessary dependencies

PR Close #37114
2020-06-22 13:38:47 -07:00
Pete Bacon Darwin 2b53b07c70 refactor(ngcc): move `sourcemaps` into `ngtsc` (#37114)
The `SourceFile` and associated code is general and reusable in
other projects (such as `@angular/localize`). Moving it to `ngtsc`
makes it more easily shared.

PR Close #37114
2020-06-22 13:38:47 -07:00
Pete Bacon Darwin 6de5a12a9d refactor(ngcc): move `logging` code into `ngtsc` (#37114)
The `Logger` interface and its related classes are general purpose
and could be used by other tooling. Moving it into ngtsc is a more
suitable place from which to share it - similar to the FileSystem stuff.

PR Close #37114
2020-06-22 13:38:47 -07:00
Pete Bacon Darwin bd7f440357 perf(ngcc): shortcircuit tokenizing in ESM dependency host (#37639)
This dependency host tokenizes files to identify all the imported
paths. This commit calculates the last place in the source code
where there can be an import path; it then exits the tokenization
when we get to this point in the file.

Testing with a reasonably large project showed that the tokenizer
spends about 2/3 as much time scanning files. For example in a
"noop" hot run of ngcc using the program-based entry-point
finder the percentage of time spent in the `scan()` function of
the TS tokenizer goes down from 9.9% to 6.6%.

PR Close #37639
2020-06-18 16:05:42 -07:00