fix(compiler-cli): mark eager `NgModuleFactory` construction as not side effectful (#38320)

Roll forward of #38147.

This allows Closure compiler to tree shake unused constructor calls to `NgModuleFactory`, which is otherwise considered
side-effectful. The Angular compiler generates factory objects which are exported but typically not used, as they are
only needed for compatibility with View Engine. This results in top-level constructor calls, such as:

```typescript
export const FooNgFactory = new NgModuleFactory(Foo);
```

`NgModuleFactory` has a side-effecting constructor, so this statement cannot be tree shaken, even if `FooNgFactory` is
never imported. The `NgModuleFactory` continues to reference its associated `NgModule` and prevents the module and all
its unused dependencies from being tree shaken, making Closure builds significantly larger than necessary.

The fix here is to wrap `NgModuleFactory` constructor with `noSideEffects(() => /* ... */)`, which tricks the Closure
compiler into assuming that the invoked function has no side effects. This allows it to tree-shake unused
`NgModuleFactory()` constructors when they aren't imported. Since the factory can be removed, the module can also be
removed (if nothing else references it), thus tree shaking unused dependencies as expected.

The one notable edge case is for lazy loaded modules. Internally, lazy loading is done as a side effect when the lazy
script is evaluated. For Angular, this side effect is registering the `NgModule`. In Ivy this is done by the
`NgModuleFactory` constructor, so lazy loaded modules **cannot** have their top-level `NgModuleFactory` constructor
call tree shaken. We handle this case by looking for the `id` field on `@NgModule` annotations. All lazy loaded modules
include an `id`. When this `id` is found, the `NgModuleFactory` is generated **without** with `noSideEffects()` call,
so Closure will not tree shake it and the module will lazy-load correctly.

PR Close #38320

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,
       });
     }
 

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])) {
+        const module = match ? moduleSymbols.get(match[1]) : null;
-          transformedStatements.push(stmt);
+        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),
+                  ),
+              ),
+          ),
+  );
+}

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(`
+        expect(factoryContents)
-        import * as i0 from "./r3_symbols";
+            .toBe(
-        import { TestModule } from './test';
+                'import * as i0 from "./r3_symbols";\n' +
-        export var TestModuleNgFactory = new i0.NgModuleFactory(TestModule);
+                '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', () => {