From 8c2e60a856b9b4873e1cd0aceb0190886991e60e Mon Sep 17 00:00:00 2001
From: Connor Clark <cjamcl@google.com>
Date: Thu, 22 Aug 2024 13:11:07 -0700
Subject: [PATCH] i18n: support reusing the same placeholder for ICU

---
 core/scripts/i18n/collect-strings.js          | 20 +++++++++-
 .../test/scripts/i18n/collect-strings-test.js | 37 ++++++++++++++-----
 2 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/core/scripts/i18n/collect-strings.js b/core/scripts/i18n/collect-strings.js
index 8c99b9940c98..3ef95a4240c5 100644
--- a/core/scripts/i18n/collect-strings.js
+++ b/core/scripts/i18n/collect-strings.js
@@ -319,6 +319,8 @@ function _processPlaceholderCustomFormattedIcu(icu) {
   icu.message = '';
   let idx = 0;
 
+  const rawNameCache = new Map();
+
   while (parts.length) {
     // Seperate out the match into parts.
     const [preambleText, rawName, format, formatType] = parts.splice(0, 4);
@@ -335,8 +337,22 @@ function _processPlaceholderCustomFormattedIcu(icu) {
       throw Error(`Unsupported custom-formatted ICU type var "${formatType}" in message "${icu.message}"`);
     }
 
+    let index;
+    const previousRawName = rawNameCache.get(rawName);
+    if (previousRawName) {
+      const [prevFormat, prevFormatType, prevIndex] = previousRawName;
+      if (prevFormat !== format || prevFormatType !== formatType) {
+        throw new Error(`must use same format and formatType for a given name. Invalid for: ${rawName}`);
+      }
+
+      index = prevIndex;
+    } else {
+      index = idx++;
+      rawNameCache.set(rawName, [format, formatType, index]);
+    }
+
     // Append ICU replacements if there are any.
-    const placeholderName = `CUSTOM_ICU_${idx++}`;
+    const placeholderName = `CUSTOM_ICU_${index}`;
     icu.message += `$${placeholderName}$`;
     let example;
 
@@ -393,7 +409,7 @@ function _processPlaceholderDirectIcu(icu, examples) {
       throw Error(`Example '${key}' provided, but has no corresponding ICU replacement in message "${icu.message}"`);
     }
     const eName = `ICU_${idx++}`;
-    tempMessage = tempMessage.replace(`{${key}}`, `$${eName}$`);
+    tempMessage = tempMessage.replaceAll(`{${key}}`, `$${eName}$`);
 
     icu.placeholders[eName] = {
       content: `{${key}}`,
diff --git a/core/test/scripts/i18n/collect-strings-test.js b/core/test/scripts/i18n/collect-strings-test.js
index 0b70d0ddd5a0..e1fae09baeaa 100644
--- a/core/test/scripts/i18n/collect-strings-test.js
+++ b/core/test/scripts/i18n/collect-strings-test.js
@@ -516,11 +516,25 @@ describe('Convert Message to Placeholder', () => {
     });
   });
 
+  it('converts custom-formatted ICU to placholders (repeated)', () => {
+    const message = 'the time is {timeInMs, number, milliseconds}. ' +
+      'I repeat, the time is {timeInMs, number, milliseconds}.';
+    const res = collect.convertMessageToCtc(message);
+    const expectation = 'the time is $CUSTOM_ICU_0$. I repeat, the time is $CUSTOM_ICU_0$.';
+    expect(res.message).toBe(expectation);
+    expect(res.placeholders).toEqual({
+      CUSTOM_ICU_0: {
+        content: '{timeInMs, number, milliseconds}',
+        example: '499',
+      },
+    });
+  });
+
   it('replaces within ICU plural', () => {
     const message = '{var, select, male{time: {timeInSec, number, seconds}} ' +
       'female{time: {timeInSec, number, seconds}} other{time: {timeInSec, number, seconds}}}';
     const expectation = '{var, select, male{time: $CUSTOM_ICU_0$} ' +
-      'female{time: $CUSTOM_ICU_1$} other{time: $CUSTOM_ICU_2$}}';
+      'female{time: $CUSTOM_ICU_0$} other{time: $CUSTOM_ICU_0$}}';
     const res = collect.convertMessageToCtc(message);
     expect(res.message).toEqual(expectation);
     expect(res.placeholders).toEqual({
@@ -528,14 +542,6 @@ describe('Convert Message to Placeholder', () => {
         content: '{timeInSec, number, seconds}',
         example: '2.4',
       },
-      CUSTOM_ICU_1: {
-        content: '{timeInSec, number, seconds}',
-        example: '2.4',
-      },
-      CUSTOM_ICU_2: {
-        content: '{timeInSec, number, seconds}',
-        example: '2.4',
-      },
     });
   });
 
@@ -558,6 +564,19 @@ describe('Convert Message to Placeholder', () => {
     });
   });
 
+  it('converts direct ICU with examples to placeholders (repeated)', () => {
+    const message = 'Hello {name}. Nice to meet you, {name}';
+    const res = collect.convertMessageToCtc(message, {name: 'Mary'});
+    const expectation = 'Hello $ICU_0$. Nice to meet you, $ICU_0$';
+    expect(res.message).toBe(expectation);
+    expect(res.placeholders).toEqual({
+      ICU_0: {
+        content: '{name}',
+        example: 'Mary',
+      },
+    });
+  });
+
   it('errors when example given without variable', () => {
     const message = 'Hello name.';
     expect(() => collect.convertMessageToCtc(message, {name: 'Mary'}))