From 8e96b450e23bfa5465b7127a51890ef78d591d37 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Tue, 3 Dec 2019 08:36:38 +0000 Subject: [PATCH] refactor(ivy): i18n - store legacy message ids separately to custom ids (#34135) This change will enable the Angular compiler to provide these legacy message ids by default, which will solve problems with ngcc not knowing whether to generate legacy ids or not. PR Close #34135 --- packages/localize/src/utils/src/constants.ts | 17 +++ packages/localize/src/utils/src/messages.ts | 42 ++++-- .../localize/src/utils/src/translations.ts | 10 +- .../localize/src/utils/test/messages_spec.ts | 140 +++++++++++++++--- 4 files changed, 178 insertions(+), 31 deletions(-) diff --git a/packages/localize/src/utils/src/constants.ts b/packages/localize/src/utils/src/constants.ts index af571a5afd..8817ad0cd0 100644 --- a/packages/localize/src/utils/src/constants.ts +++ b/packages/localize/src/utils/src/constants.ts @@ -42,3 +42,20 @@ export const MEANING_SEPARATOR = '|'; * ``` */ export const ID_SEPARATOR = '@@'; + +/** + * The marker used to separate legacy message ids from the rest of a metadata block. + * + * For example: + * + * ```ts + * $localize `:@@custom-id␟2df64767cd895a8fabe3e18b94b5b6b6f9e2e3f0: Welcome!`; + * ``` + * + * Note that this character is the "symbol for the unit separator" (␟) not the "unit separator + * character" itself, since that has no visual representation. See https://graphemica.com/%E2%90%9F. + * + * Here is some background for the original "unit separator character": + * https://stackoverflow.com/questions/8695118/whats-the-file-group-record-unit-separator-control-characters-and-its-usage + */ +export const LEGACY_ID_INDICATOR = '\u241F'; diff --git a/packages/localize/src/utils/src/messages.ts b/packages/localize/src/utils/src/messages.ts index 6664b4db67..a781266e10 100644 --- a/packages/localize/src/utils/src/messages.ts +++ b/packages/localize/src/utils/src/messages.ts @@ -6,7 +6,8 @@ * found in the LICENSE file at https://angular.io/license */ import {computeMsgId} from '@angular/compiler'; -import {BLOCK_MARKER, ID_SEPARATOR, MEANING_SEPARATOR} from './constants'; + +import {BLOCK_MARKER, ID_SEPARATOR, LEGACY_ID_INDICATOR, MEANING_SEPARATOR} from './constants'; /** * Re-export this helper function so that users of `@angular/localize` don't need to actively import @@ -62,6 +63,17 @@ export interface ParsedMessage { * The key used to look up the appropriate translation target. */ messageId: MessageId; + /** + * Legacy message ids, if provided. + * + * In legacy message formats the message id can only be computed directly from the original + * template source. + * + * Since this information is not available in `$localize` calls, the legacy message ids may be + * attached by the compiler to the `$localize` metablock so it can be used if needed at the point + * of translation if the translations are encoded using the legacy message id. + */ + legacyIds: MessageId[]; /** * A mapping of placeholder names to substitution values. */ @@ -110,8 +122,11 @@ export function parseMessage( placeholderNames.push(placeholderName); cleanedMessageParts.push(messagePart); } + const messageId = metadata.id || computeMsgId(messageString, metadata.meaning || ''); + const legacyIds = metadata.legacyIds.filter(id => id !== messageId); return { - messageId: metadata.id || computeMsgId(messageString, metadata.meaning || ''), + messageId, + legacyIds, substitutions, messageString, meaning: metadata.meaning || '', @@ -125,25 +140,29 @@ export interface MessageMetadata { meaning: string|undefined; description: string|undefined; id: string|undefined; + legacyIds: string[]; } /** * Parse the given message part (`cooked` + `raw`) to extract the message metadata from the text. * * If the message part has a metadata block this function will extract the `meaning`, - * `description` and `id` (if provided) from the block. These metadata properties are serialized in - * the string delimited by `|` and `@@` respectively. + * `description`, `customId` and `legacyId` (if provided) from the block. These metadata properties + * are serialized in the string delimited by `|`, `@@` and `␟` respectively. + * + * (Note that `␟` is the `LEGACY_ID_INDICATOR` - see `constants.ts`.) * * For example: * * ```ts - * `:meaning|description@@id` - * `:meaning|@@id` + * `:meaning|description@@custom-id` + * `:meaning|@@custom-id` * `:meaning|description` - * `description@@id` + * `description@@custom-id` * `meaning|` * `description` - * `@@id` + * `@@custom-id` + * `:meaning|description@@custom-id␟legacy-id-1␟legacy-id-2` * ``` * * @param cooked The cooked version of the message part to parse. @@ -153,9 +172,10 @@ export interface MessageMetadata { export function parseMetadata(cooked: string, raw: string): MessageMetadata { const {text, block} = splitBlock(cooked, raw); if (block === undefined) { - return {text, meaning: undefined, description: undefined, id: undefined}; + return {text, meaning: undefined, description: undefined, id: undefined, legacyIds: []}; } else { - const [meaningAndDesc, id] = block.split(ID_SEPARATOR, 2); + const [meaningDescAndId, ...legacyIds] = block.split(LEGACY_ID_INDICATOR); + const [meaningAndDesc, id] = meaningDescAndId.split(ID_SEPARATOR, 2); let [meaning, description]: (string | undefined)[] = meaningAndDesc.split(MEANING_SEPARATOR, 2); if (description === undefined) { description = meaning; @@ -164,7 +184,7 @@ export function parseMetadata(cooked: string, raw: string): MessageMetadata { if (description === '') { description = undefined; } - return {text, meaning, description, id}; + return {text, meaning, description, id, legacyIds}; } } diff --git a/packages/localize/src/utils/src/translations.ts b/packages/localize/src/utils/src/translations.ts index dd64685c71..2ad83cd9a0 100644 --- a/packages/localize/src/utils/src/translations.ts +++ b/packages/localize/src/utils/src/translations.ts @@ -38,7 +38,8 @@ export function isMissingTranslationError(e: any): e is MissingTranslationError * `substitutions`) using the given `translations`. * * The tagged-string is parsed to extract its `messageId` which is used to find an appropriate - * `ParsedTranslation`. + * `ParsedTranslation`. If this doesn't match and there are legacy ids then try matching a + * translation using those. * * If one is found then it is used to translate the message into a new set of `messageParts` and * `substitutions`. @@ -52,7 +53,12 @@ export function translate( translations: Record, messageParts: TemplateStringsArray, substitutions: readonly any[]): [TemplateStringsArray, readonly any[]] { const message = parseMessage(messageParts, substitutions); - const translation = translations[message.messageId]; + // Look up the translation using the messageId, and then the legacyId if available. + let translation = translations[message.messageId]; + // If the messageId did not match a translation, try matching the legacy ids instead + for (let i = 0; i < message.legacyIds.length && translation === undefined; i++) { + translation = translations[message.legacyIds[i]]; + } if (translation === undefined) { throw new MissingTranslationError(message); } diff --git a/packages/localize/src/utils/test/messages_spec.ts b/packages/localize/src/utils/test/messages_spec.ts index 5c7be42d9c..49f6552b07 100644 --- a/packages/localize/src/utils/test/messages_spec.ts +++ b/packages/localize/src/utils/test/messages_spec.ts @@ -9,7 +9,7 @@ import {findEndOfBlock, makeTemplateObject, parseMessage, parseMetadata, splitBl describe('messages utils', () => { describe('parseMessage', () => { - it('should use the message-id parsed from the metadata if available', () => { + it('should use the custom id parsed from the metadata if available', () => { const message = parseMessage( makeTemplateObject( [':@@custom-message-id:a', ':one:b', ':two:c'], @@ -41,6 +41,32 @@ describe('messages utils', () => { expect(message3.messageId).not.toEqual(message1.messageId); }); + it('should capture legacy ids if available', () => { + const message1 = parseMessage( + makeTemplateObject( + [':␟legacy-1␟legacy-2␟legacy-3:a', ':one:b', ':two:c'], + [':␟legacy-1␟legacy-2␟legacy-3:a', ':one:b', ':two:c']), + [1, 2]); + expect(message1.messageId).toEqual('8865273085679272414'); + expect(message1.legacyIds).toEqual(['legacy-1', 'legacy-2', 'legacy-3']); + + const message2 = parseMessage( + makeTemplateObject( + [':@@custom-message-id␟legacy-message-id:a', ':one:b', ':two:c'], + [':@@custom-message-id␟legacy-message-id:a', ':one:b', ':two:c']), + [1, 2]); + expect(message2.messageId).toEqual('custom-message-id'); + expect(message2.legacyIds).toEqual(['legacy-message-id']); + + const message3 = parseMessage( + makeTemplateObject( + [':@@custom-message-id:a', ':one:b', ':two:c'], + [':@@custom-message-id:a', ':one:b', ':two:c']), + [1, 2]); + expect(message3.messageId).toEqual('custom-message-id'); + expect(message3.legacyIds).toEqual([]); + }); + it('should infer placeholder names if not given', () => { const parts1 = ['a', 'b', 'c']; const message1 = parseMessage(makeTemplateObject(parts1, parts1), [1, 2]); @@ -147,31 +173,108 @@ describe('messages utils', () => { describe('parseMetadata()', () => { it('should return just the text if there is no block', () => { - expect(parseMetadata('abc def', 'abc def')) - .toEqual({text: 'abc def', meaning: undefined, description: undefined, id: undefined}); + expect(parseMetadata('abc def', 'abc def')).toEqual({ + text: 'abc def', + meaning: undefined, + description: undefined, + id: undefined, + legacyIds: [] + }); }); it('should extract the metadata if provided', () => { - expect(parseMetadata(':description:abc def', ':description:abc def')) - .toEqual( - {text: 'abc def', description: 'description', meaning: undefined, id: undefined}); - expect(parseMetadata(':meaning|:abc def', ':meaning|:abc def')) - .toEqual({text: 'abc def', description: undefined, meaning: 'meaning', id: undefined}); - expect(parseMetadata(':@@message-id:abc def', ':@@message-id:abc def')) - .toEqual({text: 'abc def', description: undefined, meaning: undefined, id: 'message-id'}); + expect(parseMetadata(':description:abc def', ':description:abc def')).toEqual({ + text: 'abc def', + description: 'description', + meaning: undefined, + id: undefined, + legacyIds: [] + }); + expect(parseMetadata(':meaning|:abc def', ':meaning|:abc def')).toEqual({ + text: 'abc def', + description: undefined, + meaning: 'meaning', + id: undefined, + legacyIds: [] + }); + expect(parseMetadata(':@@message-id:abc def', ':@@message-id:abc def')).toEqual({ + text: 'abc def', + description: undefined, + meaning: undefined, + id: 'message-id', + legacyIds: [] + }); expect(parseMetadata(':meaning|description:abc def', ':meaning|description:abc def')) - .toEqual( - {text: 'abc def', description: 'description', meaning: 'meaning', id: undefined}); + .toEqual({ + text: 'abc def', + description: 'description', + meaning: 'meaning', + id: undefined, + legacyIds: [] + }); expect(parseMetadata(':description@@message-id:abc def', ':description@@message-id:abc def')) - .toEqual( - {text: 'abc def', description: 'description', meaning: undefined, id: 'message-id'}); + .toEqual({ + text: 'abc def', + description: 'description', + meaning: undefined, + id: 'message-id', + legacyIds: [] + }); expect(parseMetadata(':meaning|@@message-id:abc def', ':meaning|@@message-id:abc def')) - .toEqual({text: 'abc def', description: undefined, meaning: 'meaning', id: 'message-id'}); + .toEqual({ + text: 'abc def', + description: undefined, + meaning: 'meaning', + id: 'message-id', + legacyIds: [] + }); + expect(parseMetadata( + ':description@@message-id␟legacy-1␟legacy-2␟legacy-3:abc def', + ':description@@message-id␟legacy-1␟legacy-2␟legacy-3:abc def')) + .toEqual({ + text: 'abc def', + description: 'description', + meaning: undefined, + id: 'message-id', + legacyIds: ['legacy-1', 'legacy-2', 'legacy-3'] + }); + expect(parseMetadata( + ':meaning|@@message-id␟legacy-message-id:abc def', + ':meaning|@@message-id␟legacy-message-id:abc def')) + .toEqual({ + text: 'abc def', + description: undefined, + meaning: 'meaning', + id: 'message-id', + legacyIds: ['legacy-message-id'] + }); + expect(parseMetadata( + ':meaning|␟legacy-message-id:abc def', ':meaning|␟legacy-message-id:abc def')) + .toEqual({ + text: 'abc def', + description: undefined, + meaning: 'meaning', + id: undefined, + legacyIds: ['legacy-message-id'] + }); + + expect(parseMetadata(':␟legacy-message-id:abc def', ':␟legacy-message-id:abc def')).toEqual({ + text: 'abc def', + description: undefined, + meaning: undefined, + id: undefined, + legacyIds: ['legacy-message-id'] + }); }); it('should handle an empty block if there is one', () => { - expect(parseMetadata('::abc def', '::abc def')) - .toEqual({text: 'abc def', meaning: undefined, description: undefined, id: undefined}); + expect(parseMetadata('::abc def', '::abc def')).toEqual({ + text: 'abc def', + meaning: undefined, + description: undefined, + id: undefined, + legacyIds: [] + }); }); it('should handle escaped block markers', () => { @@ -180,7 +283,8 @@ describe('messages utils', () => { text: ':part of the message:abc def', meaning: undefined, description: undefined, - id: undefined + id: undefined, + legacyIds: [] }); }); });