Skip to content

Commit dab722f

Browse files
dgp1130alxhub
dgp1130
authored and
alxhub
committed
refactor(compiler): add i18nPreserveWhitespaceForLegacyExtraction (#56507)
This configures whether or not to preserve whitespace content when extracting messages from Angular templates in the legacy (View Engine) extraction pipeline. This includes several bug fixes which unfortunately cannot be landed without changing message IDs in a breaking fashion and are necessary to properly trim whitespace. Instead these bug fixes are included only when the new flag is disabled. PR Close #56507
1 parent 0300dd2 commit dab722f

File tree

27 files changed

+840
-58
lines changed

27 files changed

+840
-58
lines changed

‎goldens/public-api/compiler-cli/compiler_options.api.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ export interface I18nOptions {
3838
i18nOutFile?: string;
3939
i18nOutFormat?: string;
4040
i18nOutLocale?: string;
41+
i18nPreserveWhitespaceForLegacyExtraction?: boolean;
4142
i18nUseExternalIds?: boolean;
4243
}
4344

‎packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,13 +251,15 @@ export class ComponentDecoratorHandler
251251
private readonly enableLetSyntax: boolean,
252252
private readonly localCompilationExtraImportsTracker: LocalCompilationExtraImportsTracker | null,
253253
private readonly jitDeclarationRegistry: JitDeclarationRegistry,
254+
private readonly i18nPreserveSignificantWhitespace: boolean,
254255
) {
255256
this.extractTemplateOptions = {
256257
enableI18nLegacyMessageIdFormat: this.enableI18nLegacyMessageIdFormat,
257258
i18nNormalizeLineEndingsInICUs: this.i18nNormalizeLineEndingsInICUs,
258259
usePoisonedData: this.usePoisonedData,
259260
enableBlockSyntax: this.enableBlockSyntax,
260261
enableLetSyntax: this.enableLetSyntax,
262+
preserveSignificantWhitespace: this.i18nPreserveSignificantWhitespace,
261263
};
262264
}
263265

@@ -278,6 +280,7 @@ export class ComponentDecoratorHandler
278280
usePoisonedData: boolean;
279281
enableBlockSyntax: boolean;
280282
enableLetSyntax: boolean;
283+
preserveSignificantWhitespace?: boolean;
281284
};
282285

283286
readonly precedence = HandlerPrecedence.PRIMARY;
@@ -646,6 +649,7 @@ export class ComponentDecoratorHandler
646649
usePoisonedData: this.usePoisonedData,
647650
enableBlockSyntax: this.enableBlockSyntax,
648651
enableLetSyntax: this.enableLetSyntax,
652+
preserveSignificantWhitespace: this.i18nPreserveSignificantWhitespace,
649653
},
650654
this.compilationMode,
651655
);

‎packages/compiler-cli/src/ngtsc/annotations/component/src/resources.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ export interface ExtractTemplateOptions {
135135
i18nNormalizeLineEndingsInICUs: boolean;
136136
enableBlockSyntax: boolean;
137137
enableLetSyntax: boolean;
138+
preserveSignificantWhitespace?: boolean;
138139
}
139140

140141
export function extractTemplate(
@@ -277,15 +278,16 @@ function parseExtractedTemplate(
277278
const parsedTemplate = parseTemplate(sourceStr, sourceMapUrl ?? '', {
278279
...commonParseOptions,
279280
preserveWhitespaces: template.preserveWhitespaces,
281+
preserveSignificantWhitespace: options.preserveSignificantWhitespace,
280282
});
281283

282284
// Unfortunately, the primary parse of the template above may not contain accurate source map
283285
// information. If used directly, it would result in incorrect code locations in template
284286
// errors, etc. There are three main problems:
285287
//
286-
// 1. `preserveWhitespaces: false` annihilates the correctness of template source mapping, as
287-
// the whitespace transformation changes the contents of HTML text nodes before they're
288-
// parsed into Angular expressions.
288+
// 1. `preserveWhitespaces: false` or `preserveSignificantWhitespace: false` annihilates the
289+
// correctness of template source mapping, as the whitespace transformation changes the
290+
// contents of HTML text nodes before they're parsed into Angular expressions.
289291
// 2. `preserveLineEndings: false` causes growing misalignments in templates that use '\r\n'
290292
// line endings, by normalizing them to '\n'.
291293
// 3. By default, the template parser strips leading trivia characters (like spaces, tabs, and
@@ -298,6 +300,7 @@ function parseExtractedTemplate(
298300
...commonParseOptions,
299301
preserveWhitespaces: true,
300302
preserveLineEndings: true,
303+
preserveSignificantWhitespace: true,
301304
leadingTriviaChars: [],
302305
});
303306

‎packages/compiler-cli/src/ngtsc/annotations/component/test/component_spec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ function setup(
147147
/* enableLetSyntax */ true,
148148
/* localCompilationExtraImportsTracker */ null,
149149
jitDeclarationRegistry,
150+
/* i18nPreserveSignificantWhitespace */ true,
150151
);
151152
return {reflectionHost, handler, resourceLoader, metaRegistry};
152153
}

‎packages/compiler-cli/src/ngtsc/core/api/src/public_options.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,14 @@ export interface I18nOptions {
390390
* The default is `false`, but this will be switched in a future major release.
391391
*/
392392
i18nNormalizeLineEndingsInICUs?: boolean;
393+
394+
/**
395+
* Whether or not to preserve whitespace when extracting messages with the legacy (View Engine)
396+
* pipeline.
397+
*
398+
* Defaults to `true`.
399+
*/
400+
i18nPreserveWhitespaceForLegacyExtraction?: boolean;
393401
}
394402

395403
/**

‎packages/compiler-cli/src/ngtsc/core/src/compiler.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1446,6 +1446,7 @@ export class NgCompiler {
14461446
this.enableLetSyntax,
14471447
localCompilationExtraImportsTracker,
14481448
jitDeclarationRegistry,
1449+
this.options.i18nPreserveWhitespaceForLegacyExtraction ?? true,
14491450
),
14501451

14511452
// TODO(alxhub): understand why the cast here is necessary (something to do with `null`

‎packages/compiler-cli/src/ngtsc/program.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,13 @@ export class NgtscProgram implements api.Program {
258258
}
259259

260260
private emitXi18n(): void {
261-
const ctx = new MessageBundle(new HtmlParser(), [], {}, this.options.i18nOutLocale ?? null);
261+
const ctx = new MessageBundle(
262+
new HtmlParser(),
263+
[],
264+
{},
265+
this.options.i18nOutLocale ?? null,
266+
this.options.i18nPreserveWhitespaceForLegacyExtraction,
267+
);
262268
this.compiler.xi18n(ctx);
263269
i18nExtract(
264270
this.options.i18nOutFormat ?? null,

‎packages/compiler-cli/src/transformers/i18n.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,13 @@ export function i18nSerialize(
5757

5858
switch (format) {
5959
case 'xmb':
60-
serializer = new Xmb();
60+
serializer = new Xmb(
61+
// Whenever we disable whitespace preservation, we also want to stop preserving
62+
// placeholders because they contain whitespace we want to drop too. Whitespace
63+
// inside `{{ name }}` should be ignored for the same reasons as whitespace
64+
// outside placeholders.
65+
/* preservePlaceholders */ options.i18nPreserveWhitespaceForLegacyExtraction,
66+
);
6167
break;
6268
case 'xliff2':
6369
case 'xlf2':

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

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,15 @@ export function computeDigest(message: i18n.Message): string {
3232
/**
3333
* Return the message id or compute it using the XLIFF2/XMB/$localize digest.
3434
*/
35-
export function decimalDigest(message: i18n.Message): string {
36-
return message.id || computeDecimalDigest(message);
35+
export function decimalDigest(message: i18n.Message, preservePlaceholders: boolean): string {
36+
return message.id || computeDecimalDigest(message, preservePlaceholders);
3737
}
3838

3939
/**
4040
* Compute the message id using the XLIFF2/XMB/$localize digest.
4141
*/
42-
export function computeDecimalDigest(message: i18n.Message): string {
43-
const visitor = new _SerializerIgnoreIcuExpVisitor();
42+
export function computeDecimalDigest(message: i18n.Message, preservePlaceholders: boolean): string {
43+
const visitor = new _SerializerIgnoreExpVisitor(preservePlaceholders);
4444
const parts = message.nodes.map((a) => a.visit(visitor, null));
4545
return computeMsgId(parts.join(''), message.meaning);
4646
}
@@ -100,12 +100,23 @@ export function serializeNodes(nodes: i18n.Node[]): string[] {
100100
/**
101101
* Serialize the i18n ast to something xml-like in order to generate an UID.
102102
*
103-
* Ignore the ICU expressions so that message IDs stays identical if only the expression changes.
103+
* Ignore the expressions so that message IDs stays identical if only the expression changes.
104104
*
105105
* @internal
106106
*/
107-
class _SerializerIgnoreIcuExpVisitor extends _SerializerVisitor {
108-
override visitIcu(icu: i18n.Icu, context: any): any {
107+
class _SerializerIgnoreExpVisitor extends _SerializerVisitor {
108+
constructor(private readonly preservePlaceholders: boolean) {
109+
super();
110+
}
111+
112+
override visitPlaceholder(ph: i18n.Placeholder, context: any): string {
113+
// Do not take the expression into account when `preservePlaceholders` is disabled.
114+
return this.preservePlaceholders
115+
? super.visitPlaceholder(ph, context)
116+
: `<ph name="${ph.name}"/>`;
117+
}
118+
119+
override visitIcu(icu: i18n.Icu): string {
109120
let strCases = Object.keys(icu.cases).map((k: string) => `${k} {${icu.cases[k].visit(this)}}`);
110121
// Do not take the expression into account
111122
return `{${icu.type}, ${strCases.join(', ')}}`;

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,9 @@ export function extractMessages(
3030
interpolationConfig: InterpolationConfig,
3131
implicitTags: string[],
3232
implicitAttrs: {[k: string]: string[]},
33+
preserveSignificantWhitespace: boolean,
3334
): ExtractionResult {
34-
const visitor = new _Visitor(implicitTags, implicitAttrs);
35+
const visitor = new _Visitor(implicitTags, implicitAttrs, preserveSignificantWhitespace);
3536
return visitor.extract(nodes, interpolationConfig);
3637
}
3738

@@ -98,6 +99,7 @@ class _Visitor implements html.Visitor {
9899
constructor(
99100
private _implicitTags: string[],
100101
private _implicitAttrs: {[k: string]: string[]},
102+
private readonly _preserveSignificantWhitespace: boolean = true,
101103
) {}
102104

103105
/**
@@ -347,6 +349,10 @@ class _Visitor implements html.Visitor {
347349
this._createI18nMessage = createI18nMessageFactory(
348350
interpolationConfig,
349351
DEFAULT_CONTAINER_BLOCKS,
352+
// When dropping significant whitespace we need to retain whitespace tokens or
353+
// else we won't be able to reuse source spans because empty tokens would be
354+
// removed and cause a mismatch.
355+
!this._preserveSignificantWhitespace /* retainEmptyTokens */,
350356
);
351357
}
352358

0 commit comments

Comments
 (0)