Skip to content

Commit a50e2da

Browse files
devversionatscott
devversion
authored and
atscott
committed
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
1 parent 4c56c45 commit a50e2da

File tree

11 files changed

+36
-122
lines changed

11 files changed

+36
-122
lines changed

‎packages/compiler/src/jit_compiler_facade.ts‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,10 @@ export class CompilerFacadeImpl implements CompilerFacade {
6868
internalType: new WrappedNodeExpr(facade.type),
6969
typeArgumentCount: facade.typeArgumentCount,
7070
providedIn: computeProvidedIn(facade.providedIn),
71-
useClass: convertToProviderExpression(facade, USE_CLASS),
72-
useFactory: wrapExpression(facade, USE_FACTORY),
73-
useValue: convertToProviderExpression(facade, USE_VALUE),
74-
useExisting: convertToProviderExpression(facade, USE_EXISTING),
71+
useClass: convertToProviderExpression(facade, 'useClass'),
72+
useFactory: wrapExpression(facade, 'useFactory'),
73+
useValue: convertToProviderExpression(facade, 'useValue'),
74+
useExisting: convertToProviderExpression(facade, 'useExisting'),
7575
deps: facade.deps?.map(convertR3DependencyMetadata),
7676
},
7777
/* resolveForwardRefs */ true);
@@ -89,10 +89,10 @@ export class CompilerFacadeImpl implements CompilerFacade {
8989
internalType: new WrappedNodeExpr(facade.type),
9090
typeArgumentCount: 0,
9191
providedIn: computeProvidedIn(facade.providedIn),
92-
useClass: convertToProviderExpression(facade, USE_CLASS),
93-
useFactory: wrapExpression(facade, USE_FACTORY),
94-
useValue: convertToProviderExpression(facade, USE_VALUE),
95-
useExisting: convertToProviderExpression(facade, USE_EXISTING),
92+
useClass: convertToProviderExpression(facade, 'useClass'),
93+
useFactory: wrapExpression(facade, 'useFactory'),
94+
useValue: convertToProviderExpression(facade, 'useValue'),
95+
useExisting: convertToProviderExpression(facade, 'useExisting'),
9696
deps: facade.deps?.map(convertR3DeclareDependencyMetadata),
9797
},
9898
/* resolveForwardRefs */ true);
@@ -288,11 +288,6 @@ type R3ComponentMetadataFacadeNoPropAndWhitespace = Pick<
288288
R3ComponentMetadataFacade,
289289
Exclude<Exclude<keyof R3ComponentMetadataFacade, 'preserveWhitespaces'>, 'propMetadata'>>;
290290

291-
const USE_CLASS = Object.keys({useClass: null})[0];
292-
const USE_FACTORY = Object.keys({useFactory: null})[0];
293-
const USE_VALUE = Object.keys({useValue: null})[0];
294-
const USE_EXISTING = Object.keys({useExisting: null})[0];
295-
296291
function convertToR3QueryMetadata(facade: R3QueryMetadataFacade): R3QueryMetadata {
297292
return {
298293
...facade,

‎packages/compiler/src/util.ts‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -134,14 +134,16 @@ declare var WorkerGlobalScope: any;
134134
// We don't want to include the whole node.d.ts this this compilation unit so we'll just fake
135135
// the global "global" var for now.
136136
declare var global: any;
137-
const __window = typeof window !== 'undefined' && window;
138-
const __self = typeof self !== 'undefined' && typeof WorkerGlobalScope !== 'undefined' &&
139-
self instanceof WorkerGlobalScope && self;
140-
const __global = typeof global !== 'undefined' && global;
141-
142-
// Check __global first, because in Node tests both __global and __window may be defined and _global
143-
// should be __global in that case.
144-
const _global: {[name: string]: any} = __global || __window || __self;
137+
138+
// Check `global` first, because in Node tests both `global` and `window` may be defined and our
139+
// `_global` variable should point to the NodeJS `global` in that case. Note: Typeof/Instanceof
140+
// checks are considered side-effects in Terser. We explicitly mark this as side-effect free:
141+
// https://github.com/terser/terser/issues/250.
142+
const _global: {[name: string]: any} = (/* @__PURE__ */ (
143+
() => (typeof global !== 'undefined' && global) || (typeof window !== 'undefined' && window) ||
144+
(typeof self !== 'undefined' && typeof WorkerGlobalScope !== 'undefined' &&
145+
self instanceof WorkerGlobalScope && self))());
146+
145147
export {_global as global};
146148

147149
export function newArray<T = any>(size: number): T[];

‎packages/core/src/util/global.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,16 @@ declare var WorkerGlobalScope: any /** TODO #9100 */;
1313
// the global "global" var for now.
1414
declare var global: any /** TODO #9100 */;
1515

16-
const __globalThis = typeof globalThis !== 'undefined' && globalThis;
17-
const __window = typeof window !== 'undefined' && window;
18-
const __self = typeof self !== 'undefined' && typeof WorkerGlobalScope !== 'undefined' &&
19-
self instanceof WorkerGlobalScope && self;
20-
const __global = typeof global !== 'undefined' && global;
21-
2216
// Always use __globalThis if available, which is the spec-defined global variable across all
2317
// environments, then fallback to __global first, because in Node tests both __global and
24-
// __window may be defined and _global should be __global in that case.
25-
const _global = __globalThis || __global || __window || __self;
18+
// __window may be defined and _global should be __global in that case. Note: Typeof/Instanceof
19+
// checks are considered side-effects in Terser. We explicitly mark this as side-effect free:
20+
// https://github.com/terser/terser/issues/250.
21+
const _global: any = (/* @__PURE__ */ (
22+
() => (typeof globalThis !== 'undefined' && globalThis) ||
23+
(typeof global !== 'undefined' && global) || (typeof window !== 'undefined' && window) ||
24+
(typeof self !== 'undefined' && typeof WorkerGlobalScope !== 'undefined' &&
25+
self instanceof WorkerGlobalScope && self))());
2626

2727
/**
2828
* Attention: whenever providing a new value, be sure to add an

‎packages/core/test/bundling/animations/bundle.golden_symbols.json‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -500,18 +500,6 @@
500500
{
501501
"name": "__forward_ref__"
502502
},
503-
{
504-
"name": "__global"
505-
},
506-
{
507-
"name": "__globalThis"
508-
},
509-
{
510-
"name": "__self"
511-
},
512-
{
513-
"name": "__window"
514-
},
515503
{
516504
"name": "_chromeNumKeyPadMap"
517505
},

‎packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -47,18 +47,6 @@
4747
{
4848
"name": "ViewEncapsulation"
4949
},
50-
{
51-
"name": "__global"
52-
},
53-
{
54-
"name": "__globalThis"
55-
},
56-
{
57-
"name": "__self"
58-
},
59-
{
60-
"name": "__window"
61-
},
6250
{
6351
"name": "_global"
6452
},

‎packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -512,18 +512,6 @@
512512
{
513513
"name": "__forward_ref__"
514514
},
515-
{
516-
"name": "__global"
517-
},
518-
{
519-
"name": "__globalThis"
520-
},
521-
{
522-
"name": "__self"
523-
},
524-
{
525-
"name": "__window"
526-
},
527515
{
528516
"name": "_chromeNumKeyPadMap"
529517
},

‎packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -506,18 +506,6 @@
506506
{
507507
"name": "__forward_ref__"
508508
},
509-
{
510-
"name": "__global"
511-
},
512-
{
513-
"name": "__globalThis"
514-
},
515-
{
516-
"name": "__self"
517-
},
518-
{
519-
"name": "__window"
520-
},
521509
{
522510
"name": "_chromeNumKeyPadMap"
523511
},

‎packages/core/test/bundling/hello_world/bundle.golden_symbols.json‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -44,18 +44,6 @@
4444
{
4545
"name": "ViewEncapsulation"
4646
},
47-
{
48-
"name": "__global"
49-
},
50-
{
51-
"name": "__globalThis"
52-
},
53-
{
54-
"name": "__self"
55-
},
56-
{
57-
"name": "__window"
58-
},
5947
{
6048
"name": "_global"
6149
},

‎packages/core/test/bundling/router/bundle.golden_symbols.json‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -743,18 +743,6 @@
743743
{
744744
"name": "__forward_ref__"
745745
},
746-
{
747-
"name": "__global"
748-
},
749-
{
750-
"name": "__globalThis"
751-
},
752-
{
753-
"name": "__self"
754-
},
755-
{
756-
"name": "__window"
757-
},
758746
{
759747
"name": "_chromeNumKeyPadMap"
760748
},

‎packages/core/test/bundling/todo/bundle.golden_symbols.json‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -158,18 +158,6 @@
158158
{
159159
"name": "__forward_ref__"
160160
},
161-
{
162-
"name": "__global"
163-
},
164-
{
165-
"name": "__globalThis"
166-
},
167-
{
168-
"name": "__self"
169-
},
170-
{
171-
"name": "__window"
172-
},
173161
{
174162
"name": "_global"
175163
},

0 commit comments

Comments
 (0)