From 91b7152852e86311a9c880e558ae0caefc7e5e26 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Tue, 12 Feb 2019 23:29:28 +0100 Subject: [PATCH] feat(compiler-cli): no longer re-export external symbols by default (#28633) With #28594 we refactored the `@angular/compiler` slightly to allow opting out from external symbol re-exports which are enabled by default. Since symbol re-exports only benefit projects which have a very strict dependency enforcement, external symbols should not be re-exported by default as this could grow the size of factory files and cause unexpected behavior with Angular's AOT symbol resolving (e.g. see: #25644). Note that the common strict dependency enforcement for source files does still work with external symbol re-exports disabled, but there are also strict dependency checks that enforce strict module dependencies also for _generated files_ (such as the ngfactory files). This is how Google3 manages it's dependencies and therefore external symbol re-exports need to be enabled within Google3. Also "ngtsc" also does not provide any way of using external symbol re-exports, so this means that with this change, NGC can partially match the behavior of "ngtsc" then (unless explicitly opted-out). As mentioned before, internally at Google symbol re-exports need to be still enabled, so the `ng_module` Bazel rule will enable the symbol re-exports by default when running within Blaze. Fixes #25644. PR Close #28633 --- packages/bazel/src/ng_module.bzl | 7 + packages/compiler-cli/src/transformers/api.ts | 9 + .../compiler-cli/src/transformers/program.ts | 1 + packages/compiler-cli/test/ngc_spec.ts | 221 +++++++++++++--- .../compiler/src/aot/summary_serializer.ts | 2 +- packages/compiler/test/aot/compiler_spec.ts | 244 +++++++++--------- .../compiler/test/aot/jit_summaries_spec.ts | 123 ++++++++- .../test/aot/summary_resolver_spec.ts | 9 +- .../test/aot/summary_serializer_spec.ts | 241 ++++++++--------- 9 files changed, 577 insertions(+), 280 deletions(-) diff --git a/packages/bazel/src/ng_module.bzl b/packages/bazel/src/ng_module.bzl index 8cb4639665..54abf74c97 100644 --- a/packages/bazel/src/ng_module.bzl +++ b/packages/bazel/src/ng_module.bzl @@ -245,6 +245,13 @@ def _ngc_tsconfig(ctx, files, srcs, **kwargs): "enableSummariesForJit": is_legacy_ngc, "enableIvy": _enable_ivy_value(ctx), "fullTemplateTypeCheck": ctx.attr.type_check, + # In Google3 we still want to use the symbol factory re-exports in order to + # not break existing apps inside Google. Unlike Bazel, Google3 does not only + # enforce strict dependencies of source files, but also for generated files + # (such as the factory files). Therefore in order to avoid that generated files + # introduce new module dependencies (which aren't explicitly declared), we need + # to enable external symbol re-exports by default when running with Blaze. + "createExternalSymbolFactoryReexports": (not _is_bazel()), # FIXME: wrong place to de-dupe "expectedOut": depset([o.path for o in expected_outs]).to_list(), } diff --git a/packages/compiler-cli/src/transformers/api.ts b/packages/compiler-cli/src/transformers/api.ts index b0fa3e193e..1ca98f4034 100644 --- a/packages/compiler-cli/src/transformers/api.ts +++ b/packages/compiler-cli/src/transformers/api.ts @@ -199,6 +199,15 @@ export interface CompilerOptions extends ts.CompilerOptions { /** @internal */ collectAllErrors?: boolean; + + /** + * Whether NGC should generate re-exports for external symbols which are referenced + * in Angular metadata (e.g. @Component, @Inject, @ViewChild). This can be enabled in + * order to avoid dynamically generated module dependencies which can break strict + * dependency enforcements. This is not enabled by default. + * Read more about this here: https://github.com/angular/angular/issues/25644. + */ + createExternalSymbolFactoryReexports?: boolean; } export interface CompilerHost extends ts.CompilerHost { diff --git a/packages/compiler-cli/src/transformers/program.ts b/packages/compiler-cli/src/transformers/program.ts index 85574b48a3..74dec4192a 100644 --- a/packages/compiler-cli/src/transformers/program.ts +++ b/packages/compiler-cli/src/transformers/program.ts @@ -942,6 +942,7 @@ function getAotCompilerOptions(options: CompilerOptions): AotCompilerOptions { fullTemplateTypeCheck: options.fullTemplateTypeCheck, allowEmptyCodegenFiles: options.allowEmptyCodegenFiles, enableIvy: options.enableIvy, + createExternalSymbolFactoryReexports: options.createExternalSymbolFactoryReexports, }; } diff --git a/packages/compiler-cli/test/ngc_spec.ts b/packages/compiler-cli/test/ngc_spec.ts index 89d388cf0f..d3de276a32 100644 --- a/packages/compiler-cli/test/ngc_spec.ts +++ b/packages/compiler-cli/test/ngc_spec.ts @@ -1076,15 +1076,17 @@ describe('ngc transformer command-line', () => { }); }); - it('should be able to compile multiple libraries with summaries', () => { - // Note: we need to emit the generated code for the libraries - // into the node_modules, as that is the only way that we - // currently support when using summaries. - // TODO(tbosch): add support for `paths` to our CompilerHost.fileNameToModuleName - // and then use `paths` here instead of writing to node_modules. + describe('with external symbol re-exports enabled', () => { - // Angular - write('tsconfig-ng.json', `{ + it('should be able to compile multiple libraries with summaries', () => { + // Note: we need to emit the generated code for the libraries + // into the node_modules, as that is the only way that we + // currently support when using summaries. + // TODO(tbosch): add support for `paths` to our CompilerHost.fileNameToModuleName + // and then use `paths` here instead of writing to node_modules. + + // Angular + write('tsconfig-ng.json', `{ "extends": "./tsconfig-base.json", "angularCompilerOptions": { "generateCodeForLibraries": true, @@ -1100,69 +1102,68 @@ describe('ngc transformer command-line', () => { ] }`); - // Lib 1 - write('lib1/tsconfig-lib1.json', `{ + // Lib 1 + write('lib1/tsconfig-lib1.json', `{ "extends": "../tsconfig-base.json", "angularCompilerOptions": { "generateCodeForLibraries": false, - "enableSummariesForJit": true + "enableSummariesForJit": true, + "createExternalSymbolFactoryReexports": true }, "compilerOptions": { "rootDir": ".", "outDir": "../node_modules/lib1_built" } }`); - write('lib1/module.ts', ` + write('lib1/module.ts', ` import {NgModule} from '@angular/core'; - export function someFactory(): any { return null; } - @NgModule({ providers: [{provide: 'foo', useFactory: someFactory}] }) export class Module {} `); - write('lib1/class1.ts', `export class Class1 {}`); + write('lib1/class1.ts', `export class Class1 {}`); - // Lib 2 - write('lib2/tsconfig-lib2.json', `{ + // Lib 2 + write('lib2/tsconfig-lib2.json', `{ "extends": "../tsconfig-base.json", "angularCompilerOptions": { "generateCodeForLibraries": false, - "enableSummariesForJit": true + "enableSummariesForJit": true, + "createExternalSymbolFactoryReexports": true }, "compilerOptions": { "rootDir": ".", "outDir": "../node_modules/lib2_built" } }`); - write('lib2/module.ts', ` + write('lib2/module.ts', ` export {Module} from 'lib1_built/module'; `); - write('lib2/class2.ts', ` - import {Class1} from 'lib1_built/class1'; + write('lib2/class2.ts', ` + import {Class1} from 'lib1_built/class1'; + export class Class2 { + constructor(class1: Class1) {} + } + `); - export class Class2 { - constructor(class1: Class1) {} - } - `); - - // Application - write('app/tsconfig-app.json', `{ + // Application + write('app/tsconfig-app.json', `{ "extends": "../tsconfig-base.json", "angularCompilerOptions": { "generateCodeForLibraries": false, - "enableSummariesForJit": true + "enableSummariesForJit": true, + "createExternalSymbolFactoryReexports": true }, "compilerOptions": { "rootDir": ".", "outDir": "../built/app" } }`); - write('app/main.ts', ` + write('app/main.ts', ` import {NgModule, Inject} from '@angular/core'; import {Module} from 'lib2_built/module'; - @NgModule({ imports: [Module] }) @@ -1171,6 +1172,149 @@ describe('ngc transformer command-line', () => { } `); + expect(main(['-p', path.join(basePath, 'lib1', 'tsconfig-lib1.json')], errorSpy)).toBe(0); + expect(main(['-p', path.join(basePath, 'lib2', 'tsconfig-lib2.json')], errorSpy)).toBe(0); + expect(main(['-p', path.join(basePath, 'app', 'tsconfig-app.json')], errorSpy)).toBe(0); + + // library 1 + // make `shouldExist` / `shouldNotExist` relative to `node_modules` + outDir = path.resolve(basePath, 'node_modules'); + shouldExist('lib1_built/module.js'); + shouldExist('lib1_built/module.ngsummary.json'); + shouldExist('lib1_built/module.ngsummary.js'); + shouldExist('lib1_built/module.ngsummary.d.ts'); + shouldExist('lib1_built/module.ngfactory.js'); + shouldExist('lib1_built/module.ngfactory.d.ts'); + + // library 2 + // make `shouldExist` / `shouldNotExist` relative to `node_modules` + outDir = path.resolve(basePath, 'node_modules'); + shouldExist('lib2_built/module.js'); + shouldExist('lib2_built/module.ngsummary.json'); + shouldExist('lib2_built/module.ngsummary.js'); + shouldExist('lib2_built/module.ngsummary.d.ts'); + shouldExist('lib2_built/module.ngfactory.js'); + shouldExist('lib2_built/module.ngfactory.d.ts'); + + shouldExist('lib2_built/class2.ngsummary.json'); + shouldNotExist('lib2_built/class2.ngsummary.js'); + shouldNotExist('lib2_built/class2.ngsummary.d.ts'); + shouldExist('lib2_built/class2.ngfactory.js'); + shouldExist('lib2_built/class2.ngfactory.d.ts'); + + // app + // make `shouldExist` / `shouldNotExist` relative to `built` + outDir = path.resolve(basePath, 'built'); + shouldExist('app/main.js'); + }); + + it('should create external symbol re-exports', () => { + writeConfig(`{ + "extends": "./tsconfig-base.json", + "angularCompilerOptions": { + "generateCodeForLibraries": false, + "createExternalSymbolFactoryReexports": true + } + }`); + + write('test.ts', ` + import {Injectable, NgZone} from '@angular/core'; + + @Injectable({providedIn: 'root'}) + export class MyService { + constructor(public ngZone: NgZone) {} + } + `); + + expect(main(['-p', basePath], errorSpy)).toBe(0); + + shouldExist('test.js'); + shouldExist('test.metadata.json'); + shouldExist('test.ngsummary.json'); + shouldExist('test.ngfactory.js'); + shouldExist('test.ngfactory.d.ts'); + + const summaryJson = require(path.join(outDir, 'test.ngsummary.json')); + const factoryOutput = fs.readFileSync(path.join(outDir, 'test.ngfactory.js'), 'utf8'); + + expect(summaryJson['symbols'][0].name).toBe('MyService'); + expect(summaryJson['symbols'][1]) + .toEqual(jasmine.objectContaining({name: 'NgZone', importAs: 'NgZone_1'})); + + expect(factoryOutput).toContain(`export { NgZone as NgZone_1 } from "@angular/core";`); + }); + }); + + it('should be able to compile multiple libraries with summaries', () => { + // Lib 1 + write('lib1/tsconfig-lib1.json', `{ + "extends": "../tsconfig-base.json", + "angularCompilerOptions": { + "generateCodeForLibraries": false, + "enableSummariesForJit": true + }, + "compilerOptions": { + "rootDir": ".", + "outDir": "../node_modules/lib1_built" + } + }`); + write('lib1/module.ts', ` + import {NgModule} from '@angular/core'; + + export function someFactory(): any { return null; } + + @NgModule({ + providers: [{provide: 'foo', useFactory: someFactory}] + }) + export class Module {} + `); + write('lib1/class1.ts', `export class Class1 {}`); + + // Lib 2 + write('lib2/tsconfig-lib2.json', `{ + "extends": "../tsconfig-base.json", + "angularCompilerOptions": { + "generateCodeForLibraries": false, + "enableSummariesForJit": true + }, + "compilerOptions": { + "rootDir": ".", + "outDir": "../node_modules/lib2_built" + } + }`); + write('lib2/module.ts', `export {Module} from 'lib1_built/module';`); + write('lib2/class2.ts', ` + import {Class1} from 'lib1_built/class1'; + + export class Class2 { + constructor(class1: Class1) {} + } + `); + + // Application + write('app/tsconfig-app.json', `{ + "extends": "../tsconfig-base.json", + "angularCompilerOptions": { + "generateCodeForLibraries": false, + "enableSummariesForJit": true + }, + "compilerOptions": { + "rootDir": ".", + "outDir": "../built/app" + } + }`); + write('app/main.ts', ` + import {NgModule, Inject} from '@angular/core'; + import {Module} from 'lib2_built/module'; + + @NgModule({ + imports: [Module] + }) + export class AppModule { + constructor(@Inject('foo') public foo: any) {} + } + `); + expect(main(['-p', path.join(basePath, 'lib1', 'tsconfig-lib1.json')], errorSpy)).toBe(0); expect(main(['-p', path.join(basePath, 'lib2', 'tsconfig-lib2.json')], errorSpy)).toBe(0); expect(main(['-p', path.join(basePath, 'app', 'tsconfig-app.json')], errorSpy)).toBe(0); @@ -1189,17 +1333,24 @@ describe('ngc transformer command-line', () => { // make `shouldExist` / `shouldNotExist` relative to `node_modules` outDir = path.resolve(basePath, 'node_modules'); shouldExist('lib2_built/module.js'); + + // "module.ts" re-exports an external symbol and will therefore + // have a summary JSON file and its corresponding JIT summary. shouldExist('lib2_built/module.ngsummary.json'); shouldExist('lib2_built/module.ngsummary.js'); shouldExist('lib2_built/module.ngsummary.d.ts'); - shouldExist('lib2_built/module.ngfactory.js'); - shouldExist('lib2_built/module.ngfactory.d.ts'); + // "module.ts" only re-exports an external symbol and the AOT compiler does not + // need to generate anything. Therefore there should be no factory files. + shouldNotExist('lib2_built/module.ngfactory.js'); + shouldNotExist('lib2_built/module.ngfactory.d.ts'); shouldExist('lib2_built/class2.ngsummary.json'); shouldNotExist('lib2_built/class2.ngsummary.js'); shouldNotExist('lib2_built/class2.ngsummary.d.ts'); - shouldExist('lib2_built/class2.ngfactory.js'); - shouldExist('lib2_built/class2.ngfactory.d.ts'); + // We don't expect factories here because the "class2.ts" file + // just exports a class that does not produce any AOT code. + shouldNotExist('lib2_built/class2.ngfactory.js'); + shouldNotExist('lib2_built/class2.ngfactory.d.ts'); // app // make `shouldExist` / `shouldNotExist` relative to `built` diff --git a/packages/compiler/src/aot/summary_serializer.ts b/packages/compiler/src/aot/summary_serializer.ts index ae63e6cf0a..f8981565a9 100644 --- a/packages/compiler/src/aot/summary_serializer.ts +++ b/packages/compiler/src/aot/summary_serializer.ts @@ -23,7 +23,7 @@ export function serializeSummaries( CompileTypeMetadata }[], createExternalSymbolReexports = - true): {json: string, exportAs: {symbol: StaticSymbol, exportAs: string}[]} { + false): {json: string, exportAs: {symbol: StaticSymbol, exportAs: string}[]} { const toJsonSerializer = new ToJsonSerializer(symbolResolver, summaryResolver, srcFileName); // for symbols, we use everything except for the class metadata itself diff --git a/packages/compiler/test/aot/compiler_spec.ts b/packages/compiler/test/aot/compiler_spec.ts index 0338985f67..9d020e581c 100644 --- a/packages/compiler/test/aot/compiler_spec.ts +++ b/packages/compiler/test/aot/compiler_spec.ts @@ -486,130 +486,142 @@ describe('compiler (unbundled Angular)', () => { inheritanceWithSummariesSpecs(() => angularSummaryFiles); - it('should not reexport type symbols mentioned in constructors', () => { - const libInput: MockDirectory = { - 'lib': { - 'base.ts': ` - export class AValue {} - export type AType = {}; + describe('external symbol re-exports enabled', () => { - export class AClass { - constructor(a: AType, b: AValue) {} - } - ` - } - }; - const appInput: MockDirectory = { - 'app': { - 'main.ts': ` - export {AClass} from '../lib/base'; - ` - } - }; + it('should not reexport type symbols mentioned in constructors', () => { + const libInput: MockDirectory = { + 'lib': { + 'base.ts': ` + export class AValue {} + export type AType = {}; + + export class AClass { + constructor(a: AType, b: AValue) {} + } + ` + } + }; + const appInput: MockDirectory = { + 'app': { + 'main.ts': ` + export {AClass} from '../lib/base'; + ` + } + }; - const {outDir: libOutDir} = compile([libInput, angularSummaryFiles], {useSummaries: true}); - const {genFiles: appGenFiles} = - compile([appInput, libOutDir, angularSummaryFiles], {useSummaries: true}); - const appNgFactory = appGenFiles.find((f) => f.genFileUrl === '/app/main.ngfactory.ts') !; - const appNgFactoryTs = toTypeScript(appNgFactory); - expect(appNgFactoryTs).not.toContain('AType'); - expect(appNgFactoryTs).toContain('AValue'); - }); + const {outDir: libOutDir} = compile( + [libInput, angularSummaryFiles], + {useSummaries: true, createExternalSymbolFactoryReexports: true}); + const {genFiles: appGenFiles} = compile( + [appInput, libOutDir, angularSummaryFiles], + {useSummaries: true, createExternalSymbolFactoryReexports: true}); + const appNgFactory = appGenFiles.find((f) => f.genFileUrl === '/app/main.ngfactory.ts') !; + const appNgFactoryTs = toTypeScript(appNgFactory); + expect(appNgFactoryTs).not.toContain('AType'); + expect(appNgFactoryTs).toContain('AValue'); + }); - it('should not reexport complex function calls', () => { - const libInput: MockDirectory = { - 'lib': { - 'base.ts': ` - export class AClass { - constructor(arg: any) {} + it('should not reexport complex function calls', () => { + const libInput: MockDirectory = { + 'lib': { + 'base.ts': ` + export class AClass { + constructor(arg: any) {} + + static create(arg: any = null): AClass { return new AClass(arg); } + + call(arg: any) {} + } + + export function simple(arg: any) { return [arg]; } + + export const ctor_arg = {}; + export const ctor_call = new AClass(ctor_arg); + + export const static_arg = {}; + export const static_call = AClass.create(static_arg); + + export const complex_arg = {}; + export const complex_call = AClass.create().call(complex_arg); + + export const simple_arg = {}; + export const simple_call = simple(simple_arg); + ` + } + }; + const appInput: MockDirectory = { + 'app': { + 'main.ts': ` + import {ctor_call, static_call, complex_call, simple_call} from '../lib/base'; + + export const calls = [ctor_call, static_call, complex_call, simple_call]; + `, + } + }; - static create(arg: any = null): AClass { return new AClass(arg); } + const {outDir: libOutDir} = compile( + [libInput, angularSummaryFiles], + {useSummaries: true, createExternalSymbolFactoryReexports: true}); + const {genFiles: appGenFiles} = compile( + [appInput, libOutDir, angularSummaryFiles], + {useSummaries: true, createExternalSymbolFactoryReexports: true}); + const appNgFactory = appGenFiles.find((f) => f.genFileUrl === '/app/main.ngfactory.ts') !; + const appNgFactoryTs = toTypeScript(appNgFactory); - call(arg: any) {} - } + // metadata of ctor calls is preserved, so we reexport the argument + expect(appNgFactoryTs).toContain('ctor_arg'); + expect(appNgFactoryTs).toContain('ctor_call'); - export function simple(arg: any) { return [arg]; } + // metadata of static calls is preserved, so we reexport the argument + expect(appNgFactoryTs).toContain('static_arg'); + expect(appNgFactoryTs).toContain('AClass'); + expect(appNgFactoryTs).toContain('static_call'); - export const ctor_arg = {}; - export const ctor_call = new AClass(ctor_arg); + // metadata of complex calls is elided, so we don't reexport the argument + expect(appNgFactoryTs).not.toContain('complex_arg'); + expect(appNgFactoryTs).toContain('complex_call'); - export const static_arg = {}; - export const static_call = AClass.create(static_arg); + // metadata of simple calls is preserved, so we reexport the argument + expect(appNgFactoryTs).toContain('simple_arg'); + expect(appNgFactoryTs).toContain('simple_call'); + }); - export const complex_arg = {}; - export const complex_call = AClass.create().call(complex_arg); + it('should not reexport already exported symbols except for lowered symbols', () => { + const libInput: MockDirectory = { + 'lib': { + 'base.ts': ` + export const exportedVar = 1; - export const simple_arg = {}; - export const simple_call = simple(simple_arg); - ` - } - }; - const appInput: MockDirectory = { - 'app': { - 'main.ts': ` - import {ctor_call, static_call, complex_call, simple_call} from '../lib/base'; + // A symbol introduced by lowering expressions + export const ɵ1 = 'lowered symbol'; + ` + } + }; + const appInput: MockDirectory = { + 'app': { + 'main.ts': `export * from '../lib/base';`, + } + }; - export const calls = [ctor_call, static_call, complex_call, simple_call]; - `, - } - }; + const {outDir: libOutDir} = compile( + [libInput, angularSummaryFiles], + {useSummaries: true, createExternalSymbolFactoryReexports: true}); + const {genFiles: appGenFiles} = compile( + [appInput, libOutDir, angularSummaryFiles], + {useSummaries: true, createExternalSymbolFactoryReexports: true}); + const appNgFactory = appGenFiles.find((f) => f.genFileUrl === '/app/main.ngfactory.ts') !; + const appNgFactoryTs = toTypeScript(appNgFactory); - const {outDir: libOutDir} = compile([libInput, angularSummaryFiles], {useSummaries: true}); - const {genFiles: appGenFiles} = - compile([appInput, libOutDir, angularSummaryFiles], {useSummaries: true}); - const appNgFactory = appGenFiles.find((f) => f.genFileUrl === '/app/main.ngfactory.ts') !; - const appNgFactoryTs = toTypeScript(appNgFactory); + // we don't need to reexport exported symbols via the .ngfactory + // as we can refer to them via the reexport. + expect(appNgFactoryTs).not.toContain('exportedVar'); - // metadata of ctor calls is preserved, so we reexport the argument - expect(appNgFactoryTs).toContain('ctor_arg'); - expect(appNgFactoryTs).toContain('ctor_call'); - - // metadata of static calls is preserved, so we reexport the argument - expect(appNgFactoryTs).toContain('static_arg'); - expect(appNgFactoryTs).toContain('AClass'); - expect(appNgFactoryTs).toContain('static_call'); - - // metadata of complex calls is elided, so we don't reexport the argument - expect(appNgFactoryTs).not.toContain('complex_arg'); - expect(appNgFactoryTs).toContain('complex_call'); - - // metadata of simple calls is preserved, so we reexport the argument - expect(appNgFactoryTs).toContain('simple_arg'); - expect(appNgFactoryTs).toContain('simple_call'); - }); - - it('should not reexport already exported symbols except for lowered symbols', () => { - const libInput: MockDirectory = { - 'lib': { - 'base.ts': ` - export const exportedVar = 1; - - // A symbol introduced by lowering expressions - export const ɵ1 = 'lowered symbol'; - ` - } - }; - const appInput: MockDirectory = { - 'app': { - 'main.ts': `export * from '../lib/base';`, - } - }; - - const {outDir: libOutDir} = compile([libInput, angularSummaryFiles], {useSummaries: true}); - const {genFiles: appGenFiles} = - compile([appInput, libOutDir, angularSummaryFiles], {useSummaries: true}); - const appNgFactory = appGenFiles.find((f) => f.genFileUrl === '/app/main.ngfactory.ts') !; - const appNgFactoryTs = toTypeScript(appNgFactory); - - // we don't need to reexport exported symbols via the .ngfactory - // as we can refer to them via the reexport. - expect(appNgFactoryTs).not.toContain('exportedVar'); - - // although ɵ1 is reexported via `export *`, we still need to reexport it - // via the .ngfactory as tsickle expands `export *` into named exports, - // and doesn't know about our lowered symbols as we introduce them - // after the typecheck phase. - expect(appNgFactoryTs).toContain('ɵ1'); + // although ɵ1 is reexported via `export *`, we still need to reexport it + // via the .ngfactory as tsickle expands `export *` into named exports, + // and doesn't know about our lowered symbols as we introduce them + // after the typecheck phase. + expect(appNgFactoryTs).toContain('ɵ1'); + }); }); }); @@ -741,12 +753,14 @@ describe('compiler (unbundled Angular)', () => { compile([lib1Input, getAngularSummaryFiles()], {useSummaries: true}); const {outDir: lib2OutDir} = compile([lib1OutDir, lib2Input, getAngularSummaryFiles()], {useSummaries: true}); - const {genFiles} = - compile([lib2OutDir, appInput, getAngularSummaryFiles()], {useSummaries: true}); + const {genFiles} = compile( + [lib1OutDir, lib2OutDir, appInput, getAngularSummaryFiles()], {useSummaries: true}); + const mainNgFactory = genFiles.find(gf => gf.srcFileUrl === '/app/main.ts') !; const flags = NodeFlags.TypeDirective | NodeFlags.Component | NodeFlags.OnDestroy; - expect(toTypeScript(mainNgFactory)) - .toContain(`${flags},(null as any),0,i1.Extends,[i2.AParam_2]`); + const mainNgFactorySource = toTypeScript(mainNgFactory); + expect(mainNgFactorySource).toContain(`import * as i2 from '/lib1/base';`); + expect(mainNgFactorySource).toContain(`${flags},(null as any),0,i1.Extends,[i2.AParam]`); }); describe('Injectable', () => { diff --git a/packages/compiler/test/aot/jit_summaries_spec.ts b/packages/compiler/test/aot/jit_summaries_spec.ts index 025aa243ee..08bbdd27df 100644 --- a/packages/compiler/test/aot/jit_summaries_spec.ts +++ b/packages/compiler/test/aot/jit_summaries_spec.ts @@ -18,7 +18,8 @@ describe('aot summaries for jit', () => { angularSummaryFiles = compile(angularFiles, {useSummaries: false, emit: true}).outDir; }); - function compileApp(rootDir: MockDirectory, options: {useSummaries?: boolean} = {}): + function compileApp( + rootDir: MockDirectory, options: {useSummaries?: boolean}& AotCompilerOptions = {}): {genFiles: GeneratedFile[], outDir: MockDirectory} { return compile( [rootDir, options.useSummaries ? angularSummaryFiles : angularFiles], @@ -255,7 +256,7 @@ describe('aot summaries for jit', () => { }); it('should create and use reexports for imported NgModules ' + - 'across compilation units', + 'across compilation units if symbol re-exports are enabled', () => { const lib1In = { 'lib1': { @@ -275,7 +276,10 @@ describe('aot summaries for jit', () => { `, } }; - const {outDir: lib2In, genFiles: lib1Gen} = compileApp(lib1In, {useSummaries: true}); + const {outDir: lib2In, genFiles: lib1Gen} = compileApp(lib1In, { + useSummaries: true, + createExternalSymbolFactoryReexports: true, + }); lib2In['lib2'] = { 'module.ts': ` @@ -292,7 +296,10 @@ describe('aot summaries for jit', () => { export const reexports: any[] = [ reexports_lib1 ]; `, }; - const {outDir: lib3In, genFiles: lib2Gen} = compileApp(lib2In, {useSummaries: true}); + const {outDir: lib3In, genFiles: lib2Gen} = compileApp(lib2In, { + useSummaries: true, + createExternalSymbolFactoryReexports: true, + }); const lib2ModuleNgSummary = lib2Gen.find(f => f.genFileUrl === '/lib2/module.ngsummary.ts') !; @@ -325,7 +332,10 @@ describe('aot summaries for jit', () => { `, }; - const lib3Gen = compileApp(lib3In, {useSummaries: true}).genFiles; + const lib3Gen = compileApp(lib3In, { + useSummaries: true, + createExternalSymbolFactoryReexports: true + }).genFiles; const lib3ModuleNgSummary = lib3Gen.find(f => f.genFileUrl === '/lib3/module.ngsummary.ts') !; const lib3ReexportNgSummary = @@ -349,4 +359,107 @@ describe('aot summaries for jit', () => { .toContain( `export {ReexportModule_2NgSummary as ReexportModule_3NgSummary} from '/lib2/reexport.ngsummary'`); }); + + it('should not create reexports for external symbols imported by NgModules', () => { + const lib1In = { + 'lib1': { + 'module.ts': ` + import { NgModule } from '@angular/core'; + + @NgModule() + export class Lib1Module {}`, + 'reexport.ts': ` + import { NgModule } from '@angular/core'; + + @NgModule() + export class ReexportModule {} + + export const reexports: any[] = [ ReexportModule ];`, + } + }; + const {outDir: lib1Out} = compileApp(lib1In, {useSummaries: true}); + + const lib2In = { + ...lib1Out, + 'lib2': { + 'module.ts': ` + import { NgModule } from '@angular/core'; + import { Lib1Module } from '../lib1/module'; + + @NgModule({ + imports: [Lib1Module] + }) + export class Lib2Module {}`, + 'reexport.ts': ` + import { reexports as reexports_lib1 } from '../lib1/reexport'; + export const reexports: any[] = [ reexports_lib1 ];`, + }, + }; + + const {outDir: lib2Out, genFiles: lib2Gen} = compileApp(lib2In, {useSummaries: true}); + + const lib2ModuleNgSummary = lib2Gen.find(f => f.genFileUrl === '/lib2/module.ngsummary.ts') !; + const lib2ReexportNgSummary = + lib2Gen.find(f => f.genFileUrl === '/lib2/reexport.ngsummary.ts') !; + + // ngsummaries should not add reexports by default for imported NgModules from a direct + // dependency + expect(toTypeScript(lib2ModuleNgSummary)) + .toContain( + `export {Lib1ModuleNgSummary as Lib1ModuleNgSummary} from '/lib1/module.ngsummary'`); + // ngsummaries should not add reexports by default for reexported values from a direct + // dependency. + expect(toTypeScript(lib2ReexportNgSummary)) + .toContain( + `export {ReexportModuleNgSummary as ReexportModuleNgSummary} from '/lib1/reexport.ngsummary'`); + + const lib3In = { + ...lib1Out, + ...lib2Out, + 'lib3': { + 'module.ts': ` + import { NgModule } from '@angular/core'; + import { Lib2Module } from '../lib2/module'; + import { reexports } from '../lib2/reexport'; + + @NgModule({ + imports: [Lib2Module, reexports] + }) + export class Lib3Module {} + `, + 'reexport.ts': ` + import { reexports as reexports_lib2 } from '../lib2/reexport'; + export const reexports: any[] = [ reexports_lib2 ]; + `, + }, + }; + + const lib3Gen = compileApp(lib3In, {useSummaries: true}).genFiles; + const lib3ModuleNgSummary = lib3Gen.find(f => f.genFileUrl === '/lib3/module.ngsummary.ts') !; + const lib3ReexportNgSummary = + lib3Gen.find(f => f.genFileUrl === '/lib3/reexport.ngsummary.ts') !; + + // ngsummary.ts files should use the external symbols which are manually re-exported from + // "lib2" from their original symbol location. With re-exported external symbols this would + // be different because there would be no *direct* dependency on "lib1" at all. + const lib3ModuleNgSummarySource = toTypeScript(lib3ModuleNgSummary); + expect(lib3ModuleNgSummarySource).toContain(`import * as i1 from '/lib1/module';`); + expect(lib3ModuleNgSummarySource).toContain(`import * as i3 from '/lib1/reexport';`); + expect(lib3ModuleNgSummarySource) + .toMatch(/export function Lib3ModuleNgSummary\(\).*modules:\[{reference:i1\.Lib1Module,/s); + expect(lib3ModuleNgSummarySource) + .toMatch(/export function Lib3ModuleNgSummary\(\).*reference:i3\.ReexportModule,/s); + // ngsummaries should re-export all used summaries directly. With external symbol re-exports + // enabled, the the "lib1" summaries would be re-exported through "lib2" in order to avoid + // a *direct* dependency on "lib1". + expect(lib3ModuleNgSummarySource) + .toContain( + `export {Lib1ModuleNgSummary as Lib1ModuleNgSummary} from '/lib1/module.ngsummary';`); + expect(lib3ModuleNgSummarySource) + .toContain( + `export {ReexportModuleNgSummary as ReexportModuleNgSummary} from '/lib1/reexport.ngsummary';`); + expect(toTypeScript(lib3ReexportNgSummary)) + .toContain( + `export {ReexportModuleNgSummary as ReexportModuleNgSummary} from '/lib1/reexport.ngsummary';`); + }); }); diff --git a/packages/compiler/test/aot/summary_resolver_spec.ts b/packages/compiler/test/aot/summary_resolver_spec.ts index 75ad8f4195..d518319c2d 100644 --- a/packages/compiler/test/aot/summary_resolver_spec.ts +++ b/packages/compiler/test/aot/summary_resolver_spec.ts @@ -30,14 +30,15 @@ const EXT = /(\.d)?\.ts$/; summaryResolver = new AotSummaryResolver(host, symbolCache); } - function serialize(symbols: ResolvedStaticSymbol[]): string { + function serialize( + symbols: ResolvedStaticSymbol[], enableExternalSymbolReexports = false): string { // Note: Don't use the top level host / summaryResolver as they might not be created yet const mockSummaryResolver = new MockSummaryResolver([]); const symbolResolver = new StaticSymbolResolver( new MockStaticSymbolResolverHost({}), symbolCache, mockSummaryResolver); return serializeSummaries( 'someFile.ts', createMockOutputContext(), mockSummaryResolver, symbolResolver, - symbols, []) + symbols, [], enableExternalSymbolReexports) .json; } @@ -67,12 +68,12 @@ const EXT = /(\.d)?\.ts$/; expect(summaryResolver.getSymbolsOf('/a.d.ts')).toEqual([asymbol]); }); - it('should fill importAs for deep symbols', () => { + it('should fill importAs for deep symbols if external symbol re-exports are enabled', () => { const libSymbol = symbolCache.get('/lib.d.ts', 'Lib'); const srcSymbol = symbolCache.get('/src.ts', 'Src'); init({ '/src.ngsummary.json': - serialize([{symbol: srcSymbol, metadata: 1}, {symbol: libSymbol, metadata: 2}]) + serialize([{symbol: srcSymbol, metadata: 1}, {symbol: libSymbol, metadata: 2}], true) }); summaryResolver.getSymbolsOf('/src.d.ts'); diff --git a/packages/compiler/test/aot/summary_serializer_spec.ts b/packages/compiler/test/aot/summary_serializer_spec.ts index 0c89e4b7c8..3410c60921 100644 --- a/packages/compiler/test/aot/summary_serializer_spec.ts +++ b/packages/compiler/test/aot/summary_serializer_spec.ts @@ -317,63 +317,6 @@ import {MockAotSummaryResolverHost, createMockOutputContext} from './summary_res expect(summaries[1].metadata).toBe('someString'); }); - it('should not create "importAs" names for ctor arguments which are types of reexported classes in libraries', - () => { - init(); - const externalSerialized = serializeSummaries( - 'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, - [ - { - symbol: symbolCache.get('/tmp/external.ts', 'type'), - metadata: {__symbolic: 'interface'} - }, - { - symbol: symbolCache.get('/tmp/external.ts', 'value'), - metadata: {__symbolic: 'class'} - }, - { - symbol: symbolCache.get('/tmp/external.ts', 'reexportClass'), - metadata: { - __symbolic: 'class', - 'members': { - '__ctor__': [{ - '__symbolic': 'constructor', - 'parameters': [ - symbolCache.get('/tmp/external.ts', 'type'), - symbolCache.get('/tmp/external.ts', 'value'), - ] - }] - } - - } - }, - ], - []); - expect(externalSerialized.exportAs).toEqual([]); - init({ - '/tmp/external.ngsummary.json': externalSerialized.json, - }); - const serialized = serializeSummaries( - 'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, [{ - symbol: symbolCache.get('/tmp/test.ts', 'mainClass'), - metadata: symbolCache.get('/tmp/external.d.ts', 'reexportClass'), - }], - []); - const importAs = - deserializeSummaries(symbolCache, summaryResolver, 'someFile.d.ts', serialized.json) - .importAs; - expect(importAs).toEqual([ - { - symbol: symbolCache.get('/tmp/external.d.ts', 'reexportClass'), - importAs: symbolCache.get('/tmp/test.d.ts', 'mainClass'), - }, - { - symbol: symbolCache.get('/tmp/external.d.ts', 'value'), - importAs: symbolCache.get('someFile.ngfactory.d.ts', 'value_3'), - } - ]); - }); - it('should use existing reexports for "importAs" for symbols of libraries', () => { init(); const externalSerialized = serializeSummaries( @@ -406,31 +349,6 @@ import {MockAotSummaryResolverHost, createMockOutputContext} from './summary_res }]); }); - it('should create reexports in the ngfactory for symbols of libraries', () => { - init(); - const serialized = serializeSummaries( - 'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, [{ - symbol: symbolCache.get('/tmp/test.ts', 'main'), - metadata: [ - symbolCache.get('/tmp/external.d.ts', 'lib'), - symbolCache.get('/tmp/external.d.ts', 'lib', ['someMember']), - ] - }], - []); - // Note: no entry for the symbol with members! - expect(serialized.exportAs).toEqual([ - {symbol: symbolCache.get('/tmp/external.d.ts', 'lib'), exportAs: 'lib_1'} - ]); - - const deserialized = - deserializeSummaries(symbolCache, summaryResolver, 'someFile.d.ts', serialized.json); - // Note: no entry for the symbol with members! - expect(deserialized.importAs).toEqual([{ - symbol: symbolCache.get('/tmp/external.d.ts', 'lib'), - importAs: symbolCache.get('someFile.ngfactory.d.ts', 'lib_1') - }]); - }); - describe('with resolved symbols', () => { it('should be able to serialize a call', () => { init(); @@ -467,40 +385,67 @@ import {MockAotSummaryResolverHost, createMockOutputContext} from './summary_res }); }); - describe('createExternalSymbolReexports disabled', () => { - it('should use existing reexports for "importAs" for symbols of libraries', () => { - init(); - const externalSerialized = serializeSummaries( - 'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, - [ - {symbol: symbolCache.get('/tmp/external.ts', 'value'), metadata: 'aValue'}, - { - symbol: symbolCache.get('/tmp/external.ts', 'reexportValue'), - metadata: symbolCache.get('/tmp/external.ts', 'value') - }, - ], - [], false); - expect(externalSerialized.exportAs).toEqual([]); - init({ - '/tmp/external.ngsummary.json': externalSerialized.json, - }); - const serialized = serializeSummaries( - 'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, [{ - symbol: symbolCache.get('/tmp/test.ts', 'mainValue'), - metadata: symbolCache.get('/tmp/external.d.ts', 'reexportValue'), - }], - []); - expect(serialized.exportAs).toEqual([]); - const importAs = - deserializeSummaries(symbolCache, summaryResolver, 'someFile.d.ts', serialized.json) - .importAs; - expect(importAs).toEqual([{ - symbol: symbolCache.get('/tmp/external.d.ts', 'value'), - importAs: symbolCache.get('/tmp/test.d.ts', 'mainValue'), - }]); - }); - it('should not create reexports in the ngfactory for external symbols', () => { + describe('symbol re-exports enabled', () => { + + it('should not create "importAs" names for ctor arguments which are types of reexported classes in libraries', + () => { + init(); + const externalSerialized = serializeSummaries( + 'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, + [ + { + symbol: symbolCache.get('/tmp/external.ts', 'type'), + metadata: {__symbolic: 'interface'} + }, + { + symbol: symbolCache.get('/tmp/external.ts', 'value'), + metadata: {__symbolic: 'class'} + }, + { + symbol: symbolCache.get('/tmp/external.ts', 'reexportClass'), + metadata: { + __symbolic: 'class', + 'members': { + '__ctor__': [{ + '__symbolic': 'constructor', + 'parameters': [ + symbolCache.get('/tmp/external.ts', 'type'), + symbolCache.get('/tmp/external.ts', 'value'), + ] + }] + } + + } + }, + ], + [], true); + expect(externalSerialized.exportAs).toEqual([]); + init({ + '/tmp/external.ngsummary.json': externalSerialized.json, + }); + const serialized = serializeSummaries( + 'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, [{ + symbol: symbolCache.get('/tmp/test.ts', 'mainClass'), + metadata: symbolCache.get('/tmp/external.d.ts', 'reexportClass'), + }], + [], true); + const importAs = + deserializeSummaries(symbolCache, summaryResolver, 'someFile.d.ts', serialized.json) + .importAs; + expect(importAs).toEqual([ + { + symbol: symbolCache.get('/tmp/external.d.ts', 'reexportClass'), + importAs: symbolCache.get('/tmp/test.d.ts', 'mainClass'), + }, + { + symbol: symbolCache.get('/tmp/external.d.ts', 'value'), + importAs: symbolCache.get('someFile.ngfactory.d.ts', 'value_3'), + } + ]); + }); + + it('should create reexports in the ngfactory for symbols of libraries', () => { init(); const serialized = serializeSummaries( 'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, [{ @@ -510,14 +455,70 @@ import {MockAotSummaryResolverHost, createMockOutputContext} from './summary_res symbolCache.get('/tmp/external.d.ts', 'lib', ['someMember']), ] }], - [], false); - expect(serialized.exportAs.length) - .toBe(0, 'Expected no external symbols to be re-exported.'); + [], true); + // Note: no entry for the symbol with members! + expect(serialized.exportAs).toEqual([ + {symbol: symbolCache.get('/tmp/external.d.ts', 'lib'), exportAs: 'lib_1'} + ]); + const deserialized = deserializeSummaries(symbolCache, summaryResolver, 'someFile.d.ts', serialized.json); - expect(deserialized.importAs.length) - .toBe(0, 'Expected no symbols that can be imported from a re-exported location'); + // Note: no entry for the symbol with members! + expect(deserialized.importAs).toEqual([{ + symbol: symbolCache.get('/tmp/external.d.ts', 'lib'), + importAs: symbolCache.get('someFile.ngfactory.d.ts', 'lib_1') + }]); }); }); + + it('should use existing reexports for "importAs" for symbols of libraries', () => { + init(); + const externalSerialized = serializeSummaries( + 'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, + [ + {symbol: symbolCache.get('/tmp/external.ts', 'value'), metadata: 'aValue'}, + { + symbol: symbolCache.get('/tmp/external.ts', 'reexportValue'), + metadata: symbolCache.get('/tmp/external.ts', 'value') + }, + ], + [], false); + expect(externalSerialized.exportAs).toEqual([]); + init({ + '/tmp/external.ngsummary.json': externalSerialized.json, + }); + const serialized = serializeSummaries( + 'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, [{ + symbol: symbolCache.get('/tmp/test.ts', 'mainValue'), + metadata: symbolCache.get('/tmp/external.d.ts', 'reexportValue'), + }], + []); + expect(serialized.exportAs).toEqual([]); + const importAs = + deserializeSummaries(symbolCache, summaryResolver, 'someFile.d.ts', serialized.json) + .importAs; + expect(importAs).toEqual([{ + symbol: symbolCache.get('/tmp/external.d.ts', 'value'), + importAs: symbolCache.get('/tmp/test.d.ts', 'mainValue'), + }]); + }); + + it('should not create reexports in the ngfactory for external symbols', () => { + init(); + const serialized = serializeSummaries( + 'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, [{ + symbol: symbolCache.get('/tmp/test.ts', 'main'), + metadata: [ + symbolCache.get('/tmp/external.d.ts', 'lib'), + symbolCache.get('/tmp/external.d.ts', 'lib', ['someMember']), + ] + }], + [], false); + expect(serialized.exportAs.length).toBe(0, 'Expected no external symbols to be re-exported.'); + const deserialized = + deserializeSummaries(symbolCache, summaryResolver, 'someFile.d.ts', serialized.json); + expect(deserialized.importAs.length) + .toBe(0, 'Expected no symbols that can be imported from a re-exported location'); + }); }); }