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 e83ced30ad..115a9e7361 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts @@ -309,7 +309,7 @@ export class NgModuleDecoratorHandler implements if (this.factoryTracker !== null) { this.factoryTracker.track(node.getSourceFile(), { name: analysis.factorySymbolName, - hasId: !!analysis.id, + hasId: analysis.id !== null, }); } diff --git a/packages/compiler-cli/src/ngtsc/shims/src/factory_generator.ts b/packages/compiler-cli/src/ngtsc/shims/src/factory_generator.ts index f9dcf6de28..4ee78ec513 100644 --- a/packages/compiler-cli/src/ngtsc/shims/src/factory_generator.ts +++ b/packages/compiler-cli/src/ngtsc/shims/src/factory_generator.ts @@ -186,8 +186,24 @@ function transformFactorySourceFile( // Otherwise, check if this export is a factory for a known NgModule, and retain it if so. const match = STRIP_NG_FACTORY.exec(decl.name.text); - if (match !== null && moduleSymbols.has(match[1])) { - transformedStatements.push(stmt); + const module = match ? moduleSymbols.get(match[1]) : null; + if (module) { + // If the module can be tree shaken, then the factory should be wrapped in a + // `noSideEffects()` call which tells Closure to treat the expression as pure, allowing + // it to be removed if the result is not used. + // + // `NgModule`s with an `id` property will be lazy loaded. Google-internal lazy loading + // infra relies on a side effect from the `new NgModuleFactory()` call, which registers + // the module globally. Because of this, we **cannot** tree shake any module which has + // an `id` property. Doing so would cause lazy loaded modules to never be registered. + const moduleIsTreeShakable = !module.hasId; + const newStmt = !moduleIsTreeShakable ? + stmt : + updateInitializers( + stmt, + (init) => init ? wrapInNoSideEffects(init) : undefined, + ); + transformedStatements.push(newStmt); } } else { // Leave the statement alone, as it can't be understood. @@ -266,3 +282,62 @@ function getFileoverviewComment(sourceFile: ts.SourceFile): string|null { return commentText; } + +/** + * Wraps the given expression in a call to `ɵnoSideEffects()`, which tells + * Closure we don't care about the side effects of this expression and it should + * be treated as "pure". Closure is free to tree shake this expression if its + * result is not used. + * + * Example: Takes `1 + 2` and returns `i0.ɵnoSideEffects(() => 1 + 2)`. + */ +function wrapInNoSideEffects(expr: ts.Expression): ts.Expression { + const noSideEffects = ts.createPropertyAccess( + ts.createIdentifier('i0'), + 'ɵnoSideEffects', + ); + + return ts.createCall( + noSideEffects, + /* typeArguments */[], + /* arguments */ + [ + ts.createFunctionExpression( + /* modifiers */[], + /* asteriskToken */ undefined, + /* name */ undefined, + /* typeParameters */[], + /* parameters */[], + /* type */ undefined, + /* body */ ts.createBlock([ + ts.createReturn(expr), + ]), + ), + ], + ); +} + +/** + * Clones and updates the initializers for a given statement to use the new + * expression provided. Does not mutate the input statement. + */ +function updateInitializers( + stmt: ts.VariableStatement, + update: (initializer?: ts.Expression) => ts.Expression | undefined, + ): ts.VariableStatement { + return ts.updateVariableStatement( + stmt, + stmt.modifiers, + ts.updateVariableDeclarationList( + stmt.declarationList, + stmt.declarationList.declarations.map( + (decl) => ts.updateVariableDeclaration( + decl, + decl.name, + decl.type, + update(decl.initializer), + ), + ), + ), + ); +} diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index 886e364fbc..5ae96b6e5c 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -3538,7 +3538,9 @@ runInEachFileSystem(os => { expect(factoryContents).toContain(`import * as i0 from '@angular/core';`); expect(factoryContents).toContain(`import { NotAModule, TestModule } from './test';`); expect(factoryContents) - .toContain(`export var TestModuleNgFactory = new i0.\u0275NgModuleFactory(TestModule);`); + .toContain( + 'export var TestModuleNgFactory = i0.\u0275noSideEffects(function () { ' + + 'return new i0.\u0275NgModuleFactory(TestModule); });'); expect(factoryContents).not.toContain(`NotAModuleNgFactory`); expect(factoryContents).not.toContain('\u0275NonEmptyModule'); @@ -3677,11 +3679,32 @@ runInEachFileSystem(os => { 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); - `)); + expect(factoryContents) + .toBe( + 'import * as i0 from "./r3_symbols";\n' + + 'import { TestModule } from \'./test\';\n' + + 'export var TestModuleNgFactory = i0.\u0275noSideEffects(function () {' + + ' return new i0.NgModuleFactory(TestModule); });\n'); + }); + + it('should generate side effectful NgModuleFactory constructor when lazy loaded', () => { + env.tsconfig({'allowEmptyCodegenFiles': true}); + + env.write('test.ts', ` + import {NgModule} from '@angular/core'; + + @NgModule({ + id: 'test', // ID to use for lazy loading. + }) + export class TestModule {} + `); + + env.driveMain(); + + // Should **not** contain noSideEffects(), because the module is lazy loaded. + const factoryContents = env.getContents('test.ngfactory.js'); + expect(factoryContents) + .toContain('export var TestModuleNgFactory = new i0.ɵNgModuleFactory(TestModule);'); }); describe('file-level comments', () => {