Skip to content

Commit 6c126d5

Browse files
dgp1130devversion
dgp1130
authored and
devversion
committed
refactor(compiler): trim insignificant whitespace in expressions when preserveSignificantWhitespace === false. (#58176)
This parses and reserializes expressions to normalize their whitespace formatting and make them more durable to insignificant changes in whitespace which might otherwise alter message IDs despite no translator-meaningful change being made. PR Close #58176
1 parent 474163c commit 6c126d5

File tree

5 files changed

+114
-23
lines changed

5 files changed

+114
-23
lines changed

‎packages/compiler/src/i18n/extractor_merger.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,8 @@ class _Visitor implements html.Visitor {
353353
// When dropping significant whitespace we need to retain whitespace tokens or
354354
// else we won't be able to reuse source spans because empty tokens would be
355355
// removed and cause a mismatch.
356-
!this._preserveSignificantWhitespace /* retainEmptyTokens */,
356+
/* retainEmptyTokens */ !this._preserveSignificantWhitespace,
357+
/* preserveExpressionWhitespace */ this._preserveSignificantWhitespace,
357358
);
358359
}
359360

‎packages/compiler/src/i18n/i18n_parser.ts‎

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,17 @@
88

99
import {Lexer as ExpressionLexer} from '../expression_parser/lexer';
1010
import {Parser as ExpressionParser} from '../expression_parser/parser';
11+
import {serialize as serializeExpression} from '../expression_parser/serializer';
1112
import * as html from '../ml_parser/ast';
1213
import {InterpolationConfig} from '../ml_parser/defaults';
1314
import {getHtmlTagDefinition} from '../ml_parser/html_tags';
14-
import {InterpolatedAttributeToken, InterpolatedTextToken, TokenType} from '../ml_parser/tokens';
15+
import {
16+
AttributeValueInterpolationToken,
17+
InterpolatedAttributeToken,
18+
InterpolatedTextToken,
19+
InterpolationToken,
20+
TokenType,
21+
} from '../ml_parser/tokens';
1522
import {ParseSourceSpan} from '../parse_util';
1623

1724
import * as i18n from './i18n_ast';
@@ -38,12 +45,14 @@ export function createI18nMessageFactory(
3845
interpolationConfig: InterpolationConfig,
3946
containerBlocks: Set<string>,
4047
retainEmptyTokens: boolean,
48+
preserveExpressionWhitespace: boolean,
4149
): I18nMessageFactory {
4250
const visitor = new _I18nVisitor(
4351
_expParser,
4452
interpolationConfig,
4553
containerBlocks,
4654
retainEmptyTokens,
55+
preserveExpressionWhitespace,
4756
);
4857
return (nodes, meaning, description, customId, visitNodeFn) =>
4958
visitor.toI18nMessage(nodes, meaning, description, customId, visitNodeFn);
@@ -68,6 +77,7 @@ class _I18nVisitor implements html.Visitor {
6877
private _interpolationConfig: InterpolationConfig,
6978
private _containerBlocks: Set<string>,
7079
private readonly _retainEmptyTokens: boolean,
80+
private readonly _preserveExpressionWhitespace: boolean,
7181
) {}
7282

7383
public toI18nMessage(
@@ -274,14 +284,24 @@ class _I18nVisitor implements html.Visitor {
274284
case TokenType.INTERPOLATION:
275285
case TokenType.ATTR_VALUE_INTERPOLATION:
276286
hasInterpolation = true;
277-
const expression = token.parts[1];
287+
const [startMarker, expression, endMarker] = token.parts;
278288
const baseName = extractPlaceholderName(expression) || 'INTERPOLATION';
279289
const phName = context.placeholderRegistry.getPlaceholderName(baseName, expression);
280-
context.placeholderToContent[phName] = {
281-
text: token.parts.join(''),
282-
sourceSpan: token.sourceSpan,
283-
};
284-
nodes.push(new i18n.Placeholder(expression, phName, token.sourceSpan));
290+
291+
if (this._preserveExpressionWhitespace) {
292+
context.placeholderToContent[phName] = {
293+
text: token.parts.join(''),
294+
sourceSpan: token.sourceSpan,
295+
};
296+
nodes.push(new i18n.Placeholder(expression, phName, token.sourceSpan));
297+
} else {
298+
const normalized = this.normalizeExpression(token);
299+
context.placeholderToContent[phName] = {
300+
text: `${startMarker}${normalized}${endMarker}`,
301+
sourceSpan: token.sourceSpan,
302+
};
303+
nodes.push(new i18n.Placeholder(normalized, phName, token.sourceSpan));
304+
}
285305
break;
286306
default:
287307
// Try to merge text tokens with previous tokens. We do this even for all tokens
@@ -332,6 +352,19 @@ class _I18nVisitor implements html.Visitor {
332352
return nodes[0];
333353
}
334354
}
355+
356+
// Normalize expression whitespace by parsing and re-serializing it. This makes
357+
// message IDs more durable to insignificant whitespace changes.
358+
normalizeExpression(token: InterpolationToken | AttributeValueInterpolationToken): string {
359+
const expression = token.parts[1];
360+
const expr = this._expressionParser.parseBinding(
361+
expression,
362+
/* location */ token.sourceSpan.start.toString(),
363+
/* absoluteOffset */ token.sourceSpan.start.offset,
364+
this._interpolationConfig,
365+
);
366+
return serializeExpression(expr);
367+
}
335368
}
336369

337370
/**

‎packages/compiler/src/render3/view/i18n/meta.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ export class I18nMetaVisitor implements html.Visitor {
9090
this.interpolationConfig,
9191
this.containerBlocks,
9292
this.retainEmptyTokens,
93+
/* preserveExpressionWhitespace */ this.preserveSignificantWhitespace,
9394
);
9495
const message = createI18nMessage(nodes, meaning, description, customId, visitNodeFn);
9596
this._setMessageId(message, meta);

‎packages/compiler/test/i18n/i18n_ast_spec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ describe('Message', () => {
1616
DEFAULT_INTERPOLATION_CONFIG,
1717
DEFAULT_CONTAINER_BLOCKS,
1818
/* retainEmptyTokens */ false,
19+
/* preserveExpressionWhitespace */ true,
1920
);
2021
describe('messageText()', () => {
2122
it('should serialize simple text', () => {

‎packages/compiler/test/i18n/i18n_parser_spec.ts‎

Lines changed: 70 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,34 @@ describe('I18nParser', () => {
227227
[['{count, plural, =0 {[{sex, select, male {[m]}}]}}'], '', '', ''],
228228
]);
229229
});
230+
231+
it('should preserve whitespace when preserving significant whitespace', () => {
232+
const html = '<div i18n="m|d">{count, plural, =0 {{{ foo }}}}</div>';
233+
expect(
234+
_humanizeMessages(
235+
html,
236+
/* implicitTags */ undefined,
237+
/* implicitAttrs */ undefined,
238+
/* preserveSignificantWhitespace */ true,
239+
),
240+
).toEqual([
241+
[['{count, plural, =0 {[[<ph name="INTERPOLATION"> foo </ph>]]}}'], 'm', 'd', ''],
242+
]);
243+
});
244+
245+
it('should normalize whitespace when not preserving significant whitespace', () => {
246+
const html = '<div i18n="m|d">{count, plural, =0 {{{ foo }}}}</div>';
247+
expect(
248+
_humanizeMessages(
249+
html,
250+
/* implicitTags */ undefined,
251+
/* implicitAttrs */ undefined,
252+
/* preserveSignificantWhitespace */ false,
253+
),
254+
).toEqual([
255+
[['{count, plural, =0 {[[, <ph name="INTERPOLATION">foo</ph>, ]]}}'], 'm', 'd', ''],
256+
]);
257+
});
230258
});
231259

232260
describe('implicit elements', () => {
@@ -318,50 +346,77 @@ describe('I18nParser', () => {
318346
'',
319347
]);
320348
});
349+
350+
it('should preserve whitespace when preserving significant whitespace', () => {
351+
const html = '<div i18n="m|d">{{ foo }}</div>';
352+
expect(
353+
_humanizeMessages(
354+
html,
355+
/* implicitTags */ undefined,
356+
/* implicitAttrs */ undefined,
357+
/* preserveSignificantWhitespace */ true,
358+
),
359+
).toEqual([[['[<ph name="INTERPOLATION"> foo </ph>]'], 'm', 'd', '']]);
360+
});
361+
362+
it('should normalize whitespace when not preserving significant whitespace', () => {
363+
const html = '<div i18n="m|d">{{ foo }}</div>';
364+
expect(
365+
_humanizeMessages(
366+
html,
367+
/* implicitTags */ undefined,
368+
/* implicitAttrs */ undefined,
369+
/* preserveSignificantWhitespace */ false,
370+
),
371+
).toEqual([[['[, <ph name="INTERPOLATION">foo</ph>, ]'], 'm', 'd', '']]);
372+
});
321373
});
322374
});
323375

324376
export function _humanizeMessages(
325377
html: string,
326378
implicitTags: string[] = [],
327379
implicitAttrs: {[k: string]: string[]} = {},
380+
preserveSignificantWhitespace = true,
328381
): [string[], string, string, string][] {
329-
return _extractMessages(html, implicitTags, implicitAttrs).map((message) => [
330-
serializeNodes(message.nodes),
331-
message.meaning,
332-
message.description,
333-
message.id,
334-
]) as [string[], string, string, string][];
382+
return _extractMessages(html, implicitTags, implicitAttrs, preserveSignificantWhitespace).map(
383+
(message) => [serializeNodes(message.nodes), message.meaning, message.description, message.id],
384+
) as [string[], string, string, string][];
335385
}
336386

337387
function _humanizePlaceholders(
338388
html: string,
339389
implicitTags: string[] = [],
340390
implicitAttrs: {[k: string]: string[]} = {},
391+
preserveSignificantWhitespace = true,
341392
): string[] {
342-
return _extractMessages(html, implicitTags, implicitAttrs).map((msg) =>
343-
Object.keys(msg.placeholders)
344-
.map((name) => `${name}=${msg.placeholders[name].text}`)
345-
.join(', '),
393+
return _extractMessages(html, implicitTags, implicitAttrs, preserveSignificantWhitespace).map(
394+
(msg) =>
395+
Object.keys(msg.placeholders)
396+
.map((name) => `${name}=${msg.placeholders[name].text}`)
397+
.join(', '),
346398
);
347399
}
348400

349401
function _humanizePlaceholdersToMessage(
350402
html: string,
351403
implicitTags: string[] = [],
352404
implicitAttrs: {[k: string]: string[]} = {},
405+
preserveSignificantWhitespace = true,
353406
): string[] {
354-
return _extractMessages(html, implicitTags, implicitAttrs).map((msg) =>
355-
Object.keys(msg.placeholderToMessage)
356-
.map((k) => `${k}=${digest(msg.placeholderToMessage[k])}`)
357-
.join(', '),
407+
return _extractMessages(html, implicitTags, implicitAttrs, preserveSignificantWhitespace).map(
408+
(msg) =>
409+
Object.keys(msg.placeholderToMessage)
410+
.map((k) => `${k}=${digest(msg.placeholderToMessage[k])}`)
411+
.join(', '),
358412
);
359413
}
360414

361415
export function _extractMessages(
362416
html: string,
363417
implicitTags: string[] = [],
364418
implicitAttrs: {[k: string]: string[]} = {},
419+
preserveSignificantWhitespace = true,
365420
): Message[] {
366421
const htmlParser = new HtmlParser();
367422
const parseResult = htmlParser.parse(html, 'extractor spec', {tokenizeExpansionForms: true});
@@ -374,6 +429,6 @@ export function _extractMessages(
374429
DEFAULT_INTERPOLATION_CONFIG,
375430
implicitTags,
376431
implicitAttrs,
377-
/* preserveSignificantWhitespace */ true,
432+
preserveSignificantWhitespace,
378433
).messages;
379434
}

0 commit comments

Comments
 (0)