fix(localize): ensure transitively loaded compiler code is tree-shakable

The localize primary entry-point (used at runtime in application code) indirectly loads from the compiler package for computing message ids. The compiler package has a couple of constants which cannot be DCE-ded/ tree-shaken due to side-effect reliance that is detected by Terser. We fix these constants to be three-shakable. Note that another issue technically would be that the compiler package has a side-effect call for `publishFacade` (for JIT), but that invocation is marked as pure by the Angular CLI babel optimization pipeline. So this results is no unused code currently but is risky and should be addressed in the future.
angular · Apr 12, 2022 · d0e31dd · d0e31dd
1 parent 97d2c31
commit d0e31dd
Show file tree

Hide file tree

Showing 11 changed files with 33 additions and 120 deletions.
diff --git a/packages/compiler/src/jit_compiler_facade.ts b/packages/compiler/src/jit_compiler_facade.ts
@@ -68,10 +68,10 @@ export class CompilerFacadeImpl implements CompilerFacade {
           internalType: new WrappedNodeExpr(facade.type),
           typeArgumentCount: facade.typeArgumentCount,
           providedIn: computeProvidedIn(facade.providedIn),
-          useClass: convertToProviderExpression(facade, USE_CLASS),
-          useFactory: wrapExpression(facade, USE_FACTORY),
-          useValue: convertToProviderExpression(facade, USE_VALUE),
-          useExisting: convertToProviderExpression(facade, USE_EXISTING),
+          useClass: convertToProviderExpression(facade, 'useClass'),
+          useFactory: wrapExpression(facade, 'useFactory'),
+          useValue: convertToProviderExpression(facade, 'useValue'),
+          useExisting: convertToProviderExpression(facade, 'useExisting'),
           deps: facade.deps?.map(convertR3DependencyMetadata),
         },
         /* resolveForwardRefs */ true);
@@ -89,10 +89,10 @@ export class CompilerFacadeImpl implements CompilerFacade {
           internalType: new WrappedNodeExpr(facade.type),
           typeArgumentCount: 0,
           providedIn: computeProvidedIn(facade.providedIn),
-          useClass: convertToProviderExpression(facade, USE_CLASS),
-          useFactory: wrapExpression(facade, USE_FACTORY),
-          useValue: convertToProviderExpression(facade, USE_VALUE),
-          useExisting: convertToProviderExpression(facade, USE_EXISTING),
+          useClass: convertToProviderExpression(facade, 'useClass'),
+          useFactory: wrapExpression(facade, 'useFactory'),
+          useValue: convertToProviderExpression(facade, 'useValue'),
+          useExisting: convertToProviderExpression(facade, 'useExisting'),
           deps: facade.deps?.map(convertR3DeclareDependencyMetadata),
         },
         /* resolveForwardRefs */ true);
@@ -286,11 +286,6 @@ type R3ComponentMetadataFacadeNoPropAndWhitespace = Pick<
     R3ComponentMetadataFacade,
     Exclude<Exclude<keyof R3ComponentMetadataFacade, 'preserveWhitespaces'>, 'propMetadata'>>;
 
-const USE_CLASS = Object.keys({useClass: null})[0];
-const USE_FACTORY = Object.keys({useFactory: null})[0];
-const USE_VALUE = Object.keys({useValue: null})[0];
-const USE_EXISTING = Object.keys({useExisting: null})[0];
-
 function convertToR3QueryMetadata(facade: R3QueryMetadataFacade): R3QueryMetadata {
   return {
     ...facade,

diff --git a/packages/compiler/src/util.ts b/packages/compiler/src/util.ts
@@ -134,14 +134,15 @@ declare var WorkerGlobalScope: any;
 // We don't want to include the whole node.d.ts this this compilation unit so we'll just fake
 // the global "global" var for now.
 declare var global: any;
-const __window = typeof window !== 'undefined' && window;
-const __self = typeof self !== 'undefined' && typeof WorkerGlobalScope !== 'undefined' &&
-    self instanceof WorkerGlobalScope && self;
-const __global = typeof global !== 'undefined' && global;
 
 // Check __global first, because in Node tests both __global and __window may be defined and _global
-// should be __global in that case.
-const _global: {[name: string]: any} = __global || __window || __self;
+// should be __global in that case. Note: Typeof/Instanceof checks are considered side-effects in
+// Terser. We explicitly mark this as side-effect free: https://github.com/terser/terser/issues/250.
+const _global: {[name: string]: any} = (/* @__PURE__ */ (
+    () => (typeof global !== 'undefined' && global) || (typeof window !== 'undefined' && window) ||
+        (typeof self !== 'undefined' && typeof WorkerGlobalScope !== 'undefined' &&
+         self instanceof WorkerGlobalScope && self))());
+
 export {_global as global};
 
 export function newArray<T = any>(size: number): T[];

diff --git a/packages/core/src/util/global.ts b/packages/core/src/util/global.ts
@@ -13,16 +13,16 @@ declare var WorkerGlobalScope: any /** TODO #9100 */;
 // the global "global" var for now.
 declare var global: any /** TODO #9100 */;
 
-const __globalThis = typeof globalThis !== 'undefined' && globalThis;
-const __window = typeof window !== 'undefined' && window;
-const __self = typeof self !== 'undefined' && typeof WorkerGlobalScope !== 'undefined' &&
-    self instanceof WorkerGlobalScope && self;
-const __global = typeof global !== 'undefined' && global;
-
 // Always use __globalThis if available, which is the spec-defined global variable across all
 // environments, then fallback to __global first, because in Node tests both __global and
-// __window may be defined and _global should be __global in that case.
-const _global = __globalThis || __global || __window || __self;
+// __window may be defined and _global should be __global in that case. Note: Typeof/Instanceof
+// checks are considered side-effects in Terser. We explicitly mark this as side-effect free:
+// https://github.com/terser/terser/issues/250.
+const _global: any = (/* @__PURE__ */ (
+    () => (typeof globalThis !== 'undefined' && globalThis) ||
+        (typeof global !== 'undefined' && global) || (typeof window !== 'undefined' && window) ||
+        (typeof self !== 'undefined' && typeof WorkerGlobalScope !== 'undefined' &&
+         self instanceof WorkerGlobalScope && self))());
 
 /**
  * Attention: whenever providing a new value, be sure to add an

diff --git a/packages/core/test/bundling/animations/bundle.golden_symbols.json b/packages/core/test/bundling/animations/bundle.golden_symbols.json
@@ -491,18 +491,6 @@
   {
     "name": "__forward_ref__"
   },
-  {
-    "name": "__global"
-  },
-  {
-    "name": "__globalThis"
-  },
-  {
-    "name": "__self"
-  },
-  {
-    "name": "__window"
-  },
   {
     "name": "_chromeNumKeyPadMap"
   },

diff --git a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json
@@ -47,18 +47,6 @@
   {
     "name": "ViewEncapsulation"
   },
-  {
-    "name": "__global"
-  },
-  {
-    "name": "__globalThis"
-  },
-  {
-    "name": "__self"
-  },
-  {
-    "name": "__window"
-  },
   {
     "name": "_global"
   },

diff --git a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json
@@ -503,18 +503,6 @@
   {
     "name": "__forward_ref__"
   },
-  {
-    "name": "__global"
-  },
-  {
-    "name": "__globalThis"
-  },
-  {
-    "name": "__self"
-  },
-  {
-    "name": "__window"
-  },
   {
     "name": "_chromeNumKeyPadMap"
   },

diff --git a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json
@@ -497,18 +497,6 @@
   {
     "name": "__forward_ref__"
   },
-  {
-    "name": "__global"
-  },
-  {
-    "name": "__globalThis"
-  },
-  {
-    "name": "__self"
-  },
-  {
-    "name": "__window"
-  },
   {
     "name": "_chromeNumKeyPadMap"
   },

diff --git a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json
@@ -44,18 +44,6 @@
   {
     "name": "ViewEncapsulation"
   },
-  {
-    "name": "__global"
-  },
-  {
-    "name": "__globalThis"
-  },
-  {
-    "name": "__self"
-  },
-  {
-    "name": "__window"
-  },
   {
     "name": "_global"
   },

diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json
@@ -731,18 +731,6 @@
   {
     "name": "__forward_ref__"
   },
-  {
-    "name": "__global"
-  },
-  {
-    "name": "__globalThis"
-  },
-  {
-    "name": "__self"
-  },
-  {
-    "name": "__window"
-  },
   {
     "name": "_chromeNumKeyPadMap"
   },

diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json
@@ -155,18 +155,6 @@
   {
     "name": "__forward_ref__"
   },
-  {
-    "name": "__global"
-  },
-  {
-    "name": "__globalThis"
-  },
-  {
-    "name": "__self"
-  },
-  {
-    "name": "__window"
-  },
   {
     "name": "_global"
   },

diff --git a/packages/localize/src/localize/src/global.ts b/packages/localize/src/localize/src/global.ts
@@ -24,12 +24,13 @@ declare global {
   var WorkerGlobalScope: {prototype: WorkerGlobalScope; new (): WorkerGlobalScope;};
 }
 
-const __globalThis = typeof globalThis !== 'undefined' && globalThis;
-const __window = typeof window !== 'undefined' && window;
-const __self = typeof self !== 'undefined' && typeof WorkerGlobalScope !== 'undefined' &&
-    self instanceof WorkerGlobalScope && self;
-const __global = typeof global !== 'undefined' && global;
-// Always use __globalThis if available; this is the spec-defined global variable across all
-// environments.
-// Then fallback to __global first; in Node tests both __global and __window may be defined.
-export const _global: any = __globalThis || __global || __window || __self;
+// Always use __globalThis if available, which is the spec-defined global variable across all
+// environments, then fallback to __global first, because in Node tests both __global and
+// __window may be defined and _global should be __global in that case. Note: Typeof/Instanceof
+// checks are considered side-effects in Terser. We explicitly mark this as side-effect free:
+// https://github.com/terser/terser/issues/250.
+export const _global: any = (/* @__PURE__ */ (
+    () => (typeof globalThis !== 'undefined' && globalThis) ||
+        (typeof global !== 'undefined' && global) || (typeof window !== 'undefined' && window) ||
+        (typeof self !== 'undefined' && typeof WorkerGlobalScope !== 'undefined' &&
+         self instanceof WorkerGlobalScope && self))());