Commit Graph
1068 Commits
Author SHA1 Message Date
Alex Rickabaugh c59f401f9a fix(compiler-cli): setComponentScope should only list used components/pipes (#39662)
ngtsc will avoid emitting generated imports that would create an import
cycle in the user's program. The main way such imports can arise is when
a component would ordinarily reference its dependencies in its component
definition `directiveDefs` and `pipeDefs`. This requires adding imports,
which run the risk of creating a cycle.

When ngtsc detects that adding such an import would cause this to occur, it
instead falls back on a strategy called "remote scoping", where a side-
effectful call to `setComponentScope` in the component's NgModule file is
used to patch `directiveDefs` and `pipeDefs` onto the component. Since the
NgModule file already imports all of the component's dependencies (to
declare them in the NgModule), this approach does not risk adding a cycle.
It has several large downsides, however:

1. it breaks under `sideEffects: false` logic in bundlers including the CLI
2. it breaks tree-shaking for the given component and its dependencies

See this doc for further details: https://hackmd.io/Odw80D0pR6yfsOjg_7XCJg?view

In particular, the impact on tree-shaking was exacerbated by the naive logic
ngtsc used to employ here. When this feature was implemented, at the time of
generating the side-effectful `setComponentScope` call, the compiler did not
know which of the component's declared dependencies were actually used in
its template. This meant that unlike the generation of `directiveDefs` in
the component definition itself, `setComponentScope` calls had to list the
_entire_ compilation scope of the component's NgModule, including directives
and pipes which were not actually used in the template. This made the tree-
shaking impact much worse, since if the component's NgModule made use of any
shared NgModules (e.g. `CommonModule`), every declaration therein would
become un-treeshakable.

Today, ngtsc does have the information on which directives/pipes are
actually used in the template, but this was not being used during the remote
scoping operation. This commit modifies remote scoping to take advantage of
the extra context and only list used dependencies in `setComponentScope`
calls, which should ameliorate the tree-shaking impact somewhat.

PR Close #39662
2020-11-13 11:57:20 -08:00
Pete Bacon Darwin 7b6ea973f6 refactor(compiler-cli): return the FileSystem from initMockFileSystem() (#39617)
It is common to want to use the file system once it is initialized,
so it makes sense for this function to return it.

PR Close #39617
2020-11-13 11:25:56 -08:00
JoostK ade6da95e4 perf(compiler-cli): reduce filesystem hits during resource resolution (#39604)
The resource loader uses TypeScript's module resolution system to
determine at which locations it needs to look for a resource file. A
marker string is used to force the module resolution to fail, such that
all failed lookup locations can then be considered for actual resource
resolution. Any filesystem requests targeting files/directories that
contain the marker are known not to exist, so no filesystem request
needs to be done at all.

PR Close #39604
2020-11-12 13:57:20 -08:00
JoostK 7c161e1679 refactor(compiler-cli): define type alias for the required delegation pattern (#39604)
The type alias allows for this pattern to be more easily used in other
areas of the compiler code. The current usages of this pattern have been
updated to use the type alias.

PR Close #39604
2020-11-12 13:57:20 -08:00
Alex Rickabaugh cf88ea0bf3 fix(compiler-cli): avoid duplicate diagnostics about unknown pipes (#39517)
TCB generation occasionally transforms binding expressions twice, which can
result in a `BindingPipe` operation being `resolve()`'d multiple times. When
the pipe does not exist, this caused multiple OOB diagnostics to be recorded
about the missing pipe.

This commit fixes the problem by making the OOB recorder track which pipe
expressions have had diagnostics produced already, and only producing them
once per expression.

PR Close #39517
2020-11-06 15:27:38 -08:00
Alan Agius 0929099e41 refactor(compiler-cli): remove TypeScript 3.9 workarounds (#39586)
With this change we remove code which was used to support both TypeScript 3.9 and TypeScript 4.0

This code is now no longer needed because G3 is on TypeScript 4.0

PR Close #39586
2020-11-06 15:26:51 -08:00
Pete Bacon Darwin b9b9178458 fix(compiler-cli): do not drop non-Angular decorators when downleveling (#39577)
There is a compiler transform that downlevels Angular class decorators
to static properties so that metadata is available for JIT compilation.
The transform was supposed to ignore non-Angular decorators but it was
actually completely dropping decorators that did not conform to a very
specific syntactic shape (i.e. the decorator was a simple identifier, or
a namespaced identifier).

This commit ensures that all non-Angular decorators are kepts as-is
even if they are built using a syntax that the Angular compiler does not
understand.

Fixes #39574

PR Close #39577
2020-11-06 09:21:51 -08:00
Andrew Scott 3a1d36cce3 refactor(language-service): Use compiler APIs in Ivy to get definitions for external resources (#39476)
Rather than re-reading component metadata that was already interpreted
by the Ivy compiler, the Language Service should instead use the
compiler APIs to get information it needs about the metadata.

PR Close #39476
2020-11-06 09:17:33 -08:00
JoostK 8c0a92bb45 feat(compiler-cli): partial compilation of directives (#39518)
This commit implements partial code generation for directives, which
will be transformed by the linker plugin to fully AOT compiled code in
follow-up work.

PR Close #39518
2020-11-04 10:44:37 -08:00
Kristiyan Kostadinov 7e33cb9626 fix(compiler-cli): generating invalid setClassMetadata call in ES5 for class with custom decorator (#39527)
When a class with a custom decorator is transpiled to ES5, it looks something like this:

```
var SomeClass = (function() {
  function SomeClass() {...};
  var SomeClass_1 = __decorate([Decorator()], SomeClass);
  SomeClass = SomeClass_1;
  return SomeClass;
})();
```

The problem is that if the class also has an Angular decorator that refers to the class itself
(e.g. `{provide: someToken, useClass: SomeClass}`), the generated `setClassMetadata` code will
be emitted after the IIFE, but will still refer to the intermediate `SomeClass_1` variable from
inside the IIFE. This happens, because we generate the `setClassMetadata` call directly from
the source AST which contains identifiers that TS will rename when it emits the ES5 code.

These changes resolve the issue by looking through the metadata AST and cloning any `Identifier`
that is referring to the class. Since TS doesn't have references to the clone, it won't rename
it when transpiling to ES5.

Fixes #39509.

PR Close #39527
2020-11-03 14:52:59 -08:00
JoostK 40bf1e0475 test(compiler-cli): remove spurious console.error call from a test (#39321)
Reporting the source file text to the console was left as a debugging
artifact.

PR Close #39321
2020-11-02 07:50:41 -08:00
JoostK 5d731354d0 perf(compiler-cli): only generate template context declaration when used (#39321)
The variable declaration for a template context is only needed when it
is referenced from somewhere, so the TCB operation to generate the
declaration is marked as optional.

PR Close #39321
2020-11-02 07:50:41 -08:00
JoostK 3b0b7d2210 fix(compiler-cli): report missing pipes when fullTemplateTypeCheck is disabled (#39320)
Even if `fullTemplateTypeCheck` is disabled should missing pipes still
be reported, as was the case in View Engine.

Fixes #38195

PR Close #39320
2020-10-30 18:01:51 -07:00
Kristiyan Kostadinov cbc0907bfd fix(compiler): preserve this.$event and this.$any accesses in expressions (#39323)
Currently expressions `$event.foo()` and `this.$event.foo()`, as well as `$any(foo)` and
`this.$any(foo)`, are treated as the same expression by the compiler, because `this` is considered
the same implicit receiver as when the receiver is omitted. This introduces the following issues:

1. Any time something called `$any` is used, it'll be stripped away, leaving only the first parameter.
2. If something called `$event` is used anywhere in a template, it'll be preserved as `$event`,
rather than being rewritten to `ctx.$event`, causing the value to undefined at runtime. This
applies to listener, property and text bindings.

These changes resolve the first issue and part of the second one by preserving anything that
is accessed through `this`, even if it's one of the "special" ones like `$any` or `$event`.
Furthermore, these changes only expose the `$event` global variable inside event listeners,
whereas previously it was available everywhere.

Fixes #30278.

PR Close #39323
2020-10-30 10:49:15 -07:00
Andrew Scott beb935613e refactor(compiler-cli): Store inline templates and styles in the resource registry (#39482)
The Language Service is not only interested in external resources, but
also inline styles and templates. By storing the expression of the
inline resources, we can more easily determine if a given position is
part of the inline template/style expression.

PR Close #39482
2020-10-30 09:58:21 -07:00
Andrew Scott 371fb9a955 refactor(compiler-cli): Track external component resources in ResourceRegistry (#39373)
In addition to the template mapping that already existed, we want to also track the mapping for external
style files. We also store the `ts.Expression` in the registry so external tools can look up a resource
on a component by expression and avoid reading the value.

PR Close #39373
2020-10-28 10:57:14 -07:00
Alex Rickabaugh 0ecdef9cfa refactor(compiler-cli): API to get directives/pipes in scope (#39278)
This commit introduces two new methods to the TemplateTypeChecker, which
retrieve the directives and pipes that are "in scope" for a given component
template. The metadata returned by this API is minimal, but enough to power
autocompletion of selectors and attributes in templates.

PR Close #39278
2020-10-27 13:17:14 -07:00
Alex Rickabaugh 01cc949722 refactor(compiler-cli): cache Symbols in the TemplateTypeCheckerImpl (#39278)
This commit introduces caching of `Symbol`s produced by the template type-
checking infrastructure, in the same way that autocompletion results are
now cached.

PR Close #39278
2020-10-27 13:17:14 -07:00
Alex Rickabaugh c4f99b6e52 refactor(compiler-cli): move global completion into new CompletionEngine (#39278)
This commit refactors the previously introduced `getGlobalCompletions()` API
for the template type-checker in a couple ways:

 * The return type is adjusted to use a `Map` instead of an array, and
   separate out the component context completion position. This allows for a
   cleaner integration in the language service.
 * A new `CompletionEngine` class is introduced which powers autocompletion
   for a single component, and can cache completion results.
 * The `CompletionEngine` for each component is itself cached on the
   `TemplateTypeCheckerImpl` and is invalidated when the component template
   is overridden or reset.

This refactoring simplifies the `TemplateTypeCheckerImpl` class by
extracting the autocompletion logic, enables caching for better performance,
and prepares for the introduction of other autocompletion APIs.

PR Close #39278
2020-10-27 13:17:14 -07:00
Pete Bacon Darwin 27a4adebcb refactor(compiler-cli): support namespaced references (#39346)
The compiler uses a `Reference` abstraction to refer to TS nodes
that it needs to refer to from other parts of the source. Such
references keep track of any identifiers that represent the referenced
node.

Prior to this commit, the compiler (and specifically `ReferenceEmitter`
classes) assumed that the reference identifiers are always free standing.
In other words a reference identifier would be an expression like
`FooDirective` in the expression `class FooDirective {}`.

But in UMD/CommonJS source, a reference can actually refer to an "exports"
declaration of the form `exports.FooDirective = ...`.
In such cases the `FooDirective` identifier is not free-standing
since it is part of a property access, so the `ReferenceEmitter`
should take this into account when emitting an expression that
refers to such a `Reference`.

This commit changes the `LocalIdentifierStrategy` reference emitter
so that if the `node` being referenced is not a declaration itself and
is in the current file, then it should be used directly, rather than
trying to use one of its identifiers.

PR Close #39346
2020-10-23 15:17:11 -07:00
Pete Bacon Darwin 413b55273b fix(ngcc): capture UMD/CommonJS inner class implementation node correctly (#39346)
Previously, UMD/CommonJS class inline declarations of the form:

```ts
exports.Foo = (function() { function Foo(); return Foo; })();
```

were capturing the whole IIFE as the implementation, rather than
the inner class (i.e. `function Foo() {}` in this case). This caused
the interpreter to break when it was trying to access such an export,
since it would try to evaluate the IIFE rather than treating it as a class
declaration.

PR Close #39346
2020-10-23 15:17:11 -07:00
Andrew Kushnir f5a3e44e9d refactor(compiler): remove support for TypeScript 3.9 (#39313)
This commit removes TypeScript 3.9 support.

BREAKING CHANGE:

TypeScript 3.9 is no longer supported, please upgrade to TypeScript 4.0.

PR Close #39313
2020-10-19 14:34:45 -07:00
Pete Bacon Darwin 7e742aea7c refactor(compiler-cli): linker - add Babel plugin, FileLinker and initial PartialLinkers (#39116)
This commit adds the basic building blocks for linking partial declarations.
In particular it provides a generic `FileLinker` class that delegates to
a set of (not yet implemented) `PartialLinker` classes.

The Babel plugin makes use of this `FileLinker` providing concrete classes
for `AstHost` and `AstFactory` that work with Babel AST. It can be created
with the following code:

```ts
const plugin = createEs2015LinkerPlugin({ /* options */ });
```

PR Close #39116
2020-10-19 11:23:45 -07:00
Pete Bacon Darwin ac0016cd82 refactor(compiler-cli): visit inline declarations with implementations differently (#39267)
Some inline declarations are of the form:

```
exports.<name> = <implementation>;
```

In this case the declaration `node` is `exports.<name>`.
When interpreting such inline declarations we actually want
to visit the `implementation` expression rather than visiting
the declaration `node`.

This commit adds `implementation?: ts.Expression` to the
`InlineDeclaration` type and updates the interpreter to visit
these expressions as described above.

PR Close #39267
2020-10-14 14:11:45 -07:00
Pete Bacon Darwin 952710b43b refactor(compiler-cli): ensure isNamed....() helpers check name is identity (#38959)
Previously the `node.name` property was only checked to ensure it was
defined. But that meant that it was a `ts.BindingName`, which also includes
`ts.BindingPattern`, which we do not support. But these helper methods were
forcefully casting the value to `ts.Identifier.

Now we also check that the `node.name` is actually an `ts.Identifier`.

PR Close #38959
2020-10-12 08:32:47 -07:00
Pete Bacon Darwin 2736a43ecb fix(compiler-cli): support namespaced query types in directives (#38959)
Previously directive "queries" that relied upon a namespaced type

```ts
queries: {
  'mcontent': new core.ContentChild('test2'),
}
```

caused an error to be thrown. This is now supported.

PR Close #38959
2020-10-12 08:32:47 -07:00
Pete Bacon Darwin 47eab61cad test(ngcc): use isNamedDeclaration() helper to simplify tests (#38959)
Previously these tests were checking multiple specific expression
types. The new helper function is more general and will also support
`PropertyAccessExpression` nodes for `InlineDeclaration` types.

PR Close #38959
2020-10-12 08:32:46 -07:00
Pete Bacon Darwin 0accd1e68d refactor(compiler-cli): implement DeclarationNode node type (#38959)
Previously the `ConcreteDeclaration` and `InlineDeclaration` had
different properties for the underlying node type. And the `InlineDeclaration`
did not store a value that represented its declaration.

It turns out that a natural declaration node for an inline type is the
expression. For example in UMD/CommonJS this would be the `exports.<name>`
property access node.

So this expression is now used for the `node` of `InlineDeclaration` types
and the `expression` property is dropped.

To support this the codebase has been refactored to use a new `DeclarationNode`
type which is a union of `ts.Declaration|ts.Expression` instead of `ts.Declaration`
throughout.

PR Close #38959
2020-10-12 08:32:46 -07:00
Pete Bacon Darwin 6650d71fe2 test(compiler-cli): make the getDeclaration() utility more resilient to code format (#38959)
Previously `getDeclaration()` would only return the first node that matched
the name passed in and then assert the predicate on this single node.
It also only considered a subset of possible declaration types that we might
care about.

Now the function will parse the whole tree collecting an array of all the
nodes that match the name. It then filters this array based on the predicate
and only errors if the filtered array is empty.

This makes this function much more resilient to more esoteric code formats
such as UMD.

PR Close #38959
2020-10-12 08:32:46 -07:00
Pete Bacon Darwin ed81588c79 refactor(compiler-cli): move map creation to avoid unnecessary work (#38959)
If the `symbol` for the given `node` does not exist then there is no
point in creating the `map`.

PR Close #38959
2020-10-12 08:32:45 -07:00