From 3a525d196b4d7bd40f7bd1bd38de8a160e0247e5 Mon Sep 17 00:00:00 2001
From: Alex Rickabaugh <alx+alxhub@alxandria.net>
Date: Thu, 30 Jul 2020 11:22:49 -0700
Subject: [PATCH] Revert "fix(compiler): mark `NgModuleFactory` construction as
 not side effectful (#38147)" (#38303)

This reverts commit 7f8c2225f2cba3dfcf3ec23e0fe08b7d2e3497e8.

This commit caused test failures internally, which were traced back to the
optimizer removing NgModuleFactory constructor calls when those calls caused
side-effectful registration of NgModules by their ids.

PR Close #38303
---
 .../compiler-cli/src/ngtsc/imports/src/core.ts  |  1 -
 .../src/ngtsc/shims/src/factory_generator.ts    |  3 +--
 .../compiler-cli/test/ngtsc/fake_core/index.ts  |  2 --
 packages/compiler-cli/test/ngtsc/ngtsc_spec.ts  | 17 ++++++-----------
 .../core/src/core_render3_private_export.ts     |  1 -
 packages/core/src/r3_symbols.ts                 |  1 -
 6 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/packages/compiler-cli/src/ngtsc/imports/src/core.ts b/packages/compiler-cli/src/ngtsc/imports/src/core.ts
index eae858337b..122b399829 100644
--- a/packages/compiler-cli/src/ngtsc/imports/src/core.ts
+++ b/packages/compiler-cli/src/ngtsc/imports/src/core.ts
@@ -65,7 +65,6 @@ const CORE_SUPPORTED_SYMBOLS = new Map<string, string>([
   ['ɵɵInjectorDef', 'ɵɵInjectorDef'],
   ['ɵɵNgModuleDefWithMeta', 'ɵɵNgModuleDefWithMeta'],
   ['ɵNgModuleFactory', 'NgModuleFactory'],
-  ['ɵnoSideEffects', 'ɵnoSideEffects'],
 ]);
 
 const CORE_MODULE = '@angular/core';
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 08f633f056..8e54e282a5 100644
--- a/packages/compiler-cli/src/ngtsc/shims/src/factory_generator.ts
+++ b/packages/compiler-cli/src/ngtsc/shims/src/factory_generator.ts
@@ -63,8 +63,7 @@ export class FactoryGenerator implements PerFileShimGenerator, FactoryTracker {
       // because it won't miss any that do.
       const varLines = symbolNames.map(
           name => `export const ${
-              name}NgFactory: i0.ɵNgModuleFactory<any> = i0.ɵnoSideEffects(() => new i0.ɵNgModuleFactory(${
-              name}));`);
+              name}NgFactory: i0.ɵNgModuleFactory<any> = new i0.ɵNgModuleFactory(${name});`);
       sourceText += [
         // This might be incorrect if the current package being compiled is Angular core, but it's
         // okay to leave in at type checking time. TypeScript can handle this reference via its path
diff --git a/packages/compiler-cli/test/ngtsc/fake_core/index.ts b/packages/compiler-cli/test/ngtsc/fake_core/index.ts
index bcbb9b0261..7aad1050cf 100644
--- a/packages/compiler-cli/test/ngtsc/fake_core/index.ts
+++ b/packages/compiler-cli/test/ngtsc/fake_core/index.ts
@@ -103,5 +103,3 @@ export interface QueryList<T>/* implements Iterable<T> */ {
 export type NgIterable<T> = Array<T>|Iterable<T>;
 
 export class NgZone {}
-
-export declare function ɵnoSideEffects<T>(fn: () => T): T;
diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
index 7af46bfd7c..6e6848a80d 100644
--- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
+++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
@@ -3538,9 +3538,7 @@ 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 = ' +
-              'i0.ɵnoSideEffects(function () { return new i0.\u0275NgModuleFactory(TestModule); });');
+          .toContain(`export var TestModuleNgFactory = new i0.\u0275NgModuleFactory(TestModule);`);
       expect(factoryContents).not.toContain(`NotAModuleNgFactory`);
       expect(factoryContents).not.toContain('\u0275NonEmptyModule');
 
@@ -3679,14 +3677,11 @@ runInEachFileSystem(os => {
         env.driveMain();
 
         const factoryContents = env.getContents('test.ngfactory.js');
-        expect(normalize(factoryContents))
-            .toBe(
-                'import * as i0 from "./r3_symbols"; ' +
-                'import { TestModule } from \'./test\'; ' +
-                'export var TestModuleNgFactory = ' +
-                'i0.\u0275noSideEffects(function () { ' +
-                'return new i0.NgModuleFactory(TestModule); ' +
-                '});');
+        expect(normalize(factoryContents)).toBe(normalize(`
+        import * as i0 from "./r3_symbols";
+        import { TestModule } from './test';
+        export var TestModuleNgFactory = new i0.NgModuleFactory(TestModule);
+      `));
       });
 
       describe('file-level comments', () => {
diff --git a/packages/core/src/core_render3_private_export.ts b/packages/core/src/core_render3_private_export.ts
index 01f844c351..d20a6449ac 100644
--- a/packages/core/src/core_render3_private_export.ts
+++ b/packages/core/src/core_render3_private_export.ts
@@ -292,6 +292,5 @@ export {
   ɵɵsanitizeUrl,
   ɵɵsanitizeUrlOrResourceUrl,
 } from './sanitization/sanitization';
-export {noSideEffects as ɵnoSideEffects} from './util/closure';
 
 // clang-format on
diff --git a/packages/core/src/r3_symbols.ts b/packages/core/src/r3_symbols.ts
index 1ee3762e83..391bb8bf56 100644
--- a/packages/core/src/r3_symbols.ts
+++ b/packages/core/src/r3_symbols.ts
@@ -28,7 +28,6 @@ export {ɵɵdefineNgModule} from './render3/definition';
 export {ɵɵFactoryDef} from './render3/interfaces/definition';
 export {setClassMetadata} from './render3/metadata';
 export {NgModuleFactory} from './render3/ng_module_ref';
-export {noSideEffects as ɵnoSideEffects} from './util/closure';