fix(ivy): i18n - use `MessageId` for matching translations (#32594)

As discussed in https://hackmd.io/33M5Wb-JT7-0fneA0JuHPA `SourceMessage`
strings are not sufficient for matching translations.

This commit updates `@angular/localize` to use `MessageId`s for translation
matching instead.

Also the run-time translation will now log a warning to the console if a
translation is missing.

BREAKING CHANGE:

Translations (loaded via the `loadTranslations()` function) must now use
`MessageId` for the translation key rather than the previous `SourceMessage`
string.

PR Close #32594

packages/compiler/src/i18n/index.ts

@@ -5,7 +5,7 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.io/license
  */
-
+export {computeMsgId} from './digest';
 export {Extractor, ExtractorHost} from './extractor';
 export {I18NHtmlParser} from './i18n_html_parser';
 export {MessageBundle} from './message_bundle';

packages/localize/BUILD.bazel

@@ -12,6 +12,7 @@ ts_library(
     ),
     module_name = "@angular/localize",
     deps = [
+        "//packages/compiler",
         "//packages/localize/src/localize",
         "@npm//@types/node",
     ],

packages/localize/src/translate.ts

@@ -15,15 +15,24 @@ import {ParsedTranslation, parseTranslation, translate as _translate} from './ut
  * Note that because the TRANSLATIONS are attached to a global object, they will be shared between
  * all applications that are running in a single page of the browser.
  */
-declare const $localize: LocalizeFn&{TRANSLATIONS: Record<string, ParsedTranslation>};
+declare const $localize: LocalizeFn&{TRANSLATIONS: Record<MessageId, ParsedTranslation>};
 
 /**
  * Load translations for `$localize`.
  *
- * The given `translations` are processed and added to a lookup based on their translation key.
+ * The given `translations` are processed and added to a lookup based on their `MessageId`.
- * A new translation will overwrite a previous translation if it has the same key.
+ * A new translation will overwrite a previous translation if it has the same `MessageId`.
+ *
+ * * If a message is generated by the Angular compiler from an `i18n` marker in a template, the
+ *   `MessageId` is passed through to the `$localize` call as a custom `MessageId`. The `MessageId`
+ *   will match what is extracted into translation files.
+ *
+ * * If the translation is from a call to `$localize` in application code, and no custom `MessageId`
+ *   is provided, then the `MessageId` can be generated by passing the tagged string message-parts
+ *   to the `parseMessage()` function (not currently public API).
  *
  * @publicApi
+ *
  */
 export function loadTranslations(translations: Record<MessageId, TargetMessage>) {
   // Ensure the translate function exists
@@ -54,5 +63,10 @@ export function clearTranslations() {
  */
 export function translate(messageParts: TemplateStringsArray, substitutions: readonly any[]):
     [TemplateStringsArray, readonly any[]] {
+  try {
     return _translate($localize.TRANSLATIONS, messageParts, substitutions);
+  } catch (e) {
+    console.warn(e.message);
+    return [messageParts, substitutions];
+  }
 }

packages/localize/src/utils/constants.ts

@@ -19,3 +19,26 @@
  * ```
  */
 export const BLOCK_MARKER = ':';
+
+/**
+ * The marker used to separate a message's "meaning" from its "description" in a metadata block.
+ *
+ * For example:
+ *
+ * ```ts
+ * $localize `:correct|Indicates that the user got the answer correct: Right!`;
+ * $localize `:movement|Button label for moving to the right: Right!`;
+ * ```
+ */
+export const MEANING_SEPARATOR = '|';
+
+/**
+ * The marker used to separate a message's custom "id" from its "description" in a metadata block.
+ *
+ * For example:
+ *
+ * ```ts
+ * $localize `:A welcome message on the home page@@myApp-homepage-welcome: Welcome!`;
+ * ```
+ */
+export const ID_SEPARATOR = '@@';

packages/localize/src/utils/messages.ts

@@ -5,7 +5,14 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.io/license
  */
-import {BLOCK_MARKER} from './constants';
+import {computeMsgId} from '@angular/compiler';
+import {BLOCK_MARKER, ID_SEPARATOR, MEANING_SEPARATOR} from './constants';
+
+/**
+ * Re-export this helper function so that users of `@angular/localize` don't need to actively import
+ * from `@angular/compiler`.
+ */
+export {computeMsgId} from '@angular/compiler';
 
 /**
  * A string containing a translation source message.
@@ -58,6 +65,10 @@ export interface ParsedMessage {
    * A mapping of placeholder names to substitution values.
    */
   substitutions: Record<string, any>;
+  /**
+   * A human readable rendering of the message
+   */
+  messageString: string;
 }
 
 /**
@@ -66,29 +77,112 @@ export interface ParsedMessage {
  * See `ParsedMessage` for an example.
  */
 export function parseMessage(
-    messageParts: TemplateStringsArray, expressions: readonly any[]): ParsedMessage {
+    messageParts: TemplateStringsArray, expressions?: readonly any[]): ParsedMessage {
-  const replacements: {[placeholderName: string]: any} = {};
+  const substitutions: {[placeholderName: string]: any} = {};
-  let messageId = messageParts[0];
+  const metadata = parseMetadata(messageParts[0], messageParts.raw[0]);
+  let messageString = metadata.text;
   for (let i = 1; i < messageParts.length; i++) {
-    const messagePart = messageParts[i];
+    const {text: messagePart, block: placeholderName = `ph_${i}`} =
-    const expression = expressions[i - 1];
+        splitBlock(messageParts[i], messageParts.raw[i]);
-    // There is a problem with synthesizing template literals in TS.
+    messageString += `{$${placeholderName}}${messagePart}`;
-    // It is not possible to provide raw values for the `messageParts` and TS is not able to compute
+    if (expressions !== undefined) {
-    // them since this requires access to the string in its original (non-existent) source code.
+      substitutions[placeholderName] = expressions[i - 1];
-    // Therefore we fall back on the non-raw version if the raw string is empty.
+    }
-    // This should be OK because synthesized nodes only come from the template compiler and they
+  }
-    // will always contain placeholder name information.
+  return {
-    // So there will be no escaped placeholder marker character (`:`) directly after a substitution.
+    messageId: metadata.id || computeMsgId(messageString, metadata.meaning || ''),
-    if ((messageParts.raw[i] || messagePart).charAt(0) === BLOCK_MARKER) {
+    substitutions,
-      const endOfPlaceholderName = messagePart.indexOf(BLOCK_MARKER, 1);
+    messageString,
-      const placeholderName = messagePart.substring(1, endOfPlaceholderName);
+  };
-      messageId += `{$${placeholderName}}${messagePart.substring(endOfPlaceholderName + 1)}`;
+}
-      replacements[placeholderName] = expression;
+
+export interface MessageMetadata {
+  text: string;
+  meaning: string|undefined;
+  description: string|undefined;
+  id: string|undefined;
+}
+
+/**
+ * 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.
+ *
+ * For example:
+ *
+ * ```ts
+ * `:meaning|description@@id`
+ * `:meaning|@@id`
+ * `:meaning|description`
+ * `description@@id`
+ * `meaning|`
+ * `description`
+ * `@@id`
+ * ```
+ *
+ * @param cooked The cooked version of the message part to parse.
+ * @param raw The raw version of the message part to parse.
+ * @returns A object containing any metadata that was parsed from the message part.
+ */
+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};
   } else {
-      const placeholderName = `ph_${i}`;
+    const [meaningAndDesc, id] = block.split(ID_SEPARATOR, 2);
-      messageId += `{$${placeholderName}}${messagePart}`;
+    let [meaning, description]: (string | undefined)[] = meaningAndDesc.split(MEANING_SEPARATOR, 2);
-      replacements[placeholderName] = expression;
+    if (description === undefined) {
+      description = meaning;
+      meaning = undefined;
+    }
+    if (description === '') {
+      description = undefined;
+    }
+    return {text, meaning, description, id};
   }
 }
-  return {messageId: messageId, substitutions: replacements};
+
+/**
+ * Split a message part (`cooked` + `raw`) into an optional delimited "block" off the front and the
+ * rest of the text of the message part.
+ *
+ * Blocks appear at the start of message parts. They are delimited by a colon `:` character at the
+ * start and end of the block.
+ *
+ * If the block is in the first message part then it will be metadata about the whole message:
+ * meaning, description, id.  Otherwise it will be metadata about the immediately preceding
+ * substitution: placeholder name.
+ *
+ * Since blocks are optional, it is possible that the content of a message block actually starts
+ * with a block marker. In this case the marker must be escaped `\:`.
+ *
+ * @param cooked The cooked version of the message part to parse.
+ * @param raw The raw version of the message part to parse.
+ * @returns An object containing the `text` of the message part and the text of the `block`, if it
+ * exists.
+ */
+export function splitBlock(cooked: string, raw: string): {text: string, block?: string} {
+  // Synthesizing AST nodes that represent template literals using the TypeScript API is problematic
+  // because it doesn't allow for the raw value of messageParts to be programmatically set.
+  // The result is that synthesized AST nodes have empty `raw` values.
+
+  // Normally we rely upon checking the `raw` value to check whether the `BLOCK_MARKER` was escaped
+  // in the original source. If the `raw` value is missing then we cannot do this.
+  // In such a case we fall back on the `cooked` version and assume that the `BLOCK_MARKER` was not
+  // escaped.
+
+  // This should be OK because synthesized nodes only come from the Angular template compiler, which
+  // always provides full id and placeholder name information so it will never escape `BLOCK_MARKER`
+  // characters.
+  if ((raw || cooked).charAt(0) !== BLOCK_MARKER) {
+    return {text: cooked};
+  } else {
+    const endOfBlock = cooked.indexOf(BLOCK_MARKER, 1);
+    return {
+      block: cooked.substring(1, endOfBlock),
+      text: cooked.substring(endOfBlock + 1),
+    };
+  }
 }

packages/localize/src/utils/translations.ts

@@ -33,7 +33,7 @@ export type ParsedTranslations = Record<MessageId, ParsedTranslation>;
  * `substitutions`.
  * The translation may reorder (or remove) substitutions as appropriate.
  *
- * If no translation matches then the original `messageParts` and `substitutions` are returned
+ * If no translation matches then an error is thrown.
  */
 export function translate(
     translations: Record<string, ParsedTranslation>, messageParts: TemplateStringsArray,
@@ -46,7 +46,8 @@ export function translate(
       translation.placeholderNames.map(placeholder => message.substitutions[placeholder])
     ];
   } else {
-    return [messageParts, substitutions];
+    throw new Error(
+        `No translation found for "${message.messageId}" ("${message.messageString}").`);
   }
 }
 

packages/localize/test/translate_spec.ts

@@ -5,19 +5,22 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.io/license
  */
+// Ensure that `$localize` is loaded to the global scope.
 import '@angular/localize/init';
+
 import {clearTranslations, loadTranslations} from '../src/translate';
+import {MessageId, TargetMessage, computeMsgId} from '../src/utils/messages';
 
 describe('$localize tag with translations', () => {
   describe('identities', () => {
     beforeEach(() => {
-      loadTranslations({
+      loadTranslations(computeIds({
         'abc': 'abc',
         'abc{$ph_1}': 'abc{$ph_1}',
         'abc{$ph_1}def': 'abc{$ph_1}def',
         'abc{$ph_1}def{$ph_2}': 'abc{$ph_1}def{$ph_2}',
         'Hello, {$ph_1}!': 'Hello, {$ph_1}!',
-      });
+      }));
     });
     afterEach(() => { clearTranslations(); });
 
@@ -33,13 +36,13 @@ describe('$localize tag with translations', () => {
 
   describe('to upper-case messageParts', () => {
     beforeEach(() => {
-      loadTranslations({
+      loadTranslations(computeIds({
         'abc': 'ABC',
         'abc{$ph_1}': 'ABC{$ph_1}',
         'abc{$ph_1}def': 'ABC{$ph_1}DEF',
         'abc{$ph_1}def{$ph_2}': 'ABC{$ph_1}DEF{$ph_2}',
         'Hello, {$ph_1}!': 'HELLO, {$ph_1}!',
-      });
+      }));
     });
     afterEach(() => { clearTranslations(); });
 
@@ -55,9 +58,9 @@ describe('$localize tag with translations', () => {
 
   describe('to reverse expressions', () => {
     beforeEach(() => {
-      loadTranslations({
+      loadTranslations(computeIds({
         'abc{$ph_1}def{$ph_2} - Hello, {$ph_3}!': 'abc{$ph_3}def{$ph_2} - Hello, {$ph_1}!',
-      });
+      }));
     });
     afterEach(() => { clearTranslations(); });
 
@@ -70,9 +73,9 @@ describe('$localize tag with translations', () => {
 
   describe('to remove expressions', () => {
     beforeEach(() => {
-      loadTranslations({
+      loadTranslations(computeIds({
         'abc{$ph_1}def{$ph_2} - Hello, {$ph_3}!': 'abc{$ph_1} - Hello, {$ph_3}!',
-      });
+      }));
     });
     afterEach(() => { clearTranslations(); });
 
@@ -83,3 +86,10 @@ describe('$localize tag with translations', () => {
     });
   });
 });
+
+function computeIds(translations: Record<MessageId, TargetMessage>):
+    Record<MessageId, TargetMessage> {
+  const processed: Record<MessageId, TargetMessage> = {};
+  Object.keys(translations).forEach(key => processed[computeMsgId(key, '')] = translations[key]);
+  return processed;
+}

packages/localize/test/utils/messages_spec.ts

@@ -5,32 +5,58 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.io/license
  */
-import {parseMessage} from '../../src/utils/messages';
+import {parseMessage, parseMetadata, splitBlock} from '../../src/utils/messages';
 import {makeTemplateObject} from '../../src/utils/translations';
 
 describe('messages utils', () => {
   describe('parseMessage', () => {
-    it('should compute the translation key', () => {
+    it('should use the message-id parsed from the metadata if available', () => {
+      const message = parseMessage(
+          makeTemplateObject(
+              [':@@custom-message-id:a', ':one:b', ':two:c'],
+              [':@@custom-message-id:a', ':one:b', ':two:c']),
+          [1, 2]);
+      expect(message.messageId).toEqual('custom-message-id');
+    });
+
+    it('should compute the translation key if no metadata', () => {
       const message = parseMessage(
           makeTemplateObject(['a', ':one:b', ':two:c'], ['a', ':one:b', ':two:c']), [1, 2]);
-      expect(message.messageId).toEqual('a{$one}b{$two}c');
+      expect(message.messageId).toEqual('8865273085679272414');
+    });
+
+    it('should compute the translation key if no id in the metadata', () => {
+      const message = parseMessage(
+          makeTemplateObject(
+              [':description:a', ':one:b', ':two:c'], [':description:a', ':one:b', ':two:c']),
+          [1, 2]);
+      expect(message.messageId).toEqual('8865273085679272414');
+    });
+
+    it('should compute a different id if the meaning changes', () => {
+      const message1 = parseMessage(makeTemplateObject(['abc'], ['abc']), []);
+      const message2 = parseMessage(makeTemplateObject([':meaning1|:abc'], [':meaning1|:abc']), []);
+      const message3 = parseMessage(makeTemplateObject([':meaning2|:abc'], [':meaning2|:abc']), []);
+      expect(message1.messageId).not.toEqual(message2.messageId);
+      expect(message2.messageId).not.toEqual(message3.messageId);
+      expect(message3.messageId).not.toEqual(message1.messageId);
     });
 
     it('should compute the translation key, inferring placeholder names if not given', () => {
       const message = parseMessage(makeTemplateObject(['a', 'b', 'c'], ['a', 'b', 'c']), [1, 2]);
-      expect(message.messageId).toEqual('a{$ph_1}b{$ph_2}c');
+      expect(message.messageId).toEqual('3269094494609300850');
     });
 
     it('should compute the translation key, ignoring escaped placeholder names', () => {
       const message = parseMessage(
           makeTemplateObject(['a', ':one:b', ':two:c'], ['a', '\\:one:b', '\\:two:c']), [1, 2]);
-      expect(message.messageId).toEqual('a{$ph_1}:one:b{$ph_2}:two:c');
+      expect(message.messageId).toEqual('529036009514785949');
     });
 
     it('should compute the translation key, handling empty raw values', () => {
       const message =
           parseMessage(makeTemplateObject(['a', ':one:b', ':two:c'], ['', '', '']), [1, 2]);
-      expect(message.messageId).toEqual('a{$one}b{$two}c');
+      expect(message.messageId).toEqual('8865273085679272414');
     });
 
     it('should build a map of named placeholders to expressions', () => {
@@ -43,5 +69,76 @@ describe('messages utils', () => {
       const message = parseMessage(makeTemplateObject(['a', 'b', 'c'], ['a', 'b', 'c']), [1, 2]);
       expect(message.substitutions).toEqual({ph_1: 1, ph_2: 2});
     });
+
+  });
+
+  describe('splitBlock()', () => {
+    it('should return just the text if there is no block',
+       () => { expect(splitBlock('abc def', 'abc def')).toEqual({text: 'abc def'}); });
+
+    it('should return just the text and block if there is one', () => {
+      expect(splitBlock(':block info:abc def', ':block info:abc def'))
+          .toEqual({text: 'abc def', block: 'block info'});
+    });
+
+    it('should handle an empty block if there is one', () => {
+      expect(splitBlock('::abc def', '::abc def')).toEqual({text: 'abc def', block: ''});
+    });
+
+    it('should handle escaped block markers', () => {
+      expect(splitBlock(':part of the message:abc def', '\\:part of the message:abc def')).toEqual({
+        text: ':part of the message:abc def'
+      });
+    });
+
+    it('should handle the empty raw part', () => {
+      expect(splitBlock(':block info:abc def', '')).toEqual({text: 'abc def', block: 'block info'});
+    });
+  });
+
+  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});
+    });
+
+    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(':meaning|description:abc def', ':meaning|description:abc def'))
+          .toEqual(
+              {text: 'abc def', description: 'description', meaning: 'meaning', id: undefined});
+      expect(parseMetadata(':description@@message-id:abc def', ':description@@message-id:abc def'))
+          .toEqual(
+              {text: 'abc def', description: 'description', meaning: undefined, id: 'message-id'});
+      expect(parseMetadata(':meaning|@@message-id:abc def', ':meaning|@@message-id:abc def'))
+          .toEqual({text: 'abc def', description: undefined, meaning: 'meaning', id: '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});
+    });
+
+    it('should handle escaped block markers', () => {
+      expect(parseMetadata(':part of the message:abc def', '\\:part of the message:abc def'))
+          .toEqual({
+            text: ':part of the message:abc def',
+            meaning: undefined,
+            description: undefined,
+            id: undefined
+          });
+    });
+
+    it('should handle the empty raw part', () => {
+      expect(parseMetadata(':description:abc def', ''))
+          .toEqual(
+              {text: 'abc def', meaning: undefined, description: 'description', id: undefined});
+    });
   });
 });

packages/localize/test/utils/translations_spec.ts

@@ -5,7 +5,7 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.io/license
  */
-import {TargetMessage} from '@angular/localize/src/utils/messages';
+import {TargetMessage, computeMsgId} from '../../src/utils/messages';
 import {ParsedTranslation, makeTemplateObject, parseTranslation, translate} from '../../src/utils/translations';
 
 describe('utils', () => {
@@ -151,7 +151,8 @@ describe('utils', () => {
         Record<string, ParsedTranslation> {
       const parsedTranslations: Record<string, ParsedTranslation> = {};
       Object.keys(translations).forEach(key => {
-        parsedTranslations[key] = parseTranslation(translations[key]);
+
+        parsedTranslations[computeMsgId(key, '')] = parseTranslation(translations[key]);
       });
       return parsedTranslations;
     }

tools/public_api_guard/localize/localize.d.ts

@@ -1,3 +1,3 @@
 export declare function clearTranslations(): void;
 
-export declare function loadTranslations(translations: Record<TranslationKey, TargetMessage>): void;
+export declare function loadTranslations(translations: Record<MessageId, TargetMessage>): void;