12 Commits

Author SHA1 Message Date
George Kalpakas
6ab43d7335 fix(ngcc): correctly detect external files from nested node_modules/ (#36559)
Previously, when we needed to detect whether a file is external to a
package, we only checked whether the relative path to the file from the
package's root started with `..`. This would detect external imports
when the packages were siblings (e.g. peer dependencies or hoisted to
the top of `node_modules/` by the package manager), but would fail to
detect imports from packages located in nested `node_modules/` as
external. For example, importing `node_modules/foo/node_modules/bar`
from a file in `node_modules/foo/` would be considered internal to the
`foo` package.

This could result in processing/analyzing more files than necessary.
More importantly it could lead to errors due to trying to analyze
non-Angular packages that were direct dependencies of Angular packages.

This commit fixes it by also verifying that the relative path to a file
does not start with `node_modules/`.

Jira issue: [FW-2068](https://angular-team.atlassian.net/browse/FW-2068)

Fixes #36526

PR Close #36559
2020-04-10 09:10:26 -07:00
Pete Bacon Darwin
8be8466a00 style(ngcc): reformat of ngcc after clang update (#36447)
PR Close #36447
2020-04-06 09:26:57 -07:00
JoostK
7659f2e24b fix(ngcc): do not attempt compilation when analysis fails (#34889)
In #34288, ngtsc was refactored to separate the result of the analysis
and resolve phase for more granular incremental rebuilds. In this model,
any errors in one phase transition the trait into an error state, which
prevents it from being ran through subsequent phases. The ngcc compiler
on the other hand did not adopt this strict error model, which would
cause incomplete metadata—due to errors in earlier phases—to be offered
for compilation that could result in a hard crash.

This commit updates ngcc to take advantage of ngtsc's `TraitCompiler`,
that internally manages all Ivy classes that are part of the
compilation. This effectively replaces ngcc's own `AnalyzedFile` and
`AnalyzedClass` types, together with all of the logic to drive the
`DecoratorHandler`s. All of this is now handled in the `TraitCompiler`,
benefiting from its explicit state transitions of `Trait`s so that the
ngcc crash is a thing of the past.

Fixes #34500
Resolves FW-1788

PR Close #34889
2020-01-23 14:47:03 -08:00
Alex Rickabaugh
74edde0a94 perf(ivy): reuse prior analysis work during incremental builds (#34288)
Previously, the compiler performed an incremental build by analyzing and
resolving all classes in the program (even unchanged ones) and then using
the dependency graph information to determine which .js files were stale and
needed to be re-emitted. This algorithm produced "correct" rebuilds, but the
cost of re-analyzing the entire program turned out to be higher than
anticipated, especially for component-heavy compilations.

To achieve performant rebuilds, it is necessary to reuse previous analysis
results if possible. Doing this safely requires knowing when prior work is
viable and when it is stale and needs to be re-done.

The new algorithm implemented by this commit is such:

1) Each incremental build starts with knowledge of the last known good
   dependency graph and analysis results from the last successful build,
   plus of course information about the set of files changed.

2) The previous dependency graph's information is used to determine the
   set of source files which have "logically" changed. A source file is
   considered logically changed if it or any of its dependencies have
   physically changed (on disk) since the last successful compilation. Any
   logically unchanged dependencies have their dependency information copied
   over to the new dependency graph.

3) During the `TraitCompiler`'s loop to consider all source files in the
   program, if a source file is logically unchanged then its previous
   analyses are "adopted" (and their 'register' steps are run). If the file
   is logically changed, then it is re-analyzed as usual.

4) Then, incremental build proceeds as before, with the new dependency graph
   being used to determine the set of files which require re-emitting.

This analysis reuse avoids template parsing operations in many circumstances
and significantly reduces the time it takes ngtsc to rebuild a large
application.

Future work will increase performance even more, by tackling a variety of
other opportunities to reuse or avoid work.

PR Close #34288
2019-12-12 13:11:45 -08:00
Alex Rickabaugh
50cdc0ac1b refactor(ivy): move analysis side effects into a register phase (#34288)
Previously 'analyze' in the various `DecoratorHandler`s not only extracts
information from the decorators on the classes being analyzed, but also has
several side effects within the compiler:

* it can register metadata about the types involved in global metadata
  trackers.
* it can register information about which .ngfactory symbols are actually
  needed.

In this commit, these side-effects are moved into a new 'register' phase,
which runs after the 'analyze' step. Currently this is a no-op refactoring
as 'register' is always called directly after 'analyze'. In the future this
opens the door for re-use of prior analysis work (with only 'register' being
called, to apply the above side effects).

Also as part of this refactoring, the reification of NgModule scope
information into the incremental dependency graph is moved to the
`NgtscProgram` instead of the `TraitCompiler` (which now only manages trait
compilation and does not have other side effects).

PR Close #34288
2019-12-12 13:11:45 -08:00
Alex Rickabaugh
252e3e9487 refactor(ivy): formalize the compilation process for matched handlers (#34288)
Prior to this commit, the `IvyCompilation` tracked the state of each matched
`DecoratorHandler` on each class in the `ts.Program`, and how they
progressed through the compilation process. This tracking was originally
simple, but had grown more complicated as the compiler evolved. The state of
each specific "target" of compilation was determined by the nullability of
a number of fields on the object which tracked it.

This commit formalizes the process of compilation of each matched handler
into a new "trait" concept. A trait is some aspect of a class which gets
created when a `DecoratorHandler` matches the class. It represents an Ivy
aspect that needs to go through the compilation process.

Traits begin in a "pending" state and undergo transitions as various steps
of compilation take place. The `IvyCompilation` class is renamed to the
`TraitCompiler`, which manages the state of all of the traits in the active
program.

Making the trait concept explicit will support future work to incrementalize
the expensive analysis process of compilation.

PR Close #34288
2019-12-12 13:11:45 -08:00
Alex Rickabaugh
b381497126 feat(ngcc): add a migration for undecorated child classes (#33362)
In Angular View Engine, there are two kinds of decorator inheritance:

1) both the parent and child classes have decorators

This case is supported by InheritDefinitionFeature, which merges some fields
of the definitions (such as the inputs or queries).

2) only the parent class has a decorator

If the child class is missing a decorator, the compiler effectively behaves
as if the parent class' decorator is applied to the child class as well.
This is the "undecorated child" scenario, and this commit adds a migration
to ngcc to support this pattern in Ivy.

This migration has 2 phases. First, the NgModules of the application are
scanned for classes in 'declarations' which are missing decorators, but
whose base classes do have decorators. These classes are the undecorated
children. This scan is performed recursively, so even if a declared class
has a base class that itself inherits a decorator, this case is handled.

Next, a synthetic decorator (either @Component or @Directive) is created
on the child class. This decorator copies some critical information such
as 'selector' and 'exportAs', as well as supports any decorated fields
(@Input, etc). A flag is passed to the decorator compiler which causes a
special feature `CopyDefinitionFeature` to be included on the compiled
definition. This feature copies at runtime the remaining aspects of the
parent definition which `InheritDefinitionFeature` does not handle,
completing the "full" inheritance of the child class' decorator from its
parent class.

PR Close #33362
2019-10-25 09:16:50 -07:00
JoostK
373e1337de fix(ngcc): consistently use outer declaration for classes (#32539)
In ngcc's reflection hosts for compiled JS bundles, such as ESM2015,
special care needs to be taken for classes as there may be an outer
declaration (referred to as "declaration") and an inner declaration
(referred to as "implementation") for a given class. Therefore, there
will also be two `ts.Symbol`s bound per class, and ngcc needs to switch
between those declarations and symbols depending on where certain
information can be found.

Prior to this commit, the `NgccReflectionHost` interface had methods
`getClassSymbol` and `findClassSymbols` that would return a `ts.Symbol`.
These class symbols would be used to kick off compilation of components
using ngtsc, so it is important for these symbols to correspond with the
publicly visible outer declaration of the class. However, the ESM2015
reflection host used to return the `ts.Symbol` for the inner
declaration, if the class was declared as follows:

```javascript
var MyClass = class MyClass {};
```

For the above code, `Esm2015ReflectionHost.getClassSymbol` would return
the `ts.Symbol` corresponding with the `class MyClass {}` declaration,
whereas it should have corresponded with the `var MyClass` declaration.
As a consequence, no `NgModule` could be resolved for the component, so
no components/directives would be in scope for the component. This
resulted in errors during runtime.

This commit resolves the issue by introducing a `NgccClassSymbol` that
contains references to both the outer and inner `ts.Symbol`, instead of
just a single `ts.Symbol`. This avoids the unclarity of whether a
`ts.Symbol` corresponds with the outer or inner declaration.

More details can be found here: https://hackmd.io/7nkgWOFWQlSRAuIW_8KPPw

Fixes #32078
Closes FW-1507

PR Close #32539
2019-09-12 11:12:10 -07:00
JoostK
2279cb8dc0 refactor(ngcc): move ClassSymbol to become NgccClassSymbol (#32539)
PR Close #32539
2019-09-12 11:12:10 -07:00
JoostK
f7471eea3c fix(ngcc): handle compilation diagnostics (#31996)
Previously, any diagnostics reported during the compilation of an
entry-point would not be shown to the user, but either be ignored or
cause a hard crash in case of a `FatalDiagnosticError`. This is
unfortunate, as such error instances contain information on which code
was responsible for producing the error, whereas only its error message
would not. Therefore, it was quite hard to determine where the error
originates from.

This commit introduces behavior to deal with error diagnostics in a more
graceful way. Such diagnostics will still cause the compilation to fail,
however the error message now contains formatted diagnostics.

Closes #31977
Resolves FW-1374

PR Close #31996
2019-08-29 12:38:02 -07:00
Pete Bacon Darwin
4d93d2406f feat(ivy): ngcc - support ngcc "migrations" (#31544)
This commit implements support for the ngcc migrations
as designed in https://hackmd.io/KhyrFV1VQHmeQsgfJq6AyQ

PR Close #31544
2019-07-23 21:11:40 -07:00
Pete Bacon Darwin
f690a4e0af fix(ivy): ngcc - do not analyze files outside the current package (#30591)
Our module resolution prefers `.js` files over `.d.ts` files because
occasionally libraries publish their typings in the same directory
structure as the compiled JS files, i.e. adjacent to each other.

The standard TS module resolution would pick up the typings
file and add that to the `ts.Program` and so they would be
ignored by our analyzers. But we need those JS files, if they
are part of the current package.

But this meant that we also bring in JS files from external
imports from outside the package, which is not desired.
This was happening for the `@fire/storage` enty-point
that was importing the `firebase/storage` path.

In this commit we solve this problem, for the case of imports
coming from a completely different package, by saying that any
file that is outside the package root directory must be an external
import and so we do not analyze those files.

This does not solve the potential problem of imports between
secondary entry-points within a package but so far that does
not appear to be a problem.

PR Close #30591
2019-06-26 08:00:03 -07:00