Skip to content

Commit

Permalink
feat(ivy): give shim generation its own compiler options (angular#33256)
Browse files Browse the repository at this point in the history 
As a hack to get the Ivy compiler ngtsc off the ground, the existing
'allowEmptyCodegenFiles' option was used to control generation of ngfactory
and ngsummary shims during compilation. This option was selected since it's
enabled in google3 but never enabled in external projects.

As ngtsc is now mature and the role shims play in compilation is now better
understood across the ecosystem, this commit introduces two new compiler
options to control shim generation:

* generateNgFactoryShims controls the generation of .ngfactory shims.
* generateNgSummaryShims controls the generation of .ngsummary shims.

The 'allowEmptyCodegenFiles' option is still honored if either of the above
flags are not set explicitly.

PR Close angular#33256
  • Loading branch information
@alxhub @AndrusGerman
alxhub authored and AndrusGerman committed Oct 22, 2019
1 parent 128f02d commit efb99f1
Show file tree
Hide file tree
Showing 4 changed files with 143 additions and 102 deletions.
2 changes: 2 additions & 0 deletions packages/bazel/src/ng_module.bzl
View file
Expand Up @@ -305,6 +305,8 @@ def _ngc_tsconfig(ctx, files, srcs, **kwargs):
"enableResourceInlining": ctx.attr.inline_resources,
"generateCodeForLibraries": False,
"allowEmptyCodegenFiles": True,
"generateNgFactoryShims": True,
"generateNgSummaryShims": True,
# Summaries are only enabled if Angular outputs are to be produced.
"enableSummariesForJit": is_legacy_ngc,
"enableIvy": _enable_ivy_value(ctx),
Expand Down
28 changes: 23 additions & 5 deletions packages/compiler-cli/src/ngtsc/program.ts
View file
Expand Up @@ -83,18 +83,30 @@ export class NgtscProgram implements api.Program {
this.rootDirs = getRootDirs(host, options);
this.closureCompilerEnabled = !!options.annotateForClosureCompiler;
this.resourceManager = new HostResourceLoader(host, options);
const shouldGenerateShims = options.allowEmptyCodegenFiles || false;
// TODO(alxhub): remove the fallback to allowEmptyCodegenFiles after verifying that the rest of
// our build tooling is no longer relying on it.
const allowEmptyCodegenFiles = options.allowEmptyCodegenFiles || false;
const shouldGenerateFactoryShims = options.generateNgFactoryShims !== undefined ?
options.generateNgFactoryShims :
allowEmptyCodegenFiles;
const shouldGenerateSummaryShims = options.generateNgSummaryShims !== undefined ?
options.generateNgSummaryShims :
allowEmptyCodegenFiles;
const normalizedRootNames = rootNames.map(n => absoluteFrom(n));
if (host.fileNameToModuleName !== undefined) {
this.fileToModuleHost = host as FileToModuleHost;
}
let rootFiles = [...rootNames];

const generators: ShimGenerator[] = [];
if (shouldGenerateShims) {
let summaryGenerator: SummaryGenerator|null = null;
if (shouldGenerateSummaryShims) {
// Summary generation.
const summaryGenerator = SummaryGenerator.forRootFiles(normalizedRootNames);
summaryGenerator = SummaryGenerator.forRootFiles(normalizedRootNames);
generators.push(summaryGenerator);
}

if (shouldGenerateFactoryShims) {
// Factory generation.
const factoryGenerator = FactoryGenerator.forRootFiles(normalizedRootNames);
const factoryFileMap = factoryGenerator.factoryFileMap;
Expand All @@ -107,8 +119,14 @@ export class NgtscProgram implements api.Program {
});

const factoryFileNames = Array.from(factoryFileMap.keys());
rootFiles.push(...factoryFileNames, ...summaryGenerator.getSummaryFileNames());
generators.push(summaryGenerator, factoryGenerator);
rootFiles.push(...factoryFileNames);
generators.push(factoryGenerator);
}

// Done separately to preserve the order of factory files before summary files in rootFiles.
// TODO(alxhub): validate that this is necessary.
if (shouldGenerateSummaryShims) {
rootFiles.push(...summaryGenerator !.getSummaryFileNames());
}

this.typeCheckFilePath = typeCheckFilePath(this.rootDirs);
Expand Down
17 changes: 17 additions & 0 deletions packages/compiler-cli/src/transformers/api.ts
View file
Expand Up @@ -196,6 +196,23 @@ export interface CompilerOptions extends ts.CompilerOptions {
*/
enableResourceInlining?: boolean;

/**
* Controls whether ngtsc will emit `.ngfactory.js` shims for each compiled `.ts` file.
*
* These shims support legacy imports from `ngfactory` files, by exporting a factory shim
* for each component or NgModule in the original `.ts` file.
*/
generateNgFactoryShims?: boolean;

/**
* Controls whether ngtsc will emit `.ngsummary.js` shims for each compiled `.ts` file.
*
* These shims support legacy imports from `ngsummary` files, by exporting an empty object
* for each NgModule in the original `.ts` file. The only purpose of summaries is to feed them to
* `TestBed`, which is a no-op in Ivy.
*/
generateNgSummaryShims?: boolean;

/**
* Tells the compiler to generate definitions using the Render3 style code generation.
* This option defaults to `true`.
Expand Down
198 changes: 101 additions & 97 deletions packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
View file
Expand Up @@ -2340,7 +2340,7 @@ runInEachFileSystem(os => {
});

it('should generate correct factory stubs for a test module', () => {
env.tsconfig({'allowEmptyCodegenFiles': true});
env.tsconfig({'generateNgFactoryShims': true});

env.write('test.ts', `
import {Injectable, NgModule} from '@angular/core';
Expand Down Expand Up @@ -2374,126 +2374,130 @@ runInEachFileSystem(os => {
expect(emptyFactory).toContain(`export var \u0275NonEmptyModule = true;`);
});

it('should generate correct type annotation for NgModuleFactory calls in ngfactories', () => {
env.tsconfig({'allowEmptyCodegenFiles': true});
env.write('test.ts', `
import {Component} from '@angular/core';
@Component({
selector: 'test',
template: '...',
})
export class TestCmp {}
`);
env.driveMain();

const ngfactoryContents = env.getContents('test.ngfactory.d.ts');
expect(ngfactoryContents).toContain(`i0.ɵNgModuleFactory<any>`);
});

it('should copy a top-level comment into a factory stub', () => {
env.tsconfig({'allowEmptyCodegenFiles': true});
describe('ngfactory shims', () => {
beforeEach(() => { env.tsconfig({'generateNgFactoryShims': true}); });

env.write('test.ts', `/** I am a top-level comment. */
import {NgModule} from '@angular/core';
it('should generate correct type annotation for NgModuleFactory calls in ngfactories', () => {
env.write('test.ts', `
import {Component} from '@angular/core';
@Component({
selector: 'test',
template: '...',
})
export class TestCmp {}
`);
env.driveMain();

@NgModule({})
export class TestModule {}
`);
env.driveMain();
const ngfactoryContents = env.getContents('test.ngfactory.d.ts');
expect(ngfactoryContents).toContain(`i0.ɵNgModuleFactory<any>`);
});

const factoryContents = env.getContents('test.ngfactory.js');
expect(factoryContents).toMatch(/^\/\*\* I am a top-level comment\. \*\//);
});
it('should copy a top-level comment into a factory stub', () => {
env.tsconfig({'allowEmptyCodegenFiles': true});

it('should be able to compile an app using the factory shim', () => {
env.tsconfig({'allowEmptyCodegenFiles': true});
env.write('test.ts', `/** I am a top-level comment. */
import {NgModule} from '@angular/core';
@NgModule({})
export class TestModule {}
`);
env.driveMain();

env.write('test.ts', `
export {MyModuleNgFactory} from './my-module.ngfactory';
`);
const factoryContents = env.getContents('test.ngfactory.js');
expect(factoryContents).toMatch(/^\/\*\* I am a top-level comment\. \*\//);
});

env.write('my-module.ts', `
import {NgModule} from '@angular/core';
it('should be able to compile an app using the factory shim', () => {
env.tsconfig({'allowEmptyCodegenFiles': true});

@NgModule({})
export class MyModule {}
`);
env.write('test.ts', `
export {MyModuleNgFactory} from './my-module.ngfactory';
`);

env.driveMain();
});
env.write('my-module.ts', `
import {NgModule} from '@angular/core';
@NgModule({})
export class MyModule {}
`);

it('should generate correct imports in factory stubs when compiling @angular/core', () => {
env.tsconfig({'allowEmptyCodegenFiles': true});
env.driveMain();
});

env.write('test.ts', `
import {NgModule} from '@angular/core';
it('should generate correct imports in factory stubs when compiling @angular/core', () => {
env.tsconfig({'allowEmptyCodegenFiles': true});

@NgModule({})
export class TestModule {}
`);
env.write('test.ts', `
import {NgModule} from '@angular/core';
@NgModule({})
export class TestModule {}
`);

// Trick the compiler into thinking it's compiling @angular/core.
env.write('r3_symbols.ts', 'export const ITS_JUST_ANGULAR = true;');
// Trick the compiler into thinking it's compiling @angular/core.
env.write('r3_symbols.ts', 'export const ITS_JUST_ANGULAR = true;');

env.driveMain();
env.driveMain();

const factoryContents = env.getContents('test.ngfactory.js');
expect(normalize(factoryContents)).toBe(normalize(`
import * as i0 from "./r3_symbols";
import { TestModule } from './test';
export var TestModuleNgFactory = new i0.NgModuleFactory(TestModule);
`));
const factoryContents = env.getContents('test.ngfactory.js');
expect(normalize(factoryContents)).toBe(normalize(`
import * as i0 from "./r3_symbols";
import { TestModule } from './test';
export var TestModuleNgFactory = new i0.NgModuleFactory(TestModule);
`));
});
});

it('should generate a summary stub for decorated classes in the input file only', () => {
env.tsconfig({'allowEmptyCodegenFiles': true});

env.write('test.ts', `
import {Injectable, NgModule} from '@angular/core';

export class NotAModule {}
describe('ngsummary shim generation', () => {
beforeEach(() => { env.tsconfig({'generateNgSummaryShims': true}); });

@NgModule({})
export class TestModule {}
`);
it('should generate a summary stub for decorated classes in the input file only', () => {
env.write('test.ts', `
import {Injectable, NgModule} from '@angular/core';
export class NotAModule {}
@NgModule({})
export class TestModule {}
`);

env.driveMain();
env.driveMain();

const summaryContents = env.getContents('test.ngsummary.js');
expect(summaryContents).toEqual(`export var TestModuleNgSummary = null;\n`);
});
const summaryContents = env.getContents('test.ngsummary.js');
expect(summaryContents).toEqual(`export var TestModuleNgSummary = null;\n`);
});

it('should generate a summary stub for classes exported via exports', () => {
env.tsconfig({'allowEmptyCodegenFiles': true});
it('should generate a summary stub for classes exported via exports', () => {
env.write('test.ts', `
import {Injectable, NgModule} from '@angular/core';
@NgModule({})
class NotDirectlyExported {}
export {NotDirectlyExported};
`);

env.write('test.ts', `
import {Injectable, NgModule} from '@angular/core';
env.driveMain();

@NgModule({})
class NotDirectlyExported {}
const summaryContents = env.getContents('test.ngsummary.js');
expect(summaryContents).toEqual(`export var NotDirectlyExportedNgSummary = null;\n`);
});

export {NotDirectlyExported};
`);
it('it should generate empty export when there are no other summary symbols, to ensure the output is a valid ES module',
() => {
env.write('empty.ts', `
export class NotAModule {}
`);

env.driveMain();
env.driveMain();

const summaryContents = env.getContents('test.ngsummary.js');
expect(summaryContents).toEqual(`export var NotDirectlyExportedNgSummary = null;\n`);
const emptySummary = env.getContents('empty.ngsummary.js');
// The empty export ensures this js file is still an ES module.
expect(emptySummary).toEqual(`export var \u0275empty = null;\n`);
});
});

it('it should generate empty export when there are no other summary symbols, to ensure the output is a valid ES module',
() => {
env.tsconfig({'allowEmptyCodegenFiles': true});
env.write('empty.ts', `
export class NotAModule {}
`);

env.driveMain();

const emptySummary = env.getContents('empty.ngsummary.js');
// The empty export ensures this js file is still an ES module.
expect(emptySummary).toEqual(`export var \u0275empty = null;\n`);
});

it('should compile a banana-in-a-box inside of a template', () => {
env.write('test.ts', `
Expand Down Expand Up @@ -2963,13 +2967,13 @@ runInEachFileSystem(os => {
it('should compile programs with typeRoots', () => {
// Write out a custom tsconfig.json that includes 'typeRoots' and 'files'. 'files' is
// necessary because otherwise TS picks up the testTypeRoot/test/index.d.ts file into the
// program automatically. Shims are also turned on (via allowEmptyCodegenFiles) because the
// shim ts.CompilerHost wrapper can break typeRoot functionality (which this test is meant to
// detect).
// program automatically. Shims are also turned on because the shim ts.CompilerHost wrapper
// can break typeRoot functionality (which this test is meant to detect).
env.write('tsconfig.json', `{
"extends": "./tsconfig-base.json",
"angularCompilerOptions": {
"allowEmptyCodegenFiles": true
"generateNgFactoryShims": true,
"generateNgSummaryShims": true,
},
"compilerOptions": {
"typeRoots": ["./testTypeRoot"],
Expand Down

0 comments on commit efb99f1

Please sign in to comment.