From e524322c43ce31c5c6d45bcdc320338b9bd66249 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Tue, 3 Dec 2019 08:36:38 +0000 Subject: [PATCH] refactor(compiler): i18n - render legacy i18n message ids (#34135) Now that `@angular/localize` can interpret multiple legacy message ids in the metablock of a `$localize` tagged template string, this commit adds those ids to each i18n message extracted from component templates, but only if the `enableI18nLegacyMessageIdFormat` is not `false`. PR Close #34135 --- .../src/ngtsc/annotations/src/component.ts | 4 +- .../ngtsc/annotations/test/component_spec.ts | 2 +- packages/compiler-cli/src/ngtsc/program.ts | 11 +- packages/compiler-cli/src/transformers/api.ts | 7 +- .../test/compliance/mock_compile.ts | 3 +- .../compliance/r3_view_compiler_i18n_spec.ts | 32 ++++++ .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 108 +++++------------- packages/compiler/src/i18n/i18n_ast.ts | 4 +- packages/compiler/src/output/output_ast.ts | 14 ++- .../compiler/src/render3/view/i18n/meta.ts | 20 ++-- .../compiler/src/render3/view/template.ts | 18 ++- packages/compiler/test/i18n/digest_spec.ts | 1 + 12 files changed, 100 insertions(+), 124 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index 7706e29874..ead2eb8ab0 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -52,7 +52,7 @@ export class ComponentDecoratorHandler implements private scopeReader: ComponentScopeReader, private scopeRegistry: LocalModuleScopeRegistry, private isCore: boolean, private resourceLoader: ResourceLoader, private rootDirs: string[], private defaultPreserveWhitespaces: boolean, private i18nUseExternalIds: boolean, - private i18nLegacyMessageIdFormat: string, private moduleResolver: ModuleResolver, + private enableI18nLegacyMessageIdFormat: boolean, private moduleResolver: ModuleResolver, private cycleAnalyzer: CycleAnalyzer, private refEmitter: ReferenceEmitter, private defaultImportRecorder: DefaultImportRecorder, private annotateForClosureCompiler: boolean, @@ -710,7 +710,7 @@ export class ComponentDecoratorHandler implements preserveWhitespaces, interpolationConfig: interpolation, range: templateRange, escapedString, - i18nLegacyMessageIdFormat: this.i18nLegacyMessageIdFormat, ...options, + enableI18nLegacyMessageIdFormat: this.enableI18nLegacyMessageIdFormat, ...options, }), template: templateStr, templateUrl, isInline: component.has('template'), diff --git a/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts index 5cb2b5b395..c44841cbe6 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts @@ -63,7 +63,7 @@ runInEachFileSystem(() => { reflectionHost, evaluator, metaRegistry, metaReader, scopeRegistry, scopeRegistry, /* isCore */ false, new NoopResourceLoader(), /* rootDirs */[''], /* defaultPreserveWhitespaces */ false, /* i18nUseExternalIds */ true, - /* i18nLegacyMessageIdFormat */ '', moduleResolver, cycleAnalyzer, refEmitter, + /* enableI18nLegacyMessageIdFormat */ false, moduleResolver, cycleAnalyzer, refEmitter, NOOP_DEFAULT_IMPORT_RECORDER, /* annotateForClosureCompiler */ false); const TestCmp = getDeclaration(program, _('/entry.ts'), 'TestCmp', isNamedClassDeclaration); const detected = handler.detect(TestCmp, reflectionHost.getDecoratorsOfDeclaration(TestCmp)); diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts index 9cc86026ee..7378985406 100644 --- a/packages/compiler-cli/src/ngtsc/program.ts +++ b/packages/compiler-cli/src/ngtsc/program.ts @@ -623,9 +623,9 @@ export class NgtscProgram implements api.Program { this.reflector, evaluator, metaRegistry, this.metaReader !, scopeReader, scopeRegistry, this.isCore, this.resourceManager, this.rootDirs, this.options.preserveWhitespaces || false, this.options.i18nUseExternalIds !== false, - this.getI18nLegacyMessageFormat(), this.moduleResolver, this.cycleAnalyzer, - this.refEmitter, this.defaultImportTracker, this.closureCompilerEnabled, - this.incrementalDriver), + this.options.enableI18nLegacyMessageIdFormat !== false, this.moduleResolver, + this.cycleAnalyzer, this.refEmitter, this.defaultImportTracker, + this.closureCompilerEnabled, this.incrementalDriver), new DirectiveDecoratorHandler( this.reflector, evaluator, metaRegistry, this.defaultImportTracker, this.isCore, this.closureCompilerEnabled), @@ -648,11 +648,6 @@ export class NgtscProgram implements api.Program { this.options.compileNonExportedClasses !== false); } - private getI18nLegacyMessageFormat(): string { - return this.options.enableI18nLegacyMessageIdFormat !== false && this.options.i18nInFormat || - ''; - } - private get reflector(): TypeScriptReflectionHost { if (this._reflector === undefined) { this._reflector = new TypeScriptReflectionHost(this.tsProgram.getTypeChecker()); diff --git a/packages/compiler-cli/src/transformers/api.ts b/packages/compiler-cli/src/transformers/api.ts index 0a806692e9..48c178363e 100644 --- a/packages/compiler-cli/src/transformers/api.ts +++ b/packages/compiler-cli/src/transformers/api.ts @@ -263,11 +263,10 @@ export interface CompilerOptions extends ts.CompilerOptions { i18nUseExternalIds?: boolean; /** - * Render `$localize` message ids with the legacy format (xlf, xlf2 or xmb) specified in - * `i18nInFormat`. + * Render `$localize` messages with legacy format ids. * - * This is only active if we are building with `enableIvy: true` and a valid - * `i18nInFormat` has been provided. The default value for now is `true`. + * This is only active if we are building with `enableIvy: true`. + * The default value for now is `true`. * * Use this option when use are using the `$localize` based localization messages but * have not migrated the translation files to use the new `$localize` message id format. diff --git a/packages/compiler-cli/test/compliance/mock_compile.ts b/packages/compiler-cli/test/compliance/mock_compile.ts index 6d55d1a376..e0c905ded7 100644 --- a/packages/compiler-cli/test/compliance/mock_compile.ts +++ b/packages/compiler-cli/test/compliance/mock_compile.ts @@ -210,7 +210,8 @@ export function compile( scripts, { target: ts.ScriptTarget.ES2015, module: ts.ModuleKind.ES2015, - moduleResolution: ts.ModuleResolutionKind.NodeJs, ...options, + moduleResolution: ts.ModuleResolutionKind.NodeJs, + enableI18nLegacyMessageIdFormat: false, ...options, }, mockCompilerHost); program.emit(); diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts index b1af024abd..9133c2c705 100644 --- a/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts @@ -3457,6 +3457,38 @@ describe('i18n support in the template compiler', () => { }); }); + describe('$localize legacy message ids', () => { + it('should add legacy message ids if `enableI18nLegacyMessageIdFormat` is true', () => { + const input = `
Some Message
`; + + const output = String.raw ` + var $I18N_0$; + if (ngI18nClosureMode) { … } + else { + $I18N_0$ = $localize \`:␟ec93160d6d6a8822214060dd7938bf821c22b226␟6795333002533525253:Some Message\`; + } + … + `; + + verify(input, output, {compilerOptions: {enableI18nLegacyMessageIdFormat: true}}); + }); + + it('should add legacy message ids if `enableI18nLegacyMessageIdFormat` is undefined', () => { + const input = `
Some Message
`; + + const output = String.raw ` + var $I18N_0$; + if (ngI18nClosureMode) { … } + else { + $I18N_0$ = $localize \`:␟ec93160d6d6a8822214060dd7938bf821c22b226␟6795333002533525253:Some Message\`; + } + … + `; + + verify(input, output, {compilerOptions: {enableI18nLegacyMessageIdFormat: undefined}}); + }); + }); + describe('errors', () => { const verifyNestedSectionsError = (errorThrown: any, expectedErrorText: string) => { expect(errorThrown.ngParseErrors.length).toBe(1); diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index 88cdaaad29..877b8dc0c7 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -2454,82 +2454,23 @@ runInEachFileSystem(os => { expect(jsContents).not.toContain('MSG_EXTERNAL_'); }); - it('should render legacy id when `enableI18nLegacyMessageIdFormat` is not false and `i18nInFormat` is set to "xlf"', - () => { - env.tsconfig({i18nInFormat: 'xlf'}); - env.write(`test.ts`, ` + it('should render legacy ids when `enableI18nLegacyMessageIdFormat` is not false', () => { + env.tsconfig({}); + env.write(`test.ts`, ` import {Component} from '@angular/core'; @Component({ selector: 'test', template: '
Some text
' }) class FooCmp {}`); - env.driveMain(); - const jsContents = env.getContents('test.js'); - expect(jsContents).toContain(':@@5dbba0a3da8dff890e20cf76eb075d58900fbcd3:Some text'); - }); + env.driveMain(); + const jsContents = env.getContents('test.js'); + expect(jsContents) + .toContain( + '":\\u241F5dbba0a3da8dff890e20cf76eb075d58900fbcd3\\u241F8321000940098097247:Some text"'); + }); - it('should render legacy id when `enableI18nLegacyMessageIdFormat` is not false and `i18nInFormat` is set to "xliff"', - () => { - env.tsconfig({i18nInFormat: 'xliff'}); - env.write(`test.ts`, ` - import {Component} from '@angular/core'; - @Component({ - selector: 'test', - template: '
Some text
' - }) - class FooCmp {}`); - env.driveMain(); - const jsContents = env.getContents('test.js'); - expect(jsContents).toContain(':@@5dbba0a3da8dff890e20cf76eb075d58900fbcd3:Some text'); - }); - - it('should render legacy id when `enableI18nLegacyMessageIdFormat` is not false and `i18nInFormat` is set to "xlf2"', - () => { - env.tsconfig({i18nInFormat: 'xlf2'}); - env.write(`test.ts`, ` - import {Component} from '@angular/core'; - @Component({ - selector: 'test', - template: '
Some text
' - }) - class FooCmp {}`); - env.driveMain(); - const jsContents = env.getContents('test.js'); - expect(jsContents).toContain(':@@8321000940098097247:Some text'); - }); - - it('should render legacy id when `enableI18nLegacyMessageIdFormat` is not false and `i18nInFormat` is set to "xliff2"', - () => { - env.tsconfig({i18nInFormat: 'xliff2'}); - env.write(`test.ts`, ` - import {Component} from '@angular/core'; - @Component({ - selector: 'test', - template: '
Some text
' - }) - class FooCmp {}`); - env.driveMain(); - const jsContents = env.getContents('test.js'); - expect(jsContents).toContain(':@@8321000940098097247:Some text'); - }); - - it('should render legacy id when `enableI18nLegacyMessageIdFormat` is not false and `i18nInFormat` is set to "xmb"', - () => { - env.tsconfig({i18nInFormat: 'xmb'}); - env.write(`test.ts`, ` - import {Component} from '@angular/core'; - @Component({ - selector: 'test', - template: '
Some text
' - }) - class FooCmp {}`); - env.driveMain(); - const jsContents = env.getContents('test.js'); - expect(jsContents).toContain(':@@8321000940098097247:Some text'); - }); - - it('should render custom id even if `enableI18nLegacyMessageIdFormat` is not false and `i18nInFormat` is set', + it('should render custom id and legacy ids if `enableI18nLegacyMessageIdFormat` is not false', () => { env.tsconfig({i18nFormatIn: 'xlf'}); env.write(`test.ts`, ` @@ -2541,25 +2482,28 @@ runInEachFileSystem(os => { class FooCmp {}`); env.driveMain(); const jsContents = env.getContents('test.js'); - expect(jsContents).toContain(':@@custom:Some text'); + expect(jsContents) + .toContain( + ':@@custom\\u241F5dbba0a3da8dff890e20cf76eb075d58900fbcd3\\u241F8321000940098097247:Some text'); }); - it('should not render legacy id when `enableI18nLegacyMessageIdFormat` is set to false', () => { - env.tsconfig({enableI18nLegacyMessageIdFormat: false, i18nInFormat: 'xmb'}); - env.write(`test.ts`, ` + it('should not render legacy ids when `enableI18nLegacyMessageIdFormat` is set to false', + () => { + env.tsconfig({enableI18nLegacyMessageIdFormat: false, i18nInFormat: 'xmb'}); + env.write(`test.ts`, ` import {Component} from '@angular/core'; @Component({ selector: 'test', template: '
Some text
' }) class FooCmp {}`); - env.driveMain(); - const jsContents = env.getContents('test.js'); - // Note that the colon would only be there if there is an id attached to the string. - expect(jsContents).not.toContain(':Some text'); - }); + env.driveMain(); + const jsContents = env.getContents('test.js'); + // Note that the colon would only be there if there is an id attached to the string. + expect(jsContents).not.toContain(':Some text'); + }); - it('should also render legacy id for ICUs when normal messages are using legacy ids', () => { + it('should also render legacy ids for ICUs when normal messages are using legacy ids', () => { env.tsconfig({i18nInFormat: 'xliff'}); env.write(`test.ts`, ` import {Component} from '@angular/core'; @@ -2572,8 +2516,10 @@ runInEachFileSystem(os => { const jsContents = env.getContents('test.js'); expect(jsContents) .toContain( - ':@@720ba589d043a0497ac721ff972f41db0c919efb:{VAR_PLURAL, plural, 10 {ten} other {other}}'); - expect(jsContents).toContain(':@@custom:Some text'); + ':\\u241F720ba589d043a0497ac721ff972f41db0c919efb\\u241F3221232817843005870:{VAR_PLURAL, plural, 10 {ten} other {other}}'); + expect(jsContents) + .toContain( + ':@@custom\\u241Fdcb6170595f5d548a3d00937e87d11858f51ad04\\u241F7419139165339437596:Some text'); }); it('@Component\'s `interpolation` should override default interpolation config', () => { diff --git a/packages/compiler/src/i18n/i18n_ast.ts b/packages/compiler/src/i18n/i18n_ast.ts index 0476214f41..9ec6afe1ba 100644 --- a/packages/compiler/src/i18n/i18n_ast.ts +++ b/packages/compiler/src/i18n/i18n_ast.ts @@ -11,8 +11,8 @@ import {ParseSourceSpan} from '../parse_util'; export class Message { sources: MessageSpan[]; id: string = this.customId; - /** The id to use if there is no custom id and if `i18nLegacyMessageIdFormat` is not empty */ - legacyId?: string = ''; + /** The ids to use if there are no custom id and if `i18nLegacyMessageIdFormat` is not empty */ + legacyIds: string[] = []; /** * @param nodes message AST diff --git a/packages/compiler/src/output/output_ast.ts b/packages/compiler/src/output/output_ast.ts index 1071ae554e..5e6660af2f 100644 --- a/packages/compiler/src/output/output_ast.ts +++ b/packages/compiler/src/output/output_ast.ts @@ -509,12 +509,20 @@ export class LocalizedString extends Expression { * @param messagePart The first part of the tagged string */ serializeI18nHead(): {cooked: string, raw: string} { + const MEANING_SEPARATOR = '|'; + const ID_SEPARATOR = '@@'; + const LEGACY_ID_INDICATOR = '␟'; + let metaBlock = this.metaBlock.description || ''; if (this.metaBlock.meaning) { - metaBlock = `${this.metaBlock.meaning}|${metaBlock}`; + metaBlock = `${this.metaBlock.meaning}${MEANING_SEPARATOR}${metaBlock}`; } - if (this.metaBlock.customId || this.metaBlock.legacyId) { - metaBlock = `${metaBlock}@@${this.metaBlock.customId || this.metaBlock.legacyId}`; + if (this.metaBlock.customId) { + metaBlock = `${metaBlock}${ID_SEPARATOR}${this.metaBlock.customId}`; + } + if (this.metaBlock.legacyIds) { + this.metaBlock.legacyIds.forEach( + legacyId => { metaBlock = `${metaBlock}${LEGACY_ID_INDICATOR}${legacyId}`; }); } return createCookedRawString(metaBlock, this.messageParts[0]); } diff --git a/packages/compiler/src/render3/view/i18n/meta.ts b/packages/compiler/src/render3/view/i18n/meta.ts index a1122d0357..fb4d5fc9d5 100644 --- a/packages/compiler/src/render3/view/i18n/meta.ts +++ b/packages/compiler/src/render3/view/i18n/meta.ts @@ -18,7 +18,7 @@ import {I18N_ATTR, I18N_ATTR_PREFIX, hasI18nAttrs, icuFromI18nMessage} from './u export type I18nMeta = { id?: string, customId?: string, - legacyId?: string, + legacyIds?: string[], description?: string, meaning?: string }; @@ -52,7 +52,7 @@ export class I18nMetaVisitor implements html.Visitor { constructor( private interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG, - private keepI18nAttrs: boolean = false, private i18nLegacyMessageIdFormat: string = '') {} + private keepI18nAttrs = false, private enableI18nLegacyMessageIdFormat = false) {} private _generateI18nMessage( nodes: html.Node[], meta: string|i18n.I18nMeta = '', @@ -60,7 +60,7 @@ export class I18nMetaVisitor implements html.Visitor { const {meaning, description, customId} = this._parseMetadata(meta); const message = this._createI18nMessage(nodes, meaning, description, customId, visitNodeFn); this._setMessageId(message, meta); - this._setLegacyId(message, meta); + this._setLegacyIds(message, meta); return message; } @@ -171,13 +171,9 @@ export class I18nMetaVisitor implements html.Visitor { * @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); + private _setLegacyIds(message: i18n.Message, meta: string|i18n.I18nMeta): void { + if (this.enableI18nLegacyMessageIdFormat) { + message.legacyIds = [computeDigest(message), 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`). @@ -186,7 +182,7 @@ export class I18nMetaVisitor implements html.Visitor { const previousMessage = meta instanceof i18n.Message ? meta : meta instanceof i18n.IcuPlaceholder ? meta.previousMessage : undefined; - message.legacyId = previousMessage && previousMessage.legacyId; + message.legacyIds = previousMessage ? previousMessage.legacyIds : []; } } } @@ -195,7 +191,7 @@ export function metaFromI18nMessage(message: i18n.Message, id: string | null = n return { id: typeof id === 'string' ? id : message.id || '', customId: message.customId, - legacyId: message.legacyId, + legacyIds: message.legacyIds, meaning: message.meaning || '', description: message.description || '' }; diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index 957935241d..897957aec9 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -1945,17 +1945,14 @@ export interface ParseTemplateOptions { leadingTriviaChars?: string[]; /** - * Render `$localize` message ids with the specified legacy format (xlf, xlf2 or xmb). + * Render `$localize` message ids with additional legacy message ids. * - * Use this option when use are using the `$localize` based localization messages but - * have not migrated the translation files to use the new `$localize` message id format. + * This option defaults to `true` but in the future the defaul will be flipped. * - * @deprecated - * `i18nLegacyMessageIdFormat` should only be used while migrating from legacy message id - * formatted translation files and will be removed at the same time as ViewEngine support is - * removed. + * For now set this option to false if you have migrated the translation files to use the new + * `$localize` message id format and you are not using compile time translation merging. */ - i18nLegacyMessageIdFormat?: string; + enableI18nLegacyMessageIdFormat?: boolean; } /** @@ -1968,7 +1965,7 @@ export interface ParseTemplateOptions { export function parseTemplate( template: string, templateUrl: string, options: ParseTemplateOptions = {}): {errors?: ParseError[], nodes: t.Node[], styleUrls: string[], styles: string[]} { - const {interpolationConfig, preserveWhitespaces, i18nLegacyMessageIdFormat} = options; + const {interpolationConfig, preserveWhitespaces, enableI18nLegacyMessageIdFormat} = options; const bindingParser = makeBindingParser(interpolationConfig); const htmlParser = new HtmlParser(); const parseResult = htmlParser.parse( @@ -1986,7 +1983,8 @@ export function parseTemplate( // extraction process (ng xi18n) relies on a raw content to generate // message ids const i18nMetaVisitor = new I18nMetaVisitor( - interpolationConfig, /* keepI18nAttrs */ !preserveWhitespaces, i18nLegacyMessageIdFormat); + interpolationConfig, /* keepI18nAttrs */ !preserveWhitespaces, + enableI18nLegacyMessageIdFormat); rootNodes = html.visitAll(i18nMetaVisitor, rootNodes); if (!preserveWhitespaces) { diff --git a/packages/compiler/test/i18n/digest_spec.ts b/packages/compiler/test/i18n/digest_spec.ts index 9cb788efac..e77e49735f 100644 --- a/packages/compiler/test/i18n/digest_spec.ts +++ b/packages/compiler/test/i18n/digest_spec.ts @@ -14,6 +14,7 @@ import {computeMsgId, digest, sha1} from '../../src/i18n/digest'; it('must return the ID if it\'s explicit', () => { expect(digest({ id: 'i', + legacyIds: [], nodes: [], placeholders: {}, placeholderToMessage: {},