Commit Graph

13 Commits

Author SHA1 Message Date
Alex Rickabaugh afcff73be3 fix(ngcc): report the correct viaModule when reflecting over commonjs (#33192)
In the ReflectionHost API, a 'viaModule' indicates that a particular value
originated in another absolute module. It should always be 'null' for values
originating in relatively-imported modules.

This commit fixes a bug in the CommonJsReflectionHost where viaModule would
be reported even for relatively-imported values, which causes invalid import
statements to be generated during compilation.

A test is added to verify the correct behavior.

FW-1628 #resolve

PR Close #33192
2019-10-17 19:43:39 -04:00
JoostK 3c7da767d8 fix(ngcc): resolve imports in `.d.ts` files for UMD/CommonJS bundles (#32619)
In ngcc's reflection host for UMD and CommonJS bundles, custom logic is
present to resolve import details of an identifier. However, this custom
logic is unable to resolve an import for an identifier inside of
declaration files, as such files use the regular ESM import syntax.

As a consequence of this limitation, ngtsc is unable to resolve
`ModuleWithProviders` imports that are declared in an external library.
In that situation, ngtsc determines the type of the actual `NgModule`
that is imported, by looking in the library's declaration files for the
generic type argument on `ModuleWithProviders`. In this process, ngtsc
resolves the import for the `ModuleWithProviders` identifier to verify
that it is indeed the `ModuleWithProviders` type from `@angular/core`.
So, when the UMD reflection host was in use this resolution would fail,
therefore no `NgModule` type could be detected.

This commit fixes the bug by using the regular import resolution logic
in addition to the custom resolution logic that is required for UMD
and CommonJS bundles.

Fixes #31791

PR Close #32619
2019-09-12 13:18:20 -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
Alex Rickabaugh 964d72610f fix(ivy): ngcc should only index .d.ts exports within the package (#32129)
ngcc needs to solve a unique problem when compiling typings for an
entrypoint: it must resolve a declaration within a .js file to its
representation in a .d.ts file. Since such .d.ts files can be used in deep
imports without ever being referenced from the "root" .d.ts, it's not enough
to simply match exported types to the root .d.ts. ngcc must build an index
of all .d.ts files.

Previously, this operation had a bug: it scanned all .d.ts files in the
.d.ts program, not only those within the package. Thus, if a class in the
program happened to share a name with a class exported from a dependency's
.d.ts, ngcc might accidentally modify the wrong .d.ts file, causing a
variety of issues downstream.

To fix this issue, ngcc's .d.ts scanner now limits the .d.ts files it
indexes to only those declared in the current package.

PR Close #32129
2019-08-15 14:46:00 -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 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 230e9766f6 test(ivy): fix strict null checks failures in ngcc tests (#30967)
9d9c9e43e5 has been created a few days ago
and wasn't rebased on top of recent changes that introduces a commonjs host.

This means that tests for the commonjs host haven't been updated to work with
the changes from from #30492 and now fail in `master`.

PR Close #30967
2019-06-11 09:37:19 -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
Pete Bacon Darwin 620cd5c148 feat(ivy): ngcc - implement `CommonJsReflectionHost` (#30200)
PR Close #30200
2019-05-22 16:24:15 -07:00