Skip to content

Commit 887c350

Browse files
dgp1130alxhub
authored andcommitted
refactor(compiler): wrap large strings in function (angular#38253)
Large strings constants are now wrapped in a function which is called whenever used. This works around a unique limitation of Closure, where it will **always** inline string literals at **every** usage, regardless of how large the string literal is or how many times it is used.The workaround is to use a function rather than a string literal. Closure has differently inlining semantics for functions, where it will check the length of the function and the number of times it is used before choosing to inline it. By using a function, `ngtsc` makes Closure more conservative about inlining large strings, and avoids blowing up the bundle size.This optimization is only used if the constant is a large string. A wrapping function is not included for other use cases, since it would just increase the bundle size and add unnecessary runtime performance overhead. PR Close angular#38253
1 parent 103a95c commit 887c350

File tree

3 files changed

+100
-8
lines changed

3 files changed

+100
-8
lines changed

packages/compiler-cli/src/ngtsc/transform/src/transform.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ function transformIvySourceFile(
249249
importRewriter: ImportRewriter, file: ts.SourceFile, isCore: boolean,
250250
isClosureCompilerEnabled: boolean,
251251
defaultImportRecorder: DefaultImportRecorder): ts.SourceFile {
252-
const constantPool = new ConstantPool();
252+
const constantPool = new ConstantPool(isClosureCompilerEnabled);
253253
const importManager = new ImportManager(importRewriter);
254254

255255
// The transformation process consists of 2 steps:

packages/compiler-cli/test/ngtsc/ngtsc_spec.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6685,6 +6685,66 @@ export const Foo = Foo__PRE_R3__;
66856685
// remains a string in the `styles` array.
66866686
'"img[_ngcontent-%COMP%] { background: url(/b/some-very-very-long-path.png); }"]');
66876687
});
6688+
6689+
it('large strings are wrapped in a function for Closure', () => {
6690+
env.tsconfig({
6691+
annotateForClosureCompiler: true,
6692+
});
6693+
6694+
env.write('test.ts', `
6695+
import {Component} from '@angular/core';
6696+
6697+
@Component({
6698+
selector: 'comp-a',
6699+
template: 'Comp A',
6700+
styles: [
6701+
'div { background: url(/a.png); }',
6702+
'div { background: url(/some-very-very-long-path.png); }',
6703+
]
6704+
})
6705+
export class CompA {}
6706+
6707+
@Component({
6708+
selector: 'comp-b',
6709+
template: 'Comp B',
6710+
styles: [
6711+
'div { background: url(/b.png); }',
6712+
'div { background: url(/some-very-very-long-path.png); }',
6713+
]
6714+
})
6715+
export class CompB {}
6716+
`);
6717+
6718+
env.driveMain();
6719+
const jsContents = env.getContents('test.js');
6720+
6721+
// Verify that long strings are extracted to a separate var. This should be wrapped in a
6722+
// function to trick Closure not to inline the contents for very large strings.
6723+
// See: https://github.com/angular/angular/pull/38253.
6724+
expect(jsContents)
6725+
.toContain(
6726+
'_c0 = function () {' +
6727+
' return "div[_ngcontent-%COMP%] {' +
6728+
' background: url(/some-very-very-long-path.png);' +
6729+
' }";' +
6730+
' };');
6731+
6732+
expect(jsContents)
6733+
.toContain(
6734+
'styles: [' +
6735+
// Check styles for component A.
6736+
'"div[_ngcontent-%COMP%] { background: url(/a.png); }", ' +
6737+
// Large string should be called from function definition.
6738+
'_c0()]');
6739+
6740+
expect(jsContents)
6741+
.toContain(
6742+
'styles: [' +
6743+
// Check styles for component B.
6744+
'"div[_ngcontent-%COMP%] { background: url(/b.png); }", ' +
6745+
// Large string should be called from function definition.
6746+
'_c0()]');
6747+
});
66886748
});
66896749

66906750
describe('non-exported classes', () => {

packages/compiler/src/constant_pool.ts

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,10 @@ export class ConstantPool {
102102

103103
private nextNameIndex = 0;
104104

105+
constructor(private readonly isClosureCompilerEnabled: boolean = false) {}
106+
105107
getConstLiteral(literal: o.Expression, forceShared?: boolean): o.Expression {
106-
if ((literal instanceof o.LiteralExpr && !isLongStringExpr(literal)) ||
108+
if ((literal instanceof o.LiteralExpr && !isLongStringLiteral(literal)) ||
107109
literal instanceof FixupExpression) {
108110
// Do no put simple literals into the constant pool or try to produce a constant for a
109111
// reference to a constant.
@@ -121,9 +123,39 @@ export class ConstantPool {
121123
if ((!newValue && !fixup.shared) || (newValue && forceShared)) {
122124
// Replace the expression with a variable
123125
const name = this.freshName();
124-
this.statements.push(
125-
o.variable(name).set(literal).toDeclStmt(o.INFERRED_TYPE, [o.StmtModifier.Final]));
126-
fixup.fixup(o.variable(name));
126+
let definition: o.WriteVarExpr;
127+
let usage: o.Expression;
128+
if (this.isClosureCompilerEnabled && isLongStringLiteral(literal)) {
129+
// For string literals, Closure will **always** inline the string at
130+
// **all** usages, duplicating it each time. For large strings, this
131+
// unnecessarily bloats bundle size. To work around this restriction, we
132+
// wrap the string in a function, and call that function for each usage.
133+
// This tricks Closure into using inline logic for functions instead of
134+
// string literals. Function calls are only inlined if the body is small
135+
// enough to be worth it. By doing this, very large strings will be
136+
// shared across multiple usages, rather than duplicating the string at
137+
// each usage site.
138+
//
139+
// const myStr = function() { return "very very very long string"; };
140+
// const usage1 = myStr();
141+
// const usage2 = myStr();
142+
definition = o.variable(name).set(new o.FunctionExpr(
143+
[], // Params.
144+
[
145+
// Statements.
146+
new o.ReturnStatement(literal),
147+
],
148+
));
149+
usage = o.variable(name).callFn([]);
150+
} else {
151+
// Just declare and use the variable directly, without a function call
152+
// indirection. This saves a few bytes and avoids an unncessary call.
153+
definition = o.variable(name).set(literal);
154+
usage = o.variable(name);
155+
}
156+
157+
this.statements.push(definition.toDeclStmt(o.INFERRED_TYPE, [o.StmtModifier.Final]));
158+
fixup.fixup(usage);
127159
}
128160

129161
return fixup;
@@ -314,7 +346,7 @@ function isVariable(e: o.Expression): e is o.ReadVarExpr {
314346
return e instanceof o.ReadVarExpr;
315347
}
316348

317-
function isLongStringExpr(expr: o.LiteralExpr): boolean {
318-
return typeof expr.value === 'string' &&
349+
function isLongStringLiteral(expr: o.Expression): boolean {
350+
return expr instanceof o.LiteralExpr && typeof expr.value === 'string' &&
319351
expr.value.length >= POOL_INCLUSION_LENGTH_THRESHOLD_FOR_STRINGS;
320-
}
352+
}

0 commit comments

Comments
 (0)