fix(compiler): switch to 'referencedFiles' for shim generation (#36211)

Shim generation was built on a lie.

Shims are files added to the program which aren't original files authored by
the user, but files authored effectively by the compiler. These fall into
two categories: files which will be generated (like the .ngfactory shims we
generate for View Engine compatibility) as well as files used internally in
compilation (like the __ng_typecheck__.ts file).

Previously, shim generation was driven by the `rootFiles` passed to the
compiler as input. These are effectively the `files` listed in the
`tsconfig.json`. Each shim generator (e.g. the `FactoryGenerator`) would
examine the `rootFiles` and produce a list of shim file names which it would
be responsible for generating. These names would then be added to the
`rootFiles` when the program was created.

The fatal flaw here is that `rootFiles` does not always account for all of
the files in the program. In fact, it's quite rare that it does. Users don't
typically specify every file directly in `files`. Instead, they rely on
TypeScript, during program creation, starting with a few root files and
transitively discovering all of the files in the program.

This happens, however, during `ts.createProgram`, which is too late to add
new files to the `rootFiles` list.

As a result, shim generation was only including shims for files actually
listed in the `tsconfig.json` file, and not for the transitive set of files
in the user's program as it should.

This commit completely rewrites shim generation to use a different technique
for adding files to the program, inspired by View Engine's shim generator.
In this new technique, as the program is being created and `ts.SourceFile`s
are being requested from the `NgCompilerHost`, shims for those files are
generated and a reference to them is patched onto the original file's
`ts.SourceFile.referencedFiles`. This causes TS to think that the original
file references the shim, and causes the shim to be included in the program.
The original `referencedFiles` array is saved and restored after program
creation, hiding this little hack from the rest of the system.

The new shim generation engine differentiates between two kinds of shims:
top-level shims (such as the flat module entrypoint file and
__ng_typecheck__.ts) and per-file shims such as ngfactory or ngsummary
files. The former are included via `rootFiles` as before, the latter are
included via the `referencedFiles` of their corresponding original files.

As a result of this change, shims are now correctly generated for all files
in the program, not just the ones named in `tsconfig.json`.

A few mitigating factors prevented this bug from being realized until now:

* in g3, `files` does include the transitive closure of files in the program
* in CLI apps, shims are not really used

This change also makes use of a novel technique for associating information
with source files: the use of an `NgExtension` `Symbol` to patch the
information directly onto the AST object. This is used in several
circumstances:

* For shims, metadata about a `ts.SourceFile`'s status as a shim and its
  origins are held in the extension data.
* For original files, the original `referencedFiles` are stashed in the
  extension data for later restoration.

The main benefit of this technique is a lot less bookkeeping around `Map`s
of `ts.SourceFile`s to various kinds of data, which need to be tracked/
invalidated as part of incremental builds.

This technique is based on designs used internally in the TypeScript
compiler and is serving as a prototype of this design in ngtsc. If it works
well, it could have benefits across the rest of the compiler.

PR Close #36211
This commit is contained in:
Alex Rickabaugh
2020-02-26 16:12:39 -08:00
parent bab90a7709
commit 4213e8d5f0
30 changed files with 1082 additions and 238 deletions

View File

@ -158,11 +158,16 @@ export class NgtscTestEnvironment {
this.multiCompileHostExt.invalidate(absFilePath);
}
tsconfig(extraOpts: {[key: string]: string|boolean|null} = {}, extraRootDirs?: string[]): void {
tsconfig(
extraOpts: {[key: string]: string|boolean|null} = {}, extraRootDirs?: string[],
files?: string[]): void {
const tsconfig: {[key: string]: any} = {
extends: './tsconfig-base.json',
angularCompilerOptions: {...extraOpts, enableIvy: true},
};
if (files !== undefined) {
tsconfig['files'] = files;
}
if (extraRootDirs !== undefined) {
tsconfig.compilerOptions = {
rootDirs: ['.', ...extraRootDirs],

View File

@ -3468,6 +3468,56 @@ runInEachFileSystem(os => {
env.tsconfig({'generateNgFactoryShims': true});
});
it('should be able to depend on an existing factory shim', () => {
// This test verifies that ngfactory files from the compilations of dependencies are
// available to import in a fresh compilation. It is derived from a bug observed in g3 where
// the shim system accidentally caused TypeScript to think that *.ngfactory.ts files always
// exist.
env.write('other.ngfactory.d.ts', `
export class OtherNgFactory {}
`);
env.write('test.ts', `
import {OtherNgFactory} from './other.ngfactory';
class DoSomethingWith extends OtherNgFactory {}
`);
expect(env.driveDiagnostics()).toEqual([]);
});
it('should generate factory shims for files not listed in root files', () => {
// This test verifies that shims are generated for all files in the user's program, even if
// only a subset of those files are listed in the tsconfig as root files.
env.tsconfig({'generateNgFactoryShims': true}, /* extraRootDirs */ undefined, [
absoluteFrom('/test.ts'),
]);
env.write('test.ts', `
import {Component} from '@angular/core';
import {OtherCmp} from './other';
@Component({
selector: 'test',
template: '...',
})
export class TestCmp {
constructor(other: OtherCmp) {}
}
`);
env.write('other.ts', `
import {Component} from '@angular/core';
@Component({
selector: 'other',
template: '...',
})
export class OtherCmp {}
`);
env.driveMain();
expect(env.getContents('other.ngfactory.js')).toContain('OtherCmp');
});
it('should generate correct type annotation for NgModuleFactory calls in ngfactories', () => {
env.write('test.ts', `
import {Component} from '@angular/core';

View File

@ -1838,6 +1838,29 @@ export declare class AnimationEvent {
expect(diags.length).toBe(0);
});
});
describe('stability', () => {
// This section tests various scenarios which have more complex ts.Program setups and thus
// exercise edge cases of the template type-checker.
it('should accept a program with a flat index', () => {
// This test asserts that flat indices don't have any negative interactions with the
// generation of template type-checking code in the program.
env.tsconfig({fullTemplateTypeCheck: true, flatModuleOutFile: 'flat.js'});
env.write('test.ts', `
import {Component} from '@angular/core';
@Component({
selector: 'test-cmp',
template: '{{expr}}'
})
export class TestCmp {
expr = 'string';
}
`);
expect(env.driveDiagnostics()).toEqual([]);
});
});
});
});