Commit Graph

31 Commits

Author SHA1 Message Date
George Kalpakas 5b42084912 fix(ngcc): do not collect private declarations from external packages (#34811)
Previously, while trying to build an `NgccReflectionHost`'s
`privateDtsDeclarationMap`, `computePrivateDtsDeclarationMap()` would
try to collect exported declarations from all source files of the
program (i.e. without checking whether they were within the target
package, as happens for declarations in `.d.ts` files).

Most of the time, that would not be a problem, because external packages
would be represented as `.d.ts` files in the program. But when an
external package had no typings, the JS files would be used instead. As
a result, the `ReflectionHost` would try to (unnecessarilly) parse the
file in order to extract exported declarations, which in turn would be
harmless in most cases.

There are certain cases, though, where the `ReflectionHost` would throw
an error, because it cannot parse the external package's JS file. This
could happen, for example, in `UmdReflectionHost`, which expects the
file to contain exactly one statement. See #34544 for more details on a
real-world failure.

This commit fixes the issue by ensuring that
`computePrivateDtsDeclarationMap()` will only collect exported
declarations from files within the target package.

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

Fixes #34544

PR Close #34811
2020-01-23 13:58:37 -08:00
Paul Gschwendtner 6b468f9b2e fix(ngcc): libraries using spread in object literals cannot be processed (#34661)
Consider a library that uses a shared constant for host bindings. e.g.

```ts
export const BASE_BINDINGS= {
  '[class.mat-themed]': '_isThemed',
}

----

@Directive({
  host: {...BASE_BINDINGS, '(click)': '...'}
})
export class Dir1 {}

@Directive({
  host: {...BASE_BINDINGS, '(click)': '...'}
})
export class Dir2 {}
```

Previously when these components were shipped as part of the
library to NPM, consumers were able to consume `Dir1` and `Dir2`.
No errors showed up.

Now with Ivy, when ngcc tries to process the library, an error
will be thrown. The error is stating that the host bindings should
be an object (which they obviously are). This happens because
TypeScript transforms the object spread to individual
`Object.assign` calls (for compatibility).

The partial evaluator used by the `@Directive` annotation handler
is unable to process this expression because there is no
integrated support for `Object.assign`. In View Engine, this was
not a problem because the `metadata.json` files from the library
were used to compute the host bindings.

Fixes #34659

PR Close #34661
2020-01-23 10:29:57 -08:00
Pete Bacon Darwin f22a6eb00e fix(ngcc): correctly match aliased classes between src and dts files (#34254)
The naïve matching algorithm we previously used to match declarations in
source files to declarations in typings files was based only on the name
of the thing being declared.  This did not handle cases where the declared
item had been exported via an alias - a common scenario when one of the two
file sets (source or typings) has been flattened, while the other has not.

The new algorithm tries to overcome this by creating two maps of export
name to declaration (i.e. `Map<string, ts.Declaration>`).
One for the source files and one for the typings files.
It then joins these two together by matching export names, resulting in a
new map that maps source declarations to typings declarations directly
(i.e. `Map<ts.Declaration, ts.Declaration>`).

This new map can handle the declaration names being different between the
source and typings as long as they are ultimately both exported with the
same alias name.

Further more, there is one map for "public exports", i.e. exported via the
root of the source tree (the entry-point), and another map for "private
exports", which are exported from individual files in the source tree but
not necessarily from the root. This second map can be used to "guess"
the mapping between exports in a deep (non-flat) file tree, which can be
used by ngcc to add required private exports to the entry-point.

Fixes #33593

PR Close #34254
2019-12-18 11:25:01 -08:00
Pete Bacon Darwin 0b837e2f0d refactor(ngcc): use bundle src to create reflection hosts (#34254)
Previously individual properties of the src bundle program were
passed to the reflection host constructors. But going forward,
more properties will be required. To prevent the signature getting
continually larger and more unwieldy, this change just passes the
whole src bundle to the constructor, allowing it to extract what it
needs.

PR Close #34254
2019-12-18 11:25:01 -08:00
Pete Bacon Darwin 1e1e242570 fix(ngcc): support minified ES5 scenarios (#33777)
The reflection hosts have been updated to support the following
code forms, which were found in some minified library code:

* The class IIFE not being wrapped in parentheses.
* Calls to `__decorate()` being combined with the IIFE return statement.

PR Close #33777
2019-11-13 11:11:48 -08:00
Pete Bacon Darwin b3c3000004 fix(ngcc): ensure that adjacent statements go after helper calls (#33689)
Previously the renderers were fixed so that they inserted extra
"adjacent" statements after the last static property of classes.

In order to help the build-optimizer (in Angular CLI) to be able to
tree-shake classes effectively, these statements should also appear
after any helper calls, such as `__decorate()`.

This commit moves the computation of this positioning into the
`NgccReflectionHost` via the `getEndOfClass()` method, which
returns the last statement that is related to the class.

FW-1668

PR Close #33689
2019-11-11 13:01:15 -08:00
Alan Agius d749dd3ea1 fix(ngcc): handle new `__spreadArrays` tslib helper (#33617)
We already have special cases for the `__spread` helper function and with this change we handle the new tslib helper introduced in version 1.10 `__spreadArrays`.

For more context see: https://github.com/microsoft/tslib/releases/tag/1.10.0

Fixes: #33614

PR Close #33617
2019-11-06 19:43:07 +00:00
Pete Bacon Darwin 93a23b9ae0 fix(ngcc): override `getInternalNameOfClass()` and `getAdjacentNameOfClass()` for ES5 (#33533)
In ES5 the class consists of an outer variable declaration that is
initialised by an IIFE. Inside the IIFE the class is implemented by
an inner function declaration that is returned from the IIFE.
This inner declaration may have a different name to the outer
declaration.

This commit overrides `getInternalNameOfClass()` and
`getAdjacentNameOfClass()` in `Esm5ReflectionHost` with methods that
can find the correct inner declaration name identifier.

PR Close #33533
2019-11-05 17:25:01 +00:00
Pete Bacon Darwin 1d141a8ab1 fix(compiler-cli): attach the correct `viaModule` to namespace imports (#33495)
Previously declarations that were imported via a namespace import
were given the same `bestGuessOwningModule` as the context
where they were imported to. This causes problems with resolving
`ModuleWithProviders` that have a type that has been imported in
this way, causing errors like:

```
ERROR in Symbol UIRouterModule declared in
.../@uirouter/angular/uiRouterNgModule.d.ts
is not exported from
.../@uirouter/angular/uirouter-angular.d.ts
(import into .../src/app/child.module.ts)
```

This commit modifies the `TypescriptReflectionHost.getDirectImportOfIdentifier()`
method so that it also understands how to attach the correct `viaModule` to
the identifier of the namespace import.

Resolves #32166

PR Close #33495
2019-10-31 22:25:48 +00:00
Pete Bacon Darwin bfd07b3c94 fix(ngcc): Esm5ReflectionHost.getDeclarationOfIdentifier should handle aliased inner declarations (#33252)
In ES5 modules, the class declarations consist of an IIFE with inner
and outer declarations that represent the class. The `EsmReflectionHost`
has logic to ensure that `getDeclarationOfIdentifier()` always returns the
outer declaration.

Before this commit, if an identifier referred to an alias of the inner
declaration, then `getDeclarationOfIdentifier()` was failing to find
the outer declaration - instead returning the inner declaration.

Now the identifier is correctly resolved up to the outer declaration
as expected.

This should fix some of the failing 3rd party packages discussed in
https://github.com/angular/ngcc-validation/issues/57.

PR Close #33252
2019-10-18 14:41:25 -04: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
Alex Rickabaugh 02bab8cf90 fix(ivy): in ngcc, handle inline exports in commonjs code (#32129)
One of the compiler's tasks is to enumerate the exports of a given ES
module. This can happen for example to resolve `foo.bar` where `foo` is a
namespace import:

```typescript
import * as foo from './foo';

@NgModule({
  directives: [foo.DIRECTIVES],
})
```

In this case, the compiler must enumerate the exports of `foo.ts` in order
to evaluate the expression `foo.DIRECTIVES`.

When this operation occurs under ngcc, it must deal with the different
module formats and types of exports that occur. In commonjs code, a problem
arises when certain exports are downleveled.

```typescript
export const DIRECTIVES = [
  FooDir,
  BarDir,
];
```

can be downleveled to:

```javascript
exports.DIRECTIVES = [
  FooDir,
  BarDir,
```

Previously, ngtsc and ngcc expected that any export would have an associated
`ts.Declaration` node. `export class`, `export function`, etc. all retain
`ts.Declaration`s even when downleveled. But the `export const` construct
above does not. Therefore, ngcc would not detect `DIRECTIVES` as an export
of `foo.ts`, and the evaluation of `foo.DIRECTIVES` would therefore fail.

To solve this problem, the core concept of an exported `Declaration`
according to the `ReflectionHost` API is split into a `ConcreteDeclaration`
which has a `ts.Declaration`, and an `InlineDeclaration` which instead has
a `ts.Expression`. Differentiating between these allows ngcc to return an
`InlineDeclaration` for `DIRECTIVES` and correctly keep track of this
export.

PR Close #32129
2019-08-15 14:45:59 -07:00
JoostK 80f290e301 fix(ivy): ngcc - recognize suffixed tslib helpers (#31614)
An identifier may become repeated when bundling multiple source files
into a single bundle, so bundlers have a strategy of suffixing non-unique
identifiers with a suffix like $2. Since ngcc operates on such bundles,
it needs to process potentially suffixed identifiers in their canonical
form without the suffix. The "ngx-pagination" package was previously not
compiled fully, as most decorators were not recognized.

This commit ensures that identifiers are first canonicalized by removing
the suffix, such that they are properly recognized and processed by ngcc.

Fixes #31540

PR Close #31614
2019-07-29 16:10:58 -07:00
JoostK 5e5be43acd refactor(ivy): ngcc - categorize the various decorate calls upfront (#31614)
Any decorator information present in TypeScript is emitted into the
generated JavaScript sources by means of `__decorate` call. This call
contains both the decorators as they existed in the original source
code, together with calls to `tslib` helpers that convey additional
information on e.g. type information and parameter decorators. These
different kinds of decorator calls were not previously distinguished on
their own, but instead all treated as `Decorator` by themselves. The
"decorators" that were actually `tslib` helper calls were conveniently
filtered out because they were not imported from `@angular/core`, a
characteristic that ngcc uses to drop certain decorators.

Note that this posed an inconsistency in ngcc when it processes
`@angular/core`'s UMD bundle, as the `tslib` helper functions have been
inlined in said bundle. Because of the inlining, the `tslib` helpers
appear to be from `@angular/core`, so ngcc would fail to drop those
apparent "decorators". This inconsistency does not currently cause any
issues, as ngtsc is specifically looking for decorators based on  their
name and any remaining decorators are simply ignored.

This commit rewrites the decorator analysis of a class to occur all in a
single phase, instead of all throughout the `ReflectionHost`. This
allows to categorize the various decorate calls in a single sweep,
instead of constantly needing to filter out undesired decorate calls on
the go. As an added benefit, the computed decorator information is now
cached per class, such that subsequent reflection queries that need
decorator information can reuse the cached info.

PR Close #31614
2019-07-29 16:10:57 -07:00
Pete Bacon Darwin 8a470b9af9 feat(ivy): add `getBaseClassIdentifier()` to `ReflectionHost` (#31544)
This method will be useful for writing ngcc `Migrations` that
need to be able to find base classes.

PR Close #31544
2019-07-23 21:11:39 -07:00
Pete Bacon Darwin 2dfd97d8f0 fix(ivy): ngcc - support bare array constructor param decorators (#30591)
Previously we expected the constructor parameter `decorators`
property to be an array wrapped in a function. Now we also support
an array not wrapped in a function.

PR Close #30591
2019-06-26 08:00:03 -07:00
Pete Bacon Darwin 7186f9c016 refactor(ivy): implement a virtual file-system layer in ngtsc + ngcc (#30921)
To improve cross platform support, all file access (and path manipulation)
is now done through a well known interface (`FileSystem`).

For testing a number of `MockFileSystem` implementations are provided.
These provide an in-memory file-system which emulates operating systems
like OS/X, Unix and Windows.

The current file system is always available via the static method,
`FileSystem.getFileSystem()`. This is also used by a number of static
methods on `AbsoluteFsPath` and `PathSegment`, to avoid having to pass
`FileSystem` objects around all the time. The result of this is that one
must be careful to ensure that the file-system has been initialized before
using any of these static methods. To prevent this happening accidentally
the current file system always starts out as an instance of `InvalidFileSystem`,
which will throw an error if any of its methods are called.

You can set the current file-system by calling `FileSystem.setFileSystem()`.
During testing you can call the helper function `initMockFileSystem(os)`
which takes a string name of the OS to emulate, and will also monkey-patch
aspects of the TypeScript library to ensure that TS is also using the
current file-system.

Finally there is the `NgtscCompilerHost` to be used for any TypeScript
compilation, which uses a given file-system.

All tests that interact with the file-system should be tested against each
of the mock file-systems. A series of helpers have been provided to support
such tests:

* `runInEachFileSystem()` - wrap your tests in this helper to run all the
wrapped tests in each of the mock file-systems.
* `addTestFilesToFileSystem()` - use this to add files and their contents
to the mock file system for testing.
* `loadTestFilesFromDisk()` - use this to load a mirror image of files on
disk into the in-memory mock file-system.
* `loadFakeCore()` - use this to load a fake version of `@angular/core`
into the mock file-system.

All ngcc and ngtsc source and tests now use this virtual file-system setup.

PR Close #30921
2019-06-25 16:25:24 -07:00
JoostK 6fbfb5a159 feat(ivy): ngcc - recognize static properties on the outer symbol in ES5 (#30795)
Packages that have been compiled using an older version of TypeScript
can have their decorators at the top-level of the ES5 bundles, instead
of inside the IIFE that is emitted for the class. Before this change,
ngcc only took static property assignments inside the IIFE into account,
therefore missing the decorators that were assigned at the top-level.

This commit extends the ES5 host to look for static properties in two
places. Testcases for all bundle formats that contain ES5 have been added
to ensure that this works in the various flavours.

A patch is included to support UMD bundles. The UMD factory affects how
TypeScripts binds the static properties to symbols, see the docblock of
the patch function for more details.

PR Close #30795
2019-06-14 13:09:56 -07:00
Paul Gschwendtner 2b4d5c7548 fix(ivy): ngcc should process undecorated base classes (#30821)
Currently undecorated classes are intentionally not processed
with ngcc. This is causing unexpected behavior because decorator
handlers such as `base_def.ts` are specifically interested in class
definitions without top-level decorators, so that the base definition
can be generated if there are Angular-specific class members.

In order to ensure that undecorated base-classes work as expected
with Ivy, we need to run the decorator handlers for all top-level
class declarations (not only for those with decorators). This is similar
to when `ngtsc` runs decorator handlers when analyzing source-files.

Resolves FW-1355. Fixes https://github.com/angular/components/issues/16178

PR Close #30821
2019-06-11 00:19:34 +00:00
JoostK 9d9c9e43e5 feat(ivy): static evaluation of TypeScript's `__spread` helper (#30492)
The usage of array spread syntax in source code may be downleveled to a
call to TypeScript's `__spread` helper function from `tslib`, depending
on the options `downlevelIteration` and `emitHelpers`. This proves
problematic for ngcc when it is processing ES5 formats, as the static
evaluator won't be able to interpret those calls.

A custom foreign function resolver is not sufficient in this case, as
`tslib` may be emitted into the library code itself. In that case, a
helper function can be resolved to an actual function with body, such
that it won't be considered as foreign function. Instead, a reflection
host can now indicate that the definition of a function corresponds with
a certain TypeScript helper, such that it becomes statically evaluable
in ngtsc.

Resolves #30299

PR Close #30492
2019-06-10 23:53:04 +00:00
Pete Bacon Darwin 0fa72a8bc8 refactor(ivy): ngcc - fake core and tslib should be typings files (#25445)
Previously these fake files were full TypeScript source
files (`.ts`) but this is not necessary as we only need the
typings not the implementation.

PR Close #25445
2019-05-16 12:11:04 -07:00
Pete Bacon Darwin 989bfd2e97 fix(ivy): ngcc - support namespaced identifiers (#25445)
In UMD formats, imports are always namespaced. This commit makes
ngcc more tolerant of such structures.

PR Close #25445
2019-05-16 12:11:04 -07:00
Pete Bacon Darwin e68490c5e4 test(ivy): fix ESM5 test code to use `var` rather than `const` (#25445)
PR Close #25445
2019-05-16 12:11:04 -07:00
Pete Bacon Darwin 20898f9f4f test(ivy): ngcc - tighten up typings in Esm5ReflectionHost specs (#29643)
PR Close #29643
2019-04-29 12:37:21 -07:00
JoostK 98f8b0f328 fix(ivy): ngcc - properly handle aliases class expressions (#29119)
In ES2015, classes could have been emitted as a variable declaration
initialized with a class expression. In certain situations, an intermediary
variable suffixed with `_1` is present such that the variable
declaration's initializer becomes a binary expression with its rhs being
the class expression, and its lhs being the identifier of the intermediate
variable. This structure was not recognized, resulting in such classes not
being considered as a class in `Esm2015ReflectionHost`.

As a consequence, the analysis of functions/methods that return a
`ModuleWithProviders` object did not take the methods of such classes into
account.

Another edge-case with such intermediate variable was that static
properties would not be considered as class members. A testcase was added
to prevent regressions.

Fixes #29078

PR Close #29119
2019-04-02 10:50:46 -07:00
Pete Bacon Darwin 8d3d75e454 feat(compiler-cli): ngcc - make logging more configurable (#29591)
This allows CLI usage to filter excessive log messages
and integrations like webpack plugins to provide their own logger.

// FW-1198

PR Close #29591
2019-04-01 11:53:28 -07:00
George Kalpakas 21835af70c fix(ivy): handle class declarations consistently in ES5 code (#29209)
PR Close #29209
2019-03-21 22:20:24 +00:00
George Kalpakas 2790352d04 refactor(ivy): use `ClassDeclaration` in more `ReflectionHost` methods (#29209)
PR Close #29209
2019-03-21 22:20:23 +00:00
George Kalpakas bb6a3632f6 refactor(ivy): correctly type class declarations in `ngtsc`/`ngcc` (#29209)
Previously, several `ngtsc` and `ngcc` APIs dealing with class
declaration nodes used inconsistent types. For example, some methods of
the `DecoratorHandler` interface expected a `ts.Declaration` argument,
but actual `DecoratorHandler` implementations specified a stricter
`ts.ClassDeclaration` type.

As a result, the stricter methods would operate under the incorrect
assumption that their arguments were of type `ts.ClassDeclaration`,
while the actual arguments might be of different types (e.g. `ngcc`
would call them with `ts.FunctionDeclaration` or
`ts.VariableDeclaration` arguments, when compiling ES5 code).

Additionally, since we need those class declarations to be referenced in
other parts of the program, `ngtsc`/`ngcc` had to either repeatedly
check for `ts.isIdentifier(node.name)` or assume there was a `name`
identifier and use `node.name!`. While this assumption happens to be
true in the current implementation, working around type-checking is
error-prone (e.g. the assumption might stop being true in the future).

This commit fixes this by introducing a new type to be used for such
class declarations (`ts.Declaration & {name: ts.Identifier}`) and using
it consistently throughput the code.

PR Close #29209
2019-03-21 22:20:23 +00:00
Pete Bacon Darwin b48d6e1b13 fix(ivy): ngcc - empower `Esm5ReflectionHost` to analyze `ModuleWithProviders` functions (#29092)
In ESM5 code, static methods appear as property assignments onto the constructor
function. For example:

```
var MyClass = (function() {
  function MyClass () {}
  MyClass.staticMethod = function() {};
  return MyClass;
})();
```

This commit teaches ngcc how to process these forms when searching
for `ModuleWithProviders` functions that need to be updated in the typings
files.

PR Close #29092
2019-03-20 14:45:55 -04:00
Pete Bacon Darwin a770aa231d refactor(ivy): move ngcc into a higher level folder (#29092)
PR Close #29092
2019-03-20 14:45:54 -04:00