From 2d372f48dbcbd85470288f7e00edf3eb8779cb0f Mon Sep 17 00:00:00 2001 From: JoostK Date: Sat, 30 Mar 2019 13:09:45 +0100 Subject: [PATCH] feat(ivy): exclude declarations from injector imports (#29598) Prior to this change, a module's imports and exports would be used verbatim as an injectors' imports. This is detrimental for tree-shaking, as a module's exports could reference declarations that would then prevent such declarations from being eligible for tree-shaking. Since an injector actually only needs NgModule references as its imports, we may safely filter out any declarations from the list of module exports. This makes them eligible for tree-shaking once again. PR Close #29598 --- integration/_payload-limits.json | 2 +- .../src/ngtsc/annotations/src/ng_module.ts | 29 +++++++-- .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 60 +++++++++++++++++++ .../compiler/src/compiler_facade_interface.ts | 4 +- packages/compiler/src/jit_compiler_facade.ts | 2 +- .../src/render3/r3_module_compiler.ts | 4 +- 6 files changed, 90 insertions(+), 11 deletions(-) diff --git a/integration/_payload-limits.json b/integration/_payload-limits.json index 190fa36ce2..b27d86daaa 100644 --- a/integration/_payload-limits.json +++ b/integration/_payload-limits.json @@ -21,7 +21,7 @@ "master": { "uncompressed": { "runtime": 1440, - "main": 212976, + "main": 155609, "polyfills": 43567 } } diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts index 3cec047fd8..3d2a3eb149 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts @@ -14,7 +14,7 @@ import {DefaultImportRecorder, Reference, ReferenceEmitter} from '../../imports' import {PartialEvaluator, ResolvedValue} from '../../partial_evaluator'; import {ClassDeclaration, Decorator, ReflectionHost, reflectObjectLiteral, typeNodeToValueExpr} from '../../reflection'; import {NgModuleRouteAnalyzer} from '../../routing'; -import {LocalModuleScopeRegistry} from '../../scope'; +import {LocalModuleScopeRegistry, ScopeData} from '../../scope'; import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence, ResolveResult} from '../../transform'; import {getSourceFile} from '../../util/src/typescript'; @@ -27,6 +27,7 @@ export interface NgModuleAnalysis { ngInjectorDef: R3InjectorMetadata; metadataStmt: Statement|null; declarations: Reference[]; + exports: Reference[]; } /** @@ -162,13 +163,13 @@ export class NgModuleDecoratorHandler implements DecoratorHandler[] = []; if (ngModule.has('imports')) { injectorImports.push(new WrappedNodeExpr(ngModule.get('imports') !)); } - if (ngModule.has('exports')) { - injectorImports.push(new WrappedNodeExpr(ngModule.get('exports') !)); - } if (this.routeAnalyzer !== null) { this.routeAnalyzer.add(node.getSourceFile(), name, rawImports, rawExports, rawProviders); @@ -180,7 +181,7 @@ export class NgModuleDecoratorHandler implements DecoratorHandler directive.ref.node === node) && + !compilation.pipes.some(pipe => pipe.ref.node === node); +} + function isDeclarationReference(ref: any): ref is Reference { return ref instanceof Reference && (ts.isClassDeclaration(ref.node) || ts.isFunctionDeclaration(ref.node) || diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index ca4d2375d3..9016f3616a 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -472,6 +472,66 @@ describe('ngtsc behavioral tests', () => { expect(jsContents).not.toContain('ɵsetNgModuleScope(TestModule,'); }); + it('should filter out directives and pipes from module exports in the injector def', () => { + env.tsconfig(); + env.write('test.ts', ` + import {NgModule} from '@angular/core'; + import {RouterComp, RouterModule} from '@angular/router'; + import {Dir, OtherDir, MyPipe, Comp} from './decls'; + + @NgModule({ + declarations: [OtherDir], + exports: [OtherDir], + }) + export class OtherModule {} + + const EXPORTS = [Dir, MyPipe, Comp, OtherModule, OtherDir, RouterModule, RouterComp]; + + @NgModule({ + declarations: [Dir, MyPipe, Comp], + imports: [OtherModule, RouterModule.forRoot()], + exports: [EXPORTS], + }) + export class TestModule {} + `); + env.write(`decls.ts`, ` + import {Component, Directive, Pipe} from '@angular/core'; + + @Directive({selector: '[dir]'}) + export class Dir {} + + @Directive({selector: '[other]'}) + export class OtherDir {} + + @Pipe({name:'pipe'}) + export class MyPipe {} + + @Component({selector: 'test', template: ''}) + export class Comp {} + `); + env.write('node_modules/@angular/router/index.d.ts', ` + import {ɵComponentDefWithMeta, ModuleWithProviders, ɵNgModuleDefWithMeta} from '@angular/core'; + + export declare class RouterComp { + static ngComponentDef: ɵComponentDefWithMeta + } + + declare class RouterModule { + static forRoot(): ModuleWithProviders; + static ngModuleDef: ɵNgModuleDefWithMeta; + } + `); + + env.driveMain(); + + const jsContents = env.getContents('test.js'); + expect(jsContents) + .toContain( + 'i0.defineInjector({ factory: function TestModule_Factory(t) ' + + '{ return new (t || TestModule)(); }, providers: [], ' + + 'imports: [[OtherModule, RouterModule.forRoot()],\n OtherModule,\n RouterModule] });'); + }); + it('should compile NgModules with services without errors', () => { env.tsconfig(); env.write('test.ts', ` diff --git a/packages/compiler/src/compiler_facade_interface.ts b/packages/compiler/src/compiler_facade_interface.ts index 2f4112afaf..a010cff02e 100644 --- a/packages/compiler/src/compiler_facade_interface.ts +++ b/packages/compiler/src/compiler_facade_interface.ts @@ -110,8 +110,8 @@ export interface R3InjectorMetadataFacade { name: string; type: any; deps: R3DependencyMetadataFacade[]|null; - providers: any; - imports: any; + providers: any[]; + imports: any[]; } export interface R3DirectiveMetadataFacade { diff --git a/packages/compiler/src/jit_compiler_facade.ts b/packages/compiler/src/jit_compiler_facade.ts index 783870af89..91eece387e 100644 --- a/packages/compiler/src/jit_compiler_facade.ts +++ b/packages/compiler/src/jit_compiler_facade.ts @@ -73,7 +73,7 @@ export class CompilerFacadeImpl implements CompilerFacade { type: new WrappedNodeExpr(facade.type), deps: convertR3DependencyMetadataArray(facade.deps), providers: new WrappedNodeExpr(facade.providers), - imports: new WrappedNodeExpr(facade.imports), + imports: facade.imports.map(i => new WrappedNodeExpr(i)), }; const res = compileInjector(meta); return this.jitExpression(res.expression, angularCoreEnv, sourceMapUrl, res.statements); diff --git a/packages/compiler/src/render3/r3_module_compiler.ts b/packages/compiler/src/render3/r3_module_compiler.ts index 71fb27b39d..51c78d2f97 100644 --- a/packages/compiler/src/render3/r3_module_compiler.ts +++ b/packages/compiler/src/render3/r3_module_compiler.ts @@ -191,7 +191,7 @@ export interface R3InjectorMetadata { type: o.Expression; deps: R3DependencyMetadata[]|null; providers: o.Expression; - imports: o.Expression; + imports: o.Expression[]; } export function compileInjector(meta: R3InjectorMetadata): R3InjectorDef { @@ -204,7 +204,7 @@ export function compileInjector(meta: R3InjectorMetadata): R3InjectorDef { const expression = o.importExpr(R3.defineInjector).callFn([mapToMapExpression({ factory: result.factory, providers: meta.providers, - imports: meta.imports, + imports: o.literalArr(meta.imports), })]); const type = new o.ExpressionType(o.importExpr(R3.InjectorDef, [new o.ExpressionType(meta.type)]));