Skip to content

Commit 6d33ab5

Browse files
dgp1130devversion
dgp1130
authored and
devversion
committed
refactor(compiler): remove preservePlaceholders (#58176)
While effective, `preservePlaceholders` unfortunately is not viable in google3 at the moment due to some complexities with how TC extracts messages. Therefore this feature is being removed in favor of whitespace trimming of expressions, which is viable for TC and provides most of the same benefit. This is a partial revert of dab722f. PR Close #58176
1 parent 6c126d5 commit 6d33ab5

File tree

10 files changed

+20
-49
lines changed

10 files changed

+20
-49
lines changed

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

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

5858
switch (format) {
5959
case '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-
);
60+
serializer = new Xmb();
6761
break;
6862
case 'xliff2':
6963
case 'xlf2':

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

Lines changed: 6 additions & 17 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, preservePlaceholders: boolean): string {
36-
return message.id || computeDecimalDigest(message, preservePlaceholders);
35+
export function decimalDigest(message: i18n.Message): string {
36+
return message.id || computeDecimalDigest(message);
3737
}
3838

3939
/**
4040
* Compute the message id using the XLIFF2/XMB/$localize digest.
4141
*/
42-
export function computeDecimalDigest(message: i18n.Message, preservePlaceholders: boolean): string {
43-
const visitor = new _SerializerIgnoreExpVisitor(preservePlaceholders);
42+
export function computeDecimalDigest(message: i18n.Message): string {
43+
const visitor = new _SerializerIgnoreIcuExpVisitor();
4444
const parts = message.nodes.map((a) => a.visit(visitor, null));
4545
return computeMsgId(parts.join(''), message.meaning);
4646
}
@@ -100,22 +100,11 @@ 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 expressions so that message IDs stays identical if only the expression changes.
103+
* Ignore the ICU expressions so that message IDs stays identical if only the expression changes.
104104
*
105105
* @internal
106106
*/
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-
107+
class _SerializerIgnoreIcuExpVisitor extends _SerializerVisitor {
119108
override visitIcu(icu: i18n.Icu): string {
120109
let strCases = Object.keys(icu.cases).map((k: string) => `${k} {${icu.cases[k].visit(this)}}`);
121110
// Do not take the expression into account

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ function createSerializer(format?: string): Serializer {
7979

8080
switch (format) {
8181
case 'xmb':
82-
return new Xmb(/* preservePlaceholders */ true);
82+
return new Xmb();
8383
case 'xtb':
8484
return new Xtb();
8585
case 'xliff2':

‎packages/compiler/src/i18n/serializers/xliff2.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ export class Xliff2 extends Serializer {
128128
}
129129

130130
override digest(message: i18n.Message): string {
131-
return decimalDigest(message, /* preservePlaceholders */ true);
131+
return decimalDigest(message);
132132
}
133133
}
134134

‎packages/compiler/src/i18n/serializers/xmb.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,6 @@ const _DOCTYPE = `<!ELEMENT messagebundle (msg)*>
4848
<!ELEMENT ex (#PCDATA)>`;
4949

5050
export class Xmb extends Serializer {
51-
constructor(private readonly preservePlaceholders: boolean = true) {
52-
super();
53-
}
54-
5551
override write(messages: i18n.Message[], locale: string | null): string {
5652
const exampleVisitor = new ExampleVisitor();
5753
const visitor = new _Visitor();
@@ -108,7 +104,7 @@ export class Xmb extends Serializer {
108104
}
109105

110106
override digest(message: i18n.Message): string {
111-
return digest(message, this.preservePlaceholders);
107+
return digest(message);
112108
}
113109

114110
override createNameMapper(message: i18n.Message): PlaceholderMapper {
@@ -206,8 +202,8 @@ class _Visitor implements i18n.Visitor {
206202
}
207203
}
208204

209-
export function digest(message: i18n.Message, preservePlaceholders: boolean): string {
210-
return decimalDigest(message, preservePlaceholders);
205+
export function digest(message: i18n.Message): string {
206+
return decimalDigest(message);
211207
}
212208

213209
// TC requires at least one non-empty example on placeholders

‎packages/compiler/src/i18n/serializers/xtb.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ export class Xtb extends Serializer {
5757
}
5858

5959
override digest(message: i18n.Message): string {
60-
return digest(message, /* preservePlaceholders */ true);
60+
return digest(message);
6161
}
6262

6363
override createNameMapper(message: i18n.Message): PlaceholderMapper {

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

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -256,9 +256,7 @@ export class I18nMetaVisitor implements html.Visitor {
256256
*/
257257
private _setMessageId(message: i18n.Message, meta: string | i18n.I18nMeta): void {
258258
if (!message.id) {
259-
message.id =
260-
(meta instanceof i18n.Message && meta.id) ||
261-
decimalDigest(message, /* preservePlaceholders */ this.preserveSignificantWhitespace);
259+
message.id = (meta instanceof i18n.Message && meta.id) || decimalDigest(message);
262260
}
263261
}
264262

@@ -270,13 +268,7 @@ export class I18nMetaVisitor implements html.Visitor {
270268
*/
271269
private _setLegacyIds(message: i18n.Message, meta: string | i18n.I18nMeta): void {
272270
if (this.enableI18nLegacyMessageIdFormat) {
273-
message.legacyIds = [
274-
computeDigest(message),
275-
computeDecimalDigest(
276-
message,
277-
/* preservePlaceholders */ this.preserveSignificantWhitespace,
278-
),
279-
];
271+
message.legacyIds = [computeDigest(message), computeDecimalDigest(message)];
280272
} else if (typeof meta !== 'string') {
281273
// This occurs if we are doing the 2nd pass after whitespace removal (see `parseTemplate()` in
282274
// `packages/compiler/src/render3/view/template.ts`).

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ xdescribe('i18n XMB/XTB integration spec', () => {
2424
beforeEach(waitForAsync(() => configureCompiler(XTB + LF_LINE_ENDING_XTB, 'xtb')));
2525

2626
it('should extract from templates', () => {
27-
const serializer = new Xmb(/* preservePlaceholders */ true);
27+
const serializer = new Xmb();
2828
const serializedXmb = serializeTranslations(HTML, serializer);
2929

3030
XMB.forEach((x) => {
@@ -43,7 +43,7 @@ xdescribe('i18n XMB/XTB integration spec', () => {
4343
beforeEach(waitForAsync(() => configureCompiler(XTB + CRLF_LINE_ENDING_XTB, 'xtb')));
4444

4545
it('should extract from templates (with CRLF line endings)', () => {
46-
const serializer = new Xmb(/* preservePlaceholders */ true);
46+
const serializer = new Xmb();
4747
const serializedXmb = serializeTranslations(HTML.replace(/\n/g, '\r\n'), serializer);
4848

4949
XMB.forEach((x) => {

‎packages/compiler/test/i18n/serializers/xmb_spec.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,15 @@ lines</msg>
7070

7171
it('should throw when trying to load an xmb file', () => {
7272
expect(() => {
73-
const serializer = new Xmb(/* preservePlaceholders */ true);
73+
const serializer = new Xmb();
7474
serializer.load(XMB, 'url');
7575
}).toThrowError(/Unsupported/);
7676
});
7777
});
7878

7979
function toXmb(html: string, url: string, locale: string | null = null): string {
8080
const catalog = new MessageBundle(new HtmlParser(), [], {}, locale);
81-
const serializer = new Xmb(/* preservePlaceholders */ true);
81+
const serializer = new Xmb();
8282

8383
catalog.updateFromTemplate(html, url, DEFAULT_INTERPOLATION_CONFIG);
8484

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ function extractMessages(source: string, preserveWhitespace: boolean): Assertabl
461461

462462
const messages = bundle.getMessages();
463463

464-
const xmbSerializer = new Xmb(/* preservePlaceholders */ preserveWhitespace);
464+
const xmbSerializer = new Xmb();
465465
return messages.map((message) => ({
466466
id: xmbSerializer.digest(message),
467467
text: message.nodes.map((node) => node.visit(debugSerializer)).join(''),

0 commit comments

Comments
 (0)