From 4e0784c2f0d9e132e2c4ad370c7beb1a44be390c Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Wed, 20 Apr 2022 08:00:50 -0700 Subject: [PATCH] refactor(compiler-cli): optimize InjectorDef imports generation (#45701) Before standalone, everything that could appear in an NgModule's `imports` was relevant to DI, and needed to be emitted in the `imports` of the generated `InjectorDef` definition. With the introduction of standalone types, NgModule `imports` can now contain components, directives, and pipes which are standalone. Only standalone components need to be included in the `imports` of the generated injector definition - directives and pipes have no effect on DI. Having them present doesn't cause any errors in the runtime (they're filtered out by the injector itself) but it does prevent tree-shaking. With this commit, the generation of `InjectorDef` now filters the `imports` to exclude directives and pipes as much as possible. It's not _always_ possible because an expression in `imports` may pull in both a directive and a `ModuleWithProviders` reference, and we have no way of referencing just the MWP part of that expression. Therefore this is an optimization, not a rule of `InjectorDef` compilation. PR Close #45701 --- .../annotations/ng_module/src/handler.ts | 38 +++++++++++- .../standalone/GOLDEN_PARTIAL.js | 57 ++++++++++++++++- .../standalone/TEST_CASES.json | 14 +++++ .../standalone/module_optimization.js | 1 + .../standalone/module_optimization.ts | 19 ++++++ .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 62 +++++++++++++++++++ .../test/ngtsc/standalone_spec.ts | 55 ++++++++++++++++ 7 files changed, 244 insertions(+), 2 deletions(-) create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/standalone/module_optimization.js create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/standalone/module_optimization.ts diff --git a/packages/compiler-cli/src/ngtsc/annotations/ng_module/src/handler.ts b/packages/compiler-cli/src/ngtsc/annotations/ng_module/src/handler.ts index def5fad4465..09c06b43269 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/ng_module/src/handler.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/ng_module/src/handler.ts @@ -471,7 +471,43 @@ export class NgModuleDecoratorHandler implements // Add all top-level imports from the `imports` field to the injector imports. for (const topLevelImport of analysis.imports) { - data.injectorImports.push(new WrappedNodeExpr(topLevelImport.expression)); + if (topLevelImport.hasModuleWithProviders) { + // We have no choice but to emit expressions which contain MWPs, as we cannot filter on + // individual references. + data.injectorImports.push(new WrappedNodeExpr(topLevelImport.expression)); + continue; + } + + const refsToEmit: Reference[] = []; + for (const ref of topLevelImport.resolvedReferences) { + const dirMeta = this.metaReader.getDirectiveMetadata(ref); + if (dirMeta !== null && !dirMeta.isComponent) { + // Skip emit of directives in imports - directives can't carry providers. + continue; + } + + const pipeMeta = dirMeta === null ? this.metaReader.getPipeMetadata(ref) : null; + if (pipeMeta !== null) { + // Skip emit of pipes in imports - pipes can't carry providers. + continue; + } + + refsToEmit.push(ref); + } + + if (refsToEmit.length === topLevelImport.resolvedReferences.length) { + // All references within this top-level import should be emitted, so just use the user's + // expression. + data.injectorImports.push(new WrappedNodeExpr(topLevelImport.expression)); + } else { + // Some references have been filtered out. Emit references to individual classes. + const context = node.getSourceFile(); + for (const ref of refsToEmit) { + const emittedRef = this.refEmitter.emit(ref, context); + assertSuccessfulReferenceEmit(emittedRef, topLevelImport.expression, 'class'); + data.injectorImports.push(emittedRef.expression); + } + } } if (scope !== null && !scope.compilation.isPoisoned) { diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/standalone/GOLDEN_PARTIAL.js b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/standalone/GOLDEN_PARTIAL.js index 8c95a258239..85fca4114f0 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/standalone/GOLDEN_PARTIAL.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/standalone/GOLDEN_PARTIAL.js @@ -155,7 +155,7 @@ export class SomeModule { } SomeModule.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: SomeModule, deps: [], target: i0.ɵɵFactoryTarget.NgModule }); SomeModule.ɵmod = i0.ɵɵngDeclareNgModule({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: SomeModule, imports: [IndirectDir, IndirectPipe], exports: [NotStandaloneStuffModule, IndirectDir, IndirectPipe] }); -SomeModule.ɵinj = i0.ɵɵngDeclareInjector({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: SomeModule, imports: [IndirectDir, IndirectPipe, NotStandaloneStuffModule] }); +SomeModule.ɵinj = i0.ɵɵngDeclareInjector({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: SomeModule, imports: [NotStandaloneStuffModule] }); i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: SomeModule, decorators: [{ type: NgModule, args: [{ @@ -286,3 +286,58 @@ export declare class RecursiveComponent { static ɵcmp: i0.ɵɵComponentDeclaration; } +/**************************************************************************************************** + * PARTIAL FILE: module_optimization.js + ****************************************************************************************************/ +import { Component, Directive, NgModule } from '@angular/core'; +import * as i0 from "@angular/core"; +export class StandaloneCmp { +} +StandaloneCmp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: StandaloneCmp, deps: [], target: i0.ɵɵFactoryTarget.Component }); +StandaloneCmp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: StandaloneCmp, isStandalone: true, selector: "standalone-cmp", ngImport: i0, template: '', isInline: true }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: StandaloneCmp, decorators: [{ + type: Component, + args: [{ + standalone: true, + selector: 'standalone-cmp', + template: '', + }] + }] }); +export class StandaloneDir { +} +StandaloneDir.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: StandaloneDir, deps: [], target: i0.ɵɵFactoryTarget.Directive }); +StandaloneDir.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: StandaloneDir, isStandalone: true, ngImport: i0 }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: StandaloneDir, decorators: [{ + type: Directive, + args: [{ standalone: true }] + }] }); +export class Module { +} +Module.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: Module, deps: [], target: i0.ɵɵFactoryTarget.NgModule }); +Module.ɵmod = i0.ɵɵngDeclareNgModule({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: Module, imports: [StandaloneCmp, StandaloneDir] }); +Module.ɵinj = i0.ɵɵngDeclareInjector({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: Module, imports: [StandaloneCmp] }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: Module, decorators: [{ + type: NgModule, + args: [{ + imports: [StandaloneCmp, StandaloneDir], + }] + }] }); + +/**************************************************************************************************** + * PARTIAL FILE: module_optimization.d.ts + ****************************************************************************************************/ +import * as i0 from "@angular/core"; +export declare class StandaloneCmp { + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; +} +export declare class StandaloneDir { + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵdir: i0.ɵɵDirectiveDeclaration; +} +export declare class Module { + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵmod: i0.ɵɵNgModuleDeclaration; + static ɵinj: i0.ɵɵInjectorDeclaration; +} + diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/standalone/TEST_CASES.json b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/standalone/TEST_CASES.json index 57a5e49bfb6..d995b6c535d 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/standalone/TEST_CASES.json +++ b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/standalone/TEST_CASES.json @@ -70,6 +70,20 @@ ] } ] + }, + { + "description": "should optimize injector imports", + "inputFiles": [ + "module_optimization.ts" + ], + "expectations": [ + { + "failureMessage": "Injector imports not optimized", + "files": [ + "module_optimization.js" + ] + } + ] } ] } \ No newline at end of file diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/standalone/module_optimization.js b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/standalone/module_optimization.js new file mode 100644 index 00000000000..fefb73c343c --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/standalone/module_optimization.js @@ -0,0 +1 @@ +Module.ɵinj = /*@__PURE__*/ i0.ɵɵdefineInjector({ imports: [StandaloneCmp] }); \ No newline at end of file diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/standalone/module_optimization.ts b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/standalone/module_optimization.ts new file mode 100644 index 00000000000..da8d7ccfd81 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/standalone/module_optimization.ts @@ -0,0 +1,19 @@ +import {Component, Directive, NgModule} from '@angular/core'; + +@Component({ + standalone: true, + selector: 'standalone-cmp', + template: '', +}) +export class StandaloneCmp { +} + +@Directive({standalone: true}) +export class StandaloneDir { +} + +@NgModule({ + imports: [StandaloneCmp, StandaloneDir], +}) +export class Module { +} \ No newline at end of file diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index d830350a890..fa03435b8a5 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -7660,6 +7660,68 @@ function allTests(os: string) { const codes = diagnostics.map((diag) => diag.code); expect(codes).toEqual([ngErrorCode(ErrorCode.INVALID_BANANA_IN_BOX)]); }); + + describe('InjectorDef emit optimizations for standalone', () => { + it('should not filter components out of NgModule.imports', () => { + env.write('test.ts', ` + import {Component, NgModule} from '@angular/core'; + + @Component({ + standalone: true, + selector: 'standalone-cmp', + template: '', + }) + export class StandaloneCmp {} + + @NgModule({ + imports: [StandaloneCmp], + exports: [StandaloneCmp], + }) + export class Module {} + `); + env.driveMain(); + + const jsContents = env.getContents('test.js'); + expect(jsContents).toContain('i0.ɵɵdefineInjector({ imports: [StandaloneCmp] })'); + }); + + it('should filter directives & pipes out of NgModule.imports', () => { + env.write('test.ts', ` + import {Directive, ModuleWithProviders, NgModule, Pipe} from '@angular/core'; + @Directive({standalone: true}) + export class StandaloneDir {} + + @NgModule({}) + export class SubModule {}; + + declare function SubModuleWithProviders(): ModuleWithProviders; + + @Pipe({standalone: true, name: 'st'}) + export class StandalonePipe {} + + export const LOCAL_ARRAY = [StandaloneDir, StandalonePipe, SubModule]; + + export const ARRAY_WITH_MWP = [StandalonePipe, SubModuleWithProviders()]; + + @NgModule({ + imports: [ + StandaloneDir, + ...LOCAL_ARRAY, + SubModule, + SubModuleWithProviders(), + ARRAY_WITH_MWP, + ], + }) + export class Module {} + `); + env.driveMain(); + const jsContents = env.getContents('test.js'); + const expectedImports = 'SubModule, SubModule, SubModuleWithProviders(), ARRAY_WITH_MWP'; + expect(jsContents.replace(/\s/g, '')) + .toContain(`Module.ɵinj=/*@__PURE__*/i0.ɵɵdefineInjector({imports:[${ + expectedImports.replace(/\s/g, '')}]});`); + }); + }); }); function expectTokenAtPosition( diff --git a/packages/compiler-cli/test/ngtsc/standalone_spec.ts b/packages/compiler-cli/test/ngtsc/standalone_spec.ts index 667898c212b..f0e55634ae6 100644 --- a/packages/compiler-cli/test/ngtsc/standalone_spec.ts +++ b/packages/compiler-cli/test/ngtsc/standalone_spec.ts @@ -597,5 +597,60 @@ runInEachFileSystem(() => { expect(jsCode).toContain('dependencies: [StandalonePipe]'); }); }); + + describe('optimizations', () => { + it('does emit standalone components in injector imports', () => { + env.write('test.ts', ` + import {Component, NgModule} from '@angular/core'; + + @Component({ + standalone: true, + selector: 'standalone-cmp', + template: '', + }) + export class StandaloneCmp {} + + @NgModule({ + imports: [StandaloneCmp], + exports: [StandaloneCmp], + }) + export class Module {} + `); + env.driveMain(); + + const jsCode = env.getContents('test.js'); + expect(jsCode).toContain('i0.ɵɵdefineInjector({ imports: [StandaloneCmp] });'); + }); + + it('does not emit standalone directives or pipes in injector imports', () => { + env.write('test.ts', ` + import {Directive, NgModule, Pipe} from '@angular/core'; + + @Directive({ + standalone: true, + selector: '[dir]', + }) + export class StandaloneDir {} + + @Pipe({ + standalone: true, + name: 'standalone', + }) + export class StandalonePipe { + transform(value: any): any {} + } + + @NgModule({ + imports: [StandaloneDir, StandalonePipe], + exports: [StandaloneDir, StandalonePipe], + }) + export class Module {} + `); + env.driveMain(); + + const jsCode = env.getContents('test.js'); + expect(jsCode).toContain('i0.ɵɵdefineInjector({});'); + }); + }); }); });