From 034de06ab1a5927c570047dae5a485df4fbb381b Mon Sep 17 00:00:00 2001
From: Andrew Kushnir <akushnir@google.com>
Date: Mon, 25 Feb 2019 12:42:50 -0800
Subject: [PATCH] fix(ivy): avoid duplicate i18n consts to be present in
 generated output (#28967)

Prior to this change, the logic that outputs i18n consts (like `const MSG_XXX = goog.getMsg(...)`) didn't have a check whether a given const that represent a certain i18n message was already included into the generated output. This commit adds the logic to mark corresponding i18n contexts after translation was generated, to avoid duplicate consts in the output.

PR Close #28967
---
 .../compliance/r3_view_compiler_i18n_spec.ts  | 78 ++++++++++++++++---
 .../compiler/src/render3/view/i18n/context.ts |  1 +
 .../compiler/src/render3/view/template.ts     |  5 +-
 3 files changed, 72 insertions(+), 12 deletions(-)

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 99dbc96717..61539b698b 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
@@ -24,17 +24,22 @@ const angularFiles = setup({
 
 const htmlParser = new HtmlParser();
 
+// TODO: update translation extraction RegExp to support i18nLocalize calls once #28689 lands.
+const EXTRACT_GENERATED_TRANSLATIONS_REGEXP =
+    /const\s*(.*?)\s*=\s*goog\.getMsg\("(.*?)",?\s*(.*?)\)/g;
+
 const diff = (a: Set<string>, b: Set<string>): Set<string> =>
     new Set([...Array.from(a)].filter(x => !b.has(x)));
 
-const extract = (from: string, regex: any, transformFn: (match: any[]) => any) => {
-  const result = new Set<string>();
-  let item;
-  while ((item = regex.exec(from)) !== null) {
-    result.add(transformFn(item));
-  }
-  return result;
-};
+const extract =
+    (from: string, regex: any, transformFn: (match: any[], state?: Set<any>) => any) => {
+      const result = new Set<any>();
+      let item;
+      while ((item = regex.exec(from)) !== null) {
+        result.add(transformFn(item, result));
+      }
+      return result;
+    };
 
 // verify that we extracted all the necessary translations
 // and their ids match the ones extracted via 'ng xi18n'
@@ -73,8 +78,7 @@ const verifyTranslationIds =
 // placeholders object defined as goog.getMsg function argument
 const verifyPlaceholdersIntegrity = (output: string) => {
   const extactTranslations = (from: string) => {
-    const regex = /const\s*(.*?)\s*=\s*goog\.getMsg\("(.*?)",?\s*(.*?)\)/g;
-    return extract(from, regex, v => [v[2], v[3]]);
+    return extract(from, EXTRACT_GENERATED_TRANSLATIONS_REGEXP, v => [v[2], v[3]]);
   };
   const extractPlaceholdersFromBody = (body: string) => {
     const regex = /{\$(.*?)}/g;
@@ -95,6 +99,19 @@ const verifyPlaceholdersIntegrity = (output: string) => {
   return true;
 };
 
+const verifyUniqueConsts = (output: string) => {
+  extract(
+      output, EXTRACT_GENERATED_TRANSLATIONS_REGEXP,
+      (current: string[], state: Set<any>): string => {
+        const key = current[1];
+        if (state.has(key)) {
+          throw new Error(`Duplicate const ${key} found in generated output!`);
+        }
+        return key;
+      });
+  return true;
+};
+
 const getAppFilesWithTemplate = (template: string, args: any = {}) => ({
   app: {
     'spec.ts': `
@@ -134,6 +151,7 @@ const verify = (input: string, output: string, extra: any = {}): void => {
     const result = compile(files, angularFiles, opts(false));
     maybePrint(result.source, extra.verbose);
     expect(verifyPlaceholdersIntegrity(result.source)).toBe(true);
+    expect(verifyUniqueConsts(result.source)).toBe(true);
     expectEmit(result.source, output, 'Incorrect template');
   }
 
@@ -147,6 +165,7 @@ const verify = (input: string, output: string, extra: any = {}): void => {
     expect(verifyTranslationIds(input, result.source, extra.exceptions, interpolationConfig))
         .toBe(true);
     expect(verifyPlaceholdersIntegrity(result.source)).toBe(true);
+    expect(verifyUniqueConsts(result.source)).toBe(true);
     expectEmit(result.source, output, 'Incorrect template');
   }
 };
@@ -1627,6 +1646,45 @@ describe('i18n support in the view compiler', () => {
 
       verify(input, output);
     });
+
+    it('should not emit duplicate i18n consts for nested <ng-container>s', () => {
+      const input = `
+        <ng-template i18n>
+          Root content
+          <ng-container *ngIf="visible">
+            Nested content
+          </ng-container>
+        </ng-template>
+      `;
+
+      const output = String.raw `
+        const $MSG_EXTERNAL_8537814667662432133$$APP_SPEC_TS__0$ = goog.getMsg(" Root content {$startTagNgContainer} Nested content {$closeTagNgContainer}", {
+          "startTagNgContainer": "\uFFFD*1:1\uFFFD\uFFFD#1:1\uFFFD",
+          "closeTagNgContainer": "\uFFFD/#1:1\uFFFD\uFFFD/*1:1\uFFFD"
+        });
+        …
+      `;
+
+      verify(input, output);
+    });
+
+    it('should not emit duplicate i18n consts for elements with the same content', () => {
+      const input = `
+        <div i18n>Test</div>
+        <div i18n>Test</div>
+      `;
+
+      // TODO(FW-635): currently we generate unique consts for each i18n block even though it might
+      // contain the same content. This should be optimized by translation statements caching, that
+      // can be implemented in the future within FW-635.
+      const output = String.raw `
+        const $MSG_EXTERNAL_6563391987554512024$$APP_SPEC_TS_0$ = goog.getMsg("Test");
+        const $MSG_EXTERNAL_6563391987554512024$$APP_SPEC_TS_1$ = goog.getMsg("Test");
+        …
+      `;
+
+      verify(input, output);
+    });
   });
 
   describe('whitespace preserving mode', () => {
diff --git a/packages/compiler/src/render3/view/i18n/context.ts b/packages/compiler/src/render3/view/i18n/context.ts
index 87c50ac6f8..6be118d8d3 100644
--- a/packages/compiler/src/render3/view/i18n/context.ts
+++ b/packages/compiler/src/render3/view/i18n/context.ts
@@ -42,6 +42,7 @@ export class I18nContext {
   public readonly id: number;
   public bindings = new Set<o.Expression>();
   public placeholders = new Map<string, any[]>();
+  public isEmitted: boolean = false;
 
   private _registry !: any;
   private _unresolvedCtxCount: number = 0;
diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts
index fb0af532dc..e5048daf71 100644
--- a/packages/compiler/src/render3/view/template.ts
+++ b/packages/compiler/src/render3/view/template.ts
@@ -373,8 +373,9 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
   }
 
   i18nUpdateRef(context: I18nContext): void {
-    const {icus, meta, isRoot, isResolved} = context;
-    if (isRoot && isResolved && !isSingleI18nIcu(meta)) {
+    const {icus, meta, isRoot, isResolved, isEmitted} = context;
+    if (isRoot && isResolved && !isEmitted && !isSingleI18nIcu(meta)) {
+      context.isEmitted = true;
       const placeholders = context.getSerializedPlaceholders();
       let icuMapping: {[name: string]: o.Expression} = {};
       let params: {[name: string]: o.Expression} =