From a50e2da64a4297d1bc994a267203c2097da2efaf Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Tue, 12 Apr 2022 13:28:28 +0000 Subject: [PATCH] fix(localize): ensure transitively loaded compiler code is tree-shakable (#45405) 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. PR Close #45405 --- packages/compiler/src/jit_compiler_facade.ts | 21 +++++++------------ packages/compiler/src/util.ts | 16 +++++++------- packages/core/src/util/global.ts | 16 +++++++------- .../animations/bundle.golden_symbols.json | 12 ----------- .../cyclic_import/bundle.golden_symbols.json | 12 ----------- .../forms_reactive/bundle.golden_symbols.json | 12 ----------- .../bundle.golden_symbols.json | 12 ----------- .../hello_world/bundle.golden_symbols.json | 12 ----------- .../router/bundle.golden_symbols.json | 12 ----------- .../bundling/todo/bundle.golden_symbols.json | 12 ----------- packages/localize/src/localize/src/global.ts | 19 +++++++++-------- 11 files changed, 35 insertions(+), 121 deletions(-) diff --git a/packages/compiler/src/jit_compiler_facade.ts b/packages/compiler/src/jit_compiler_facade.ts index b9943a5be84..76d2554fd03 100644 --- 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); @@ -288,11 +288,6 @@ type R3ComponentMetadataFacadeNoPropAndWhitespace = Pick< R3ComponentMetadataFacade, Exclude, '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 index cbfcf6e1be1..464f82a673d 100644 --- a/packages/compiler/src/util.ts +++ b/packages/compiler/src/util.ts @@ -134,14 +134,16 @@ 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; +// Check `global` first, because in Node tests both `global` and `window` may be defined and our +// `_global` variable should point to the NodeJS `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(size: number): T[]; diff --git a/packages/core/src/util/global.ts b/packages/core/src/util/global.ts index e4f01f5a93d..2ee32b03815 100644 --- 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 index b722ca21946..5f33c242577 100644 --- a/packages/core/test/bundling/animations/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations/bundle.golden_symbols.json @@ -500,18 +500,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 index 1eb2c7ed45b..bc3afb50623 100644 --- 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 index 0cbfe69c6f8..1f67c11f317 100644 --- a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json @@ -512,18 +512,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 index e37efc19381..612735b3ebd 100644 --- a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json @@ -506,18 +506,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 index 47b409fd12c..a7b2a9a0171 100644 --- 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 index 5d4ae8a30cb..7823b512b11 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -743,18 +743,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 index 20c7ad8be30..0d9e6a2a3b9 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -158,18 +158,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 index b1881128387..0c9c72e5f30 100644 --- 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))());