From 45c6360e5a66968a90dba35515dd0e8e235daf28 Mon Sep 17 00:00:00 2001 From: JoostK Date: Fri, 29 Mar 2019 21:31:22 +0100 Subject: [PATCH] feat(ivy): emit module scope metadata using pure function call (#29598) Prior to this change, all module metadata would be included in the `defineNgModule` call that is set as the `ngModuleDef` field of module types. Part of the metadata is scope information like declarations, imports and exports that is used for computing the transitive module scope in JIT environments, preventing those references from being tree-shaken for production builds. This change moves the metadata for scope computations to a pure function call that patches the scope references onto the module type. Because the function is marked pure, it may be tree-shaken out during production builds such that references to declarations and exports are dropped, which in turn allows for tree-shaken any declaration that is not otherwise referenced. Fixes #28077, FW-1035 PR Close #29598 --- .../ngcc/src/rendering/renderer.ts | 19 +++-- .../ngcc/test/rendering/renderer_spec.ts | 20 ++--- .../src/ngtsc/imports/src/core.ts | 1 + .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 27 +++++-- .../compiler-cli/test/ngtsc/scope_spec.ts | 52 +++++++++++++ .../compiler/src/render3/r3_identifiers.ts | 1 + .../src/render3/r3_module_compiler.ts | 77 ++++++++++++++++--- .../core/src/core_render3_private_export.ts | 1 + packages/core/src/render3/definition.ts | 50 +++++++++++- packages/core/src/render3/index.ts | 3 +- packages/core/src/render3/jit/environment.ts | 1 + 11 files changed, 214 insertions(+), 38 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/rendering/renderer.ts b/packages/compiler-cli/ngcc/src/rendering/renderer.ts index b8fc9ae370..639606bfb7 100644 --- a/packages/compiler-cli/ngcc/src/rendering/renderer.ts +++ b/packages/compiler-cli/ngcc/src/rendering/renderer.ts @@ -487,16 +487,15 @@ export function renderDefinitions( const name = compiledClass.declaration.name; const translate = (stmt: Statement) => translateStatement(stmt, imports, NOOP_DEFAULT_IMPORT_RECORDER); - const definitions = - compiledClass.compilation - .map( - c => c.statements.map(statement => translate(statement)) - .concat(translate(createAssignmentStatement(name, c.name, c.initializer))) - .map( - statement => - printer.printNode(ts.EmitHint.Unspecified, statement, sourceFile)) - .join('\n')) - .join('\n'); + const print = (stmt: Statement) => + printer.printNode(ts.EmitHint.Unspecified, translate(stmt), sourceFile); + const definitions = compiledClass.compilation + .map( + c => [createAssignmentStatement(name, c.name, c.initializer)] + .concat(c.statements) + .map(print) + .join('\n')) + .join('\n'); return definitions; } diff --git a/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts b/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts index db9dcbff8c..86b54eb10c 100644 --- a/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts +++ b/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts @@ -151,16 +151,17 @@ describe('Renderer', () => { moduleWithProvidersAnalyses); const addDefinitionsSpy = renderer.addDefinitions as jasmine.Spy; expect(addDefinitionsSpy.calls.first().args[2]) - .toEqual(`/*@__PURE__*/ ɵngcc0.ɵsetClassMetadata(A, [{ - type: Component, - args: [{ selector: 'a', template: '{{ person!.name }}' }] - }], null, null); -A.ngComponentDef = ɵngcc0.ɵdefineComponent({ type: A, selectors: [["a"]], factory: function A_Factory(t) { return new (t || A)(); }, consts: 1, vars: 1, template: function A_Template(rf, ctx) { if (rf & 1) { + .toEqual( + `A.ngComponentDef = ɵngcc0.ɵdefineComponent({ type: A, selectors: [["a"]], factory: function A_Factory(t) { return new (t || A)(); }, consts: 1, vars: 1, template: function A_Template(rf, ctx) { if (rf & 1) { ɵngcc0.ɵtext(0); } if (rf & 2) { ɵngcc0.ɵselect(0); ɵngcc0.ɵtextBinding(0, ɵngcc0.ɵinterpolation1("", ctx.person.name, "")); - } }, encapsulation: 2 });`); + } }, encapsulation: 2 }); +/*@__PURE__*/ ɵngcc0.ɵsetClassMetadata(A, [{ + type: Component, + args: [{ selector: 'a', template: '{{ person!.name }}' }] + }], null, null);`); }); @@ -195,11 +196,12 @@ A.ngComponentDef = ɵngcc0.ɵdefineComponent({ type: A, selectors: [["a"]], fact decorators: [jasmine.objectContaining({name: 'Directive'})], })); expect(addDefinitionsSpy.calls.first().args[2]) - .toEqual(`/*@__PURE__*/ ɵngcc0.ɵsetClassMetadata(A, [{ + .toEqual( + `A.ngDirectiveDef = ɵngcc0.ɵdefineDirective({ type: A, selectors: [["", "a", ""]], factory: function A_Factory(t) { return new (t || A)(); } }); +/*@__PURE__*/ ɵngcc0.ɵsetClassMetadata(A, [{ type: Directive, args: [{ selector: '[a]' }] - }], null, { foo: [] }); -A.ngDirectiveDef = ɵngcc0.ɵdefineDirective({ type: A, selectors: [["", "a", ""]], factory: function A_Factory(t) { return new (t || A)(); } });`); + }], null, { foo: [] });`); }); it('should call removeDecorators with the source code, a map of class decorators that have been analyzed', diff --git a/packages/compiler-cli/src/ngtsc/imports/src/core.ts b/packages/compiler-cli/src/ngtsc/imports/src/core.ts index 803fae9121..80b24430a7 100644 --- a/packages/compiler-cli/src/ngtsc/imports/src/core.ts +++ b/packages/compiler-cli/src/ngtsc/imports/src/core.ts @@ -51,6 +51,7 @@ const CORE_SUPPORTED_SYMBOLS = new Map([ ['defineInjectable', 'defineInjectable'], ['defineInjector', 'defineInjector'], ['ɵdefineNgModule', 'defineNgModule'], + ['ɵsetNgModuleScope', 'setNgModuleScope'], ['inject', 'inject'], ['ɵsetClassMetadata', 'setClassMetadata'], ['ɵInjectableDef', 'InjectableDef'], diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index e05903abf2..ca4d2375d3 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -442,10 +442,9 @@ describe('ngtsc behavioral tests', () => { env.driveMain(); const jsContents = env.getContents('test.js'); + expect(jsContents).toContain('i0.ɵdefineNgModule({ type: TestModule, bootstrap: [TestCmp] });'); expect(jsContents) - .toContain( - 'i0.ɵdefineNgModule({ type: TestModule, bootstrap: [TestCmp], ' + - 'declarations: [TestCmp] })'); + .toContain('/*@__PURE__*/ i0.ɵsetNgModuleScope(TestModule, { declarations: [TestCmp] });'); const dtsContents = env.getContents('test.d.ts'); expect(dtsContents) @@ -457,6 +456,22 @@ describe('ngtsc behavioral tests', () => { expect(dtsContents).not.toContain('__decorate'); }); + it('should not emit a setNgModuleScope call when no scope metadata is present', () => { + env.tsconfig(); + env.write('test.ts', ` + import {NgModule} from '@angular/core'; + + @NgModule({}) + export class TestModule {} + `); + + env.driveMain(); + + const jsContents = env.getContents('test.js'); + expect(jsContents).toContain('i0.ɵdefineNgModule({ type: TestModule });'); + expect(jsContents).not.toContain('ɵsetNgModuleScope(TestModule,'); + }); + it('should compile NgModules with services without errors', () => { env.tsconfig(); env.write('test.ts', ` @@ -484,7 +499,7 @@ describe('ngtsc behavioral tests', () => { env.driveMain(); const jsContents = env.getContents('test.js'); - expect(jsContents).toContain('i0.ɵdefineNgModule({ type: TestModule,'); + expect(jsContents).toContain('i0.ɵdefineNgModule({ type: TestModule });'); expect(jsContents) .toContain( `TestModule.ngInjectorDef = i0.defineInjector({ factory: ` + @@ -525,7 +540,7 @@ describe('ngtsc behavioral tests', () => { env.driveMain(); const jsContents = env.getContents('test.js'); - expect(jsContents).toContain('i0.ɵdefineNgModule({ type: TestModule,'); + expect(jsContents).toContain('i0.ɵdefineNgModule({ type: TestModule });'); expect(jsContents) .toContain( `TestModule.ngInjectorDef = i0.defineInjector({ factory: ` + @@ -570,7 +585,7 @@ describe('ngtsc behavioral tests', () => { env.driveMain(); const jsContents = env.getContents('test.js'); - expect(jsContents).toContain('i0.ɵdefineNgModule({ type: TestModule,'); + expect(jsContents).toContain('i0.ɵdefineNgModule({ type: TestModule });'); expect(jsContents) .toContain( `TestModule.ngInjectorDef = i0.defineInjector({ factory: ` + diff --git a/packages/compiler-cli/test/ngtsc/scope_spec.ts b/packages/compiler-cli/test/ngtsc/scope_spec.ts index 07588c4484..6fed20ca76 100644 --- a/packages/compiler-cli/test/ngtsc/scope_spec.ts +++ b/packages/compiler-cli/test/ngtsc/scope_spec.ts @@ -21,6 +21,32 @@ describe('ngtsc module scopes', () => { describe('diagnostics', () => { describe('imports', () => { + it('should emit imports in a pure function call', () => { + env.tsconfig(); + env.write('test.ts', ` + import {NgModule} from '@angular/core'; + + @NgModule({}) + export class OtherModule {} + + @NgModule({imports: [OtherModule]}) + export class TestModule {} + `); + + env.driveMain(); + + const jsContents = env.getContents('test.js'); + expect(jsContents).toContain('i0.ɵdefineNgModule({ type: TestModule });'); + expect(jsContents) + .toContain( + '/*@__PURE__*/ i0.ɵsetNgModuleScope(TestModule, { imports: [OtherModule] });'); + + const dtsContents = env.getContents('test.d.ts'); + expect(dtsContents) + .toContain( + 'static ngModuleDef: i0.ɵNgModuleDefWithMeta'); + }); + it('should produce an error when an invalid class is imported', () => { env.write('test.ts', ` import {NgModule} from '@angular/core'; @@ -57,6 +83,32 @@ describe('ngtsc module scopes', () => { }); describe('exports', () => { + it('should emit exports in a pure function call', () => { + env.tsconfig(); + env.write('test.ts', ` + import {NgModule} from '@angular/core'; + + @NgModule({}) + export class OtherModule {} + + @NgModule({exports: [OtherModule]}) + export class TestModule {} + `); + + env.driveMain(); + + const jsContents = env.getContents('test.js'); + expect(jsContents).toContain('i0.ɵdefineNgModule({ type: TestModule });'); + expect(jsContents) + .toContain( + '/*@__PURE__*/ i0.ɵsetNgModuleScope(TestModule, { exports: [OtherModule] });'); + + const dtsContents = env.getContents('test.d.ts'); + expect(dtsContents) + .toContain( + 'static ngModuleDef: i0.ɵNgModuleDefWithMeta'); + }); + it('should produce an error when a non-NgModule class is exported', () => { env.write('test.ts', ` import {NgModule} from '@angular/core'; diff --git a/packages/compiler/src/render3/r3_identifiers.ts b/packages/compiler/src/render3/r3_identifiers.ts index 5de7100fe4..7adb73011f 100644 --- a/packages/compiler/src/render3/r3_identifiers.ts +++ b/packages/compiler/src/render3/r3_identifiers.ts @@ -195,6 +195,7 @@ export class Identifiers { }; static defineNgModule: o.ExternalReference = {name: 'ɵdefineNgModule', moduleName: CORE}; + static setNgModuleScope: o.ExternalReference = {name: 'ɵsetNgModuleScope', moduleName: CORE}; static PipeDefWithMeta: o.ExternalReference = {name: 'ɵPipeDefWithMeta', moduleName: CORE}; diff --git a/packages/compiler/src/render3/r3_module_compiler.ts b/packages/compiler/src/render3/r3_module_compiler.ts index 14a5bf0ba5..71fb27b39d 100644 --- a/packages/compiler/src/render3/r3_module_compiler.ts +++ b/packages/compiler/src/render3/r3_module_compiler.ts @@ -80,9 +80,11 @@ export function compileNgModule(meta: R3NgModuleMetadata): R3NgModuleDef { imports, exports, schemas, - containsForwardDecls + containsForwardDecls, + emitInline } = meta; + const additionalStatements: o.Statement[] = []; const definitionMap = { type: moduleType } as{ @@ -99,16 +101,29 @@ export function compileNgModule(meta: R3NgModuleMetadata): R3NgModuleDef { definitionMap.bootstrap = refsToArray(bootstrap, containsForwardDecls); } - if (declarations.length) { - definitionMap.declarations = refsToArray(declarations, containsForwardDecls); + // If requested to emit scope information inline, pass the declarations, imports and exports to + // the `defineNgModule` call. The JIT compilation uses this. + if (emitInline) { + if (declarations.length) { + definitionMap.declarations = refsToArray(declarations, containsForwardDecls); + } + + if (imports.length) { + definitionMap.imports = refsToArray(imports, containsForwardDecls); + } + + if (exports.length) { + definitionMap.exports = refsToArray(exports, containsForwardDecls); + } } - if (imports.length) { - definitionMap.imports = refsToArray(imports, containsForwardDecls); - } - - if (exports.length) { - definitionMap.exports = refsToArray(exports, containsForwardDecls); + // If not emitting inline, the scope information is not passed into `defineNgModule` as it would + // prevent tree-shaking of the declarations, imports and exports references. + else { + const setNgModuleScopeCall = generateSetNgModuleScopeCall(meta); + if (setNgModuleScopeCall !== null) { + additionalStatements.push(setNgModuleScopeCall); + } } if (schemas && schemas.length) { @@ -121,10 +136,50 @@ export function compileNgModule(meta: R3NgModuleMetadata): R3NgModuleDef { tupleTypeOf(exports) ])); - const additionalStatements: o.Statement[] = []; + return {expression, type, additionalStatements}; } +/** + * Generates a function call to `setNgModuleScope` with all necessary information so that the + * transitive module scope can be computed during runtime in JIT mode. This call is marked pure + * such that the references to declarations, imports and exports may be elided causing these + * symbols to become tree-shakeable. + */ +function generateSetNgModuleScopeCall(meta: R3NgModuleMetadata): o.Statement|null { + const {type: moduleType, declarations, imports, exports, containsForwardDecls} = meta; + + const scopeMap = {} as{ + declarations: o.Expression, + imports: o.Expression, + exports: o.Expression, + }; + + if (declarations.length) { + scopeMap.declarations = refsToArray(declarations, containsForwardDecls); + } + + if (imports.length) { + scopeMap.imports = refsToArray(imports, containsForwardDecls); + } + + if (exports.length) { + scopeMap.exports = refsToArray(exports, containsForwardDecls); + } + + if (Object.keys(scopeMap).length === 0) { + return null; + } + + const fnCall = new o.InvokeFunctionExpr( + /* fn */ o.importExpr(R3.setNgModuleScope), + /* args */[moduleType, mapToMapExpression(scopeMap)], + /* type */ undefined, + /* sourceSpan */ undefined, + /* pure */ true); + return fnCall.toStmt(); +} + export interface R3InjectorDef { expression: o.Expression; type: o.Type; @@ -200,4 +255,4 @@ function tupleTypeOf(exp: R3Reference[]): o.Type { function refsToArray(refs: R3Reference[], shouldForwardDeclare: boolean): o.Expression { const values = o.literalArr(refs.map(ref => ref.value)); return shouldForwardDeclare ? o.fn([], [new o.ReturnStatement(values)]) : values; -} \ No newline at end of file +} diff --git a/packages/core/src/core_render3_private_export.ts b/packages/core/src/core_render3_private_export.ts index 6ca05d3c77..db9a85429e 100644 --- a/packages/core/src/core_render3_private_export.ts +++ b/packages/core/src/core_render3_private_export.ts @@ -26,6 +26,7 @@ export { getFactoryOf as ɵgetFactoryOf, getInheritedFactory as ɵgetInheritedFactory, setComponentScope as ɵsetComponentScope, + setNgModuleScope as ɵsetNgModuleScope, templateRefExtractor as ɵtemplateRefExtractor, ProvidersFeature as ɵProvidersFeature, InheritDefinitionFeature as ɵInheritDefinitionFeature, diff --git a/packages/core/src/render3/definition.ts b/packages/core/src/render3/definition.ts index a887acb1df..e46572dd5e 100644 --- a/packages/core/src/render3/definition.ts +++ b/packages/core/src/render3/definition.ts @@ -327,7 +327,28 @@ export function extractPipeDef(type: PipeType): PipeDef { return def !; } -export function defineNgModule(def: {type: T} & Partial>): never { +export function defineNgModule(def: { + /** Token representing the module. Used by DI. */ + type: T; + + /** List of components to bootstrap. */ + bootstrap?: Type[] | (() => Type[]); + + /** List of components, directives, and pipes declared by this module. */ + declarations?: Type[] | (() => Type[]); + + /** List of modules or `ModuleWithProviders` imported by this module. */ + imports?: Type[] | (() => Type[]); + + /** + * List of modules, `ModuleWithProviders`, components, directives, or pipes exported by this + * module. + */ + exports?: Type[] | (() => Type[]); + + /** The set of schemas that declare elements to be allowed in the NgModule. */ + schemas?: SchemaMetadata[] | null; +}): never { const res: NgModuleDef = { type: def.type, bootstrap: def.bootstrap || EMPTY_ARRAY, @@ -340,6 +361,33 @@ export function defineNgModule(def: {type: T} & Partial>): nev return res as never; } +/** + * Adds the module metadata that is necessary to compute the module's transitive scope to an + * existing module definition. + * + * Scope metadata of modules is not used in production builds, so calls to this function can be + * marked pure to tree-shake it from the bundle, allowing for all referenced declarations + * to become eligible for tree-shaking as well. + */ +export function setNgModuleScope(type: any, scope: { + /** List of components, directives, and pipes declared by this module. */ + declarations?: Type[] | (() => Type[]); + + /** List of modules or `ModuleWithProviders` imported by this module. */ + imports?: Type[] | (() => Type[]); + + /** + * List of modules, `ModuleWithProviders`, components, directives, or pipes exported by this + * module. + */ + exports?: Type[] | (() => Type[]); +}): void { + const ngModuleDef = getNgModuleDef(type, true); + ngModuleDef.declarations = scope.declarations || EMPTY_ARRAY; + ngModuleDef.imports = scope.imports || EMPTY_ARRAY; + ngModuleDef.exports = scope.exports || EMPTY_ARRAY; +} + /** * Inverts an inputs or outputs lookup such that the keys, which were the * minified keys, are part of the values, and the values are parsed so that diff --git a/packages/core/src/render3/index.ts b/packages/core/src/render3/index.ts index 6d3d38f68c..45d7a702d7 100644 --- a/packages/core/src/render3/index.ts +++ b/packages/core/src/render3/index.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ import {LifecycleHooksFeature, renderComponent, whenRendered} from './component'; -import {defineBase, defineComponent, defineDirective, defineNgModule, definePipe, setComponentScope} from './definition'; +import {defineBase, defineComponent, defineDirective, defineNgModule, definePipe, setComponentScope, setNgModuleScope} from './definition'; import {InheritDefinitionFeature} from './features/inherit_definition_feature'; import {NgOnChangesFeature} from './features/ng_onchanges_feature'; import {ProvidersFeature} from './features/providers_feature'; @@ -186,6 +186,7 @@ export { getRenderedText, renderComponent, setComponentScope, + setNgModuleScope, whenRendered, }; diff --git a/packages/core/src/render3/jit/environment.ts b/packages/core/src/render3/jit/environment.ts index 03737e3dd3..0e23bce15c 100644 --- a/packages/core/src/render3/jit/environment.ts +++ b/packages/core/src/render3/jit/environment.ts @@ -121,6 +121,7 @@ export const angularCoreEnv: {[name: string]: Function} = { 'ɵresolveDocument': r3.resolveDocument, 'ɵresolveBody': r3.resolveBody, 'ɵsetComponentScope': r3.setComponentScope, + 'ɵsetNgModuleScope': r3.setNgModuleScope, 'ɵsanitizeHtml': sanitization.sanitizeHtml, 'ɵsanitizeStyle': sanitization.sanitizeStyle,