From 5d86e4a9b166f9a9b9f521b9fa8a83f1187d76df Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Tue, 22 Oct 2019 15:05:44 +0100 Subject: [PATCH] fix(compiler): ensure that legacy ids are rendered for ICUs (#33318) When computing i18n messages for templates there are two passes. This is because messages must be computed before any whitespace is removed. Then on a second pass, the messages must be recreated but reusing the message ids from the first pass. Previously ICUs were losing their legacy ids that had been computed via the first pass. This commit fixes that by keeping track of the message from the first pass (`previousMessage`) for ICU placeholder nodes. // FW-1637 PR Close #33318 --- .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 17 ++++ packages/compiler/src/i18n/i18n_ast.ts | 2 + packages/compiler/src/i18n/i18n_parser.ts | 6 +- .../compiler/src/render3/view/i18n/meta.ts | 95 ++++++++++++++----- .../compiler/test/render3/view/i18n_spec.ts | 2 +- 5 files changed, 92 insertions(+), 30 deletions(-) diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index 105731277c..738b965499 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -2179,6 +2179,23 @@ runInEachFileSystem(os => { expect(jsContents).not.toContain(':Some text'); }); + it('should also render legacy id for ICUs when normal messages are using legacy ids', () => { + env.tsconfig({i18nInFormat: 'xliff'}); + env.write(`test.ts`, ` + import {Component} from '@angular/core'; + @Component({ + selector: 'test', + template: '
Some text {age, plural, 10 {ten} other {other}}
' + }) + class FooCmp {}`); + env.driveMain(); + const jsContents = env.getContents('test.js'); + expect(jsContents) + .toContain( + ':@@720ba589d043a0497ac721ff972f41db0c919efb:{VAR_PLURAL, plural, 10 {ten} other {other}}'); + expect(jsContents).toContain(':@@custom:Some text'); + }); + it('@Component\'s `interpolation` should override default interpolation config', () => { env.write(`test.ts`, ` import {Component} from '@angular/core'; diff --git a/packages/compiler/src/i18n/i18n_ast.ts b/packages/compiler/src/i18n/i18n_ast.ts index 6e41f09a79..0476214f41 100644 --- a/packages/compiler/src/i18n/i18n_ast.ts +++ b/packages/compiler/src/i18n/i18n_ast.ts @@ -93,6 +93,8 @@ export class Placeholder implements Node { } export class IcuPlaceholder implements Node { + /** Used to capture a message computed from a previous processing pass (see `setI18nRefs()`). */ + previousMessage?: Message; constructor(public value: Icu, public name: string, public sourceSpan: ParseSourceSpan) {} visit(visitor: Visitor, context?: any): any { return visitor.visitIcuPlaceholder(this, context); } diff --git a/packages/compiler/src/i18n/i18n_parser.ts b/packages/compiler/src/i18n/i18n_parser.ts index 92da672346..f1be0895c8 100644 --- a/packages/compiler/src/i18n/i18n_parser.ts +++ b/packages/compiler/src/i18n/i18n_parser.ts @@ -21,8 +21,8 @@ const _expParser = new ExpressionParser(new ExpressionLexer()); export type VisitNodeFn = (html: html.Node, i18n: i18n.Node) => i18n.Node; export interface I18nMessageFactory { - (nodes: html.Node[], meaning: string, description: string, customId: string, - visitNodeFn?: VisitNodeFn): i18n.Message; + (nodes: html.Node[], meaning: string|undefined, description: string|undefined, + customId: string|undefined, visitNodeFn?: VisitNodeFn): i18n.Message; } /** @@ -54,7 +54,7 @@ class _I18nVisitor implements html.Visitor { private _interpolationConfig: InterpolationConfig) {} public toI18nMessage( - nodes: html.Node[], meaning: string, description: string, customId: string, + nodes: html.Node[], meaning = '', description = '', customId = '', visitNodeFn: VisitNodeFn|undefined): i18n.Message { const context: I18nMessageVisitorContext = { isIcu: nodes.length == 1 && nodes[0] instanceof html.Expansion, diff --git a/packages/compiler/src/render3/view/i18n/meta.ts b/packages/compiler/src/render3/view/i18n/meta.ts index 6e6b67cb0c..3fed00ce11 100644 --- a/packages/compiler/src/render3/view/i18n/meta.ts +++ b/packages/compiler/src/render3/view/i18n/meta.ts @@ -23,10 +23,20 @@ export type I18nMeta = { meaning?: string }; -function setI18nRefs(html: html.Node & {i18n?: i18n.I18nMeta}, i18n: i18n.Node): i18n.Node { - html.i18n = i18n; - return i18n; -} + +const setI18nRefs: VisitNodeFn = (htmlNode, i18nNode) => { + if (htmlNode instanceof html.NodeWithI18n) { + if (i18nNode instanceof i18n.IcuPlaceholder && htmlNode.i18n instanceof i18n.Message) { + // This html node represents an ICU but this is a second processing pass, and the legacy id + // was computed in the previous pass and stored in the `i18n` property as a message. + // We are about to wipe out that property so capture the previous message to be reused when + // generating the message for this ICU later. See `_generateI18nMessage()`. + i18nNode.previousMessage = htmlNode.i18n; + } + htmlNode.i18n = i18nNode; + } + return i18nNode; +}; /** * This visitor walks over HTML parse tree and converts information stored in @@ -44,28 +54,10 @@ export class I18nMetaVisitor implements html.Visitor { private _generateI18nMessage( nodes: html.Node[], meta: string|i18n.I18nMeta = '', visitNodeFn?: VisitNodeFn): i18n.Message { - const parsed: I18nMeta = - typeof meta === 'string' ? parseI18nMeta(meta) : metaFromI18nMessage(meta as i18n.Message); - const message = this._createI18nMessage( - nodes, parsed.meaning || '', parsed.description || '', parsed.customId || '', visitNodeFn); - if (!message.id) { - // generate (or restore) message id if not specified in template - message.id = typeof meta !== 'string' && (meta as i18n.Message).id || decimalDigest(message); - } - - if (this.i18nLegacyMessageIdFormat === 'xlf' || this.i18nLegacyMessageIdFormat === 'xliff') { - message.legacyId = computeDigest(message); - } else if ( - this.i18nLegacyMessageIdFormat === 'xlf2' || this.i18nLegacyMessageIdFormat === 'xliff2' || - this.i18nLegacyMessageIdFormat === 'xmb') { - message.legacyId = computeDecimalDigest(message); - } else if (typeof meta !== 'string') { - // This occurs if we are doing the 2nd pass after whitespace removal - // In that case we want to reuse the legacy message generated in the 1st pass - // See `parseTemplate()` in `packages/compiler/src/render3/view/template.ts` - message.legacyId = (meta as i18n.Message).legacyId; - } - + const {meaning, description, customId} = this._parseMetadata(meta); + const message = this._createI18nMessage(nodes, meaning, description, customId, visitNodeFn); + this._setMessageId(message, meta); + this._setLegacyId(message, meta); return message; } @@ -141,6 +133,57 @@ export class I18nMetaVisitor implements html.Visitor { visitAttribute(attribute: html.Attribute): any { return attribute; } visitComment(comment: html.Comment): any { return comment; } visitExpansionCase(expansionCase: html.ExpansionCase): any { return expansionCase; } + + /** + * Parse the general form `meta` passed into extract the explicit metadata needed to create a + * `Message`. + * + * There are three possibilities for the `meta` variable + * 1) a string from an `i18n` template attribute: parse it to extract the metadata values. + * 2) a `Message` from a previous processing pass: reuse the metadata values in the message. + * 4) other: ignore this and just process the message metadata as normal + * + * @param meta the bucket that holds information about the message + * @returns the parsed metadata. + */ + private _parseMetadata(meta: string|i18n.I18nMeta): I18nMeta { + return typeof meta === 'string' ? parseI18nMeta(meta) : + meta instanceof i18n.Message ? metaFromI18nMessage(meta) : {}; + } + + /** + * Generate (or restore) message id if not specified already. + */ + private _setMessageId(message: i18n.Message, meta: string|i18n.I18nMeta): void { + if (!message.id) { + message.id = meta instanceof i18n.Message && meta.id || decimalDigest(message); + } + } + + /** + * Update the `message` with a `legacyId` if necessary. + * + * @param message the message whose legacy id should be set + * @param meta information about the message being processed + */ + private _setLegacyId(message: i18n.Message, meta: string|i18n.I18nMeta): void { + if (this.i18nLegacyMessageIdFormat === 'xlf' || this.i18nLegacyMessageIdFormat === 'xliff') { + message.legacyId = computeDigest(message); + } else if ( + this.i18nLegacyMessageIdFormat === 'xlf2' || this.i18nLegacyMessageIdFormat === 'xliff2' || + this.i18nLegacyMessageIdFormat === 'xmb') { + message.legacyId = computeDecimalDigest(message); + } else if (typeof meta !== 'string') { + // This occurs if we are doing the 2nd pass after whitespace removal (see `parseTemplate()` in + // `packages/compiler/src/render3/view/template.ts`). + // In that case we want to reuse the legacy message generated in the 1st pass (see + // `setI18nRefs()`). + const previousMessage = meta instanceof i18n.Message ? + meta : + meta instanceof i18n.IcuPlaceholder ? meta.previousMessage : undefined; + message.legacyId = previousMessage && previousMessage.legacyId; + } + } } export function metaFromI18nMessage(message: i18n.Message, id: string | null = null): I18nMeta { diff --git a/packages/compiler/test/render3/view/i18n_spec.ts b/packages/compiler/test/render3/view/i18n_spec.ts index c69ad86693..b9b77c1ca3 100644 --- a/packages/compiler/test/render3/view/i18n_spec.ts +++ b/packages/compiler/test/render3/view/i18n_spec.ts @@ -21,7 +21,7 @@ import {formatI18nPlaceholderName} from '../../../src/render3/view/i18n/util'; import {parseR3 as parse} from './util'; const expressionParser = new Parser(new Lexer()); -const i18nOf = (element: t.Node & {i18n?: i18n.AST}) => element.i18n !; +const i18nOf = (element: t.Node & {i18n?: i18n.I18nMeta}) => element.i18n !; describe('I18nContext', () => { it('should support i18n content collection', () => {