From 23e0d654718b9396f90509d229feb3111c60ace2 Mon Sep 17 00:00:00 2001
From: crisbeto <crisbeto@abv.bg>
Date: Sun, 7 Jul 2019 16:35:58 +0200
Subject: [PATCH] perf(ivy): add self-closing elementContainer instruction
 (#31444)

Adds a new `elementContainer` instruction that can be used to avoid two instruction (`elementContainerStart` and `elementContainerEnd`) for `ng-container` that has text-only content. This is particularly useful when we have `ng-container` inside i18n sections.

This PR resolves FW-1105.

PR Close #31444
---
 .../compliance/r3_compiler_compliance_spec.ts |  5 +-
 .../compliance/r3_view_compiler_i18n_spec.ts  | 73 ++++++++++++++++++-
 .../compiler/src/render3/r3_identifiers.ts    |  2 +
 .../compiler/src/render3/view/template.ts     | 20 +++--
 .../core/src/core_render3_private_export.ts   |  1 +
 packages/core/src/render3/index.ts            |  1 +
 .../render3/instructions/element_container.ts | 16 ++++
 packages/core/src/render3/jit/environment.ts  |  1 +
 packages/core/test/acceptance/i18n_spec.ts    | 32 +++++++-
 tools/public_api_guard/core/core.d.ts         |  2 +
 10 files changed, 135 insertions(+), 18 deletions(-)

diff --git a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts
index 8f35af395d..03781d8717 100644
--- a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts
+++ b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts
@@ -253,7 +253,7 @@ describe('compiler compliance', () => {
       expectEmit(result.source, template, 'Incorrect template');
     });
 
-    it('should generate elementContainerStart/End instructions for empty <ng-container>', () => {
+    it('should generate self-closing elementContainer instruction for empty <ng-container>', () => {
       const files = {
         app: {
           'spec.ts': `
@@ -276,8 +276,7 @@ describe('compiler compliance', () => {
           …
           template: function MyComponent_Template(rf, ctx) {
             if (rf & 1) {
-              i0.ɵɵelementContainerStart(0);
-              i0.ɵɵelementContainerEnd();
+              i0.ɵɵelementContainer(0);
             }
           }
         `;
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 c4d612240e..c8fb19cfb3 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
@@ -1977,9 +1977,8 @@ describe('i18n support in the view compiler', () => {
             $r3$.ɵɵelementStart(0, "div");
             $r3$.ɵɵi18nStart(1, $I18N_0$);
             $r3$.ɵɵtemplate(2, MyComponent_ng_template_2_Template, 2, 3, "ng-template");
-            $r3$.ɵɵelementContainerStart(3);
+            $r3$.ɵɵelementContainer(3);
             $r3$.ɵɵpipe(4, "uppercase");
-            $r3$.ɵɵelementContainerEnd();
             $r3$.ɵɵi18nEnd();
             $r3$.ɵɵelementEnd();
           }
@@ -2324,6 +2323,76 @@ describe('i18n support in the view compiler', () => {
 
       verify(input, output);
     });
+
+    it('should generate a self-closing container instruction for ng-container inside i18n', () => {
+      const input = `
+        <div i18n>
+          Hello <ng-container>there</ng-container>
+        </div>
+      `;
+
+      const output = String.raw `
+        var $I18N_0$;
+        if (ngI18nClosureMode) {
+          const $MSG_APP_SPEC_TS_1$ = goog.getMsg(" Hello {$startTagNgContainer}there{$closeTagNgContainer}", { "startTagNgContainer": "\uFFFD#2\uFFFD", "closeTagNgContainer": "\uFFFD/#2\uFFFD" });
+          $I18N_0$ = $MSG_APP_SPEC_TS_1$;
+        }
+        else {
+          $I18N_0$ = $r3$.ɵɵi18nLocalize(" Hello {$startTagNgContainer}there{$closeTagNgContainer}", { "startTagNgContainer": "\uFFFD#2\uFFFD", "closeTagNgContainer": "\uFFFD/#2\uFFFD" });
+        }
+        …
+        consts: 3,
+        vars: 0,
+        template: function MyComponent_Template(rf, ctx) {
+          if (rf & 1) {
+            $r3$.ɵɵelementStart(0, "div");
+            $r3$.ɵɵi18nStart(1, I18N_0);
+            $r3$.ɵɵelementContainer(2);
+            $r3$.ɵɵi18nEnd();
+            $r3$.ɵɵelementEnd();
+          }
+        }
+      `;
+
+      verify(input, output);
+    });
+
+    it('should not generate a self-closing container instruction for ng-container with non-text content inside i18n',
+       () => {
+         const input = `
+          <div i18n>
+            Hello <ng-container>there <strong>!</strong></ng-container>
+          </div>
+        `;
+
+         const output = String.raw `
+          var $I18N_0$;
+          if (ngI18nClosureMode) {
+            const $MSG_APP_SPEC_TS_1$ = goog.getMsg(" Hello {$startTagNgContainer}there {$startTagStrong}!{$closeTagStrong}{$closeTagNgContainer}", { "startTagNgContainer": "\uFFFD#2\uFFFD", "startTagStrong": "\uFFFD#3\uFFFD", "closeTagStrong": "\uFFFD/#3\uFFFD", "closeTagNgContainer": "\uFFFD/#2\uFFFD" });
+            $I18N_0$ = $MSG_APP_SPEC_TS_1$;
+          }
+          else {
+            $I18N_0$ = $r3$.ɵɵi18nLocalize(" Hello {$startTagNgContainer}there {$startTagStrong}!{$closeTagStrong}{$closeTagNgContainer}", { "startTagNgContainer": "\uFFFD#2\uFFFD", "startTagStrong": "\uFFFD#3\uFFFD", "closeTagStrong": "\uFFFD/#3\uFFFD", "closeTagNgContainer": "\uFFFD/#2\uFFFD" });
+          }
+          …
+          consts: 4,
+          vars: 0,
+          template: function MyComponent_Template(rf, ctx) {
+            if (rf & 1) {
+              $r3$.ɵɵelementStart(0, "div");
+              $r3$.ɵɵi18nStart(1, I18N_0);
+              $r3$.ɵɵelementContainerStart(2);
+              $r3$.ɵɵelement(3, "strong");
+              $r3$.ɵɵelementContainerEnd();
+              $r3$.ɵɵi18nEnd();
+              $r3$.ɵɵelementEnd();
+            }
+          }
+        `;
+
+         verify(input, output);
+       });
+
   });
 
   describe('whitespace preserving mode', () => {
diff --git a/packages/compiler/src/render3/r3_identifiers.ts b/packages/compiler/src/render3/r3_identifiers.ts
index 63e68f8d40..695141ea68 100644
--- a/packages/compiler/src/render3/r3_identifiers.ts
+++ b/packages/compiler/src/render3/r3_identifiers.ts
@@ -66,6 +66,8 @@ export class Identifiers {
   static elementContainerEnd:
       o.ExternalReference = {name: 'ɵɵelementContainerEnd', moduleName: CORE};
 
+  static elementContainer: o.ExternalReference = {name: 'ɵɵelementContainer', moduleName: CORE};
+
   static styling: o.ExternalReference = {name: 'ɵɵstyling', moduleName: CORE};
 
   static styleMap: o.ExternalReference = {name: 'ɵɵstyleMap', moduleName: CORE};
diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts
index e1c61c7451..abe513896e 100644
--- a/packages/compiler/src/render3/view/template.ts
+++ b/packages/compiler/src/render3/view/template.ts
@@ -613,23 +613,21 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
       this.i18n.appendElement(element.i18n !, elementIndex);
     }
 
-    const hasChildren = () => {
-      if (!isI18nRootElement && this.i18n) {
-        // we do not append text node instructions and ICUs inside i18n section,
-        // so we exclude them while calculating whether current element has children
-        return !hasTextChildrenOnly(element.children);
-      }
-      return element.children.length > 0;
-    };
+    // Note that we do not append text node instructions and ICUs inside i18n section,
+    // so we exclude them while calculating whether current element has children
+    const hasChildren = (!isI18nRootElement && this.i18n) ? !hasTextChildrenOnly(element.children) :
+                                                            element.children.length > 0;
 
-    const createSelfClosingInstruction = !stylingBuilder.hasBindings && !isNgContainer &&
-        element.outputs.length === 0 && i18nAttrs.length === 0 && !hasChildren();
+    const createSelfClosingInstruction = !stylingBuilder.hasBindings &&
+        element.outputs.length === 0 && i18nAttrs.length === 0 && !hasChildren;
 
     const createSelfClosingI18nInstruction = !createSelfClosingInstruction &&
         !stylingBuilder.hasBindings && hasTextChildrenOnly(element.children);
 
     if (createSelfClosingInstruction) {
-      this.creationInstruction(element.sourceSpan, R3.element, trimTrailingNulls(parameters));
+      this.creationInstruction(
+          element.sourceSpan, isNgContainer ? R3.elementContainer : R3.element,
+          trimTrailingNulls(parameters));
     } else {
       this.creationInstruction(
           element.sourceSpan, isNgContainer ? R3.elementContainerStart : R3.elementStart,
diff --git a/packages/core/src/core_render3_private_export.ts b/packages/core/src/core_render3_private_export.ts
index a96b280a85..217bd76f57 100644
--- a/packages/core/src/core_render3_private_export.ts
+++ b/packages/core/src/core_render3_private_export.ts
@@ -118,6 +118,7 @@ export {
   ɵɵallocHostVars,
   ɵɵelementContainerStart,
   ɵɵelementContainerEnd,
+  ɵɵelementContainer,
   ɵɵstyling,
   ɵɵstyleMap,
   ɵɵclassMap,
diff --git a/packages/core/src/render3/index.ts b/packages/core/src/render3/index.ts
index d264d0dab3..b6bc407d40 100644
--- a/packages/core/src/render3/index.ts
+++ b/packages/core/src/render3/index.ts
@@ -56,6 +56,7 @@ export {
   ɵɵdirectiveInject,
 
   ɵɵelement,
+  ɵɵelementContainer,
   ɵɵelementContainerEnd,
 
   ɵɵelementContainerStart,
diff --git a/packages/core/src/render3/instructions/element_container.ts b/packages/core/src/render3/instructions/element_container.ts
index 456051db21..d2e9af7486 100644
--- a/packages/core/src/render3/instructions/element_container.ts
+++ b/packages/core/src/render3/instructions/element_container.ts
@@ -100,3 +100,19 @@ export function ɵɵelementContainerEnd(): void {
 
   registerPostOrderHooks(tView, previousOrParentTNode);
 }
+
+/**
+ * Creates an empty logical container using {@link elementContainerStart}
+ * and {@link elementContainerEnd}
+ *
+ * @param index Index of the element in the LView array
+ * @param attrs Set of attributes to be used when matching directives.
+ * @param localRefs A set of local reference bindings on the element.
+ *
+ * @codeGenApi
+ */
+export function ɵɵelementContainer(
+    index: number, attrs?: TAttributes | null, localRefs?: string[] | null): void {
+  ɵɵelementContainerStart(index, attrs, localRefs);
+  ɵɵelementContainerEnd();
+}
diff --git a/packages/core/src/render3/jit/environment.ts b/packages/core/src/render3/jit/environment.ts
index c40e296f66..71f3a60f59 100644
--- a/packages/core/src/render3/jit/environment.ts
+++ b/packages/core/src/render3/jit/environment.ts
@@ -61,6 +61,7 @@ export const angularCoreEnv: {[name: string]: Function} =
        'ɵɵelement': r3.ɵɵelement,
        'ɵɵelementContainerStart': r3.ɵɵelementContainerStart,
        'ɵɵelementContainerEnd': r3.ɵɵelementContainerEnd,
+       'ɵɵelementContainer': r3.ɵɵelementContainer,
        'ɵɵpureFunction0': r3.ɵɵpureFunction0,
        'ɵɵpureFunction1': r3.ɵɵpureFunction1,
        'ɵɵpureFunction2': r3.ɵɵpureFunction2,
diff --git a/packages/core/test/acceptance/i18n_spec.ts b/packages/core/test/acceptance/i18n_spec.ts
index fabf81798d..2ba68ebafd 100644
--- a/packages/core/test/acceptance/i18n_spec.ts
+++ b/packages/core/test/acceptance/i18n_spec.ts
@@ -8,7 +8,7 @@
 
 import {registerLocaleData} from '@angular/common';
 import localeRo from '@angular/common/locales/ro';
-import {Component, ContentChild, ContentChildren, Directive, HostBinding, Input, LOCALE_ID, QueryList, TemplateRef, Type, ViewChild, ViewContainerRef, ɵi18nConfigureLocalize} from '@angular/core';
+import {Component, ContentChild, ContentChildren, Directive, HostBinding, Input, LOCALE_ID, QueryList, TemplateRef, Type, ViewChild, ViewContainerRef, ɵi18nConfigureLocalize, Pipe, PipeTransform} from '@angular/core';
 import {setDelayProjection} from '@angular/core/src/render3/instructions/projection';
 import {TestBed} from '@angular/core/testing';
 import {By} from '@angular/platform-browser';
@@ -18,7 +18,7 @@ import {onlyInIvy} from '@angular/private/testing';
 
 onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => {
   beforeEach(() => {
-    TestBed.configureTestingModule({declarations: [AppComp, DirectiveWithTplRef]});
+    TestBed.configureTestingModule({declarations: [AppComp, DirectiveWithTplRef, UppercasePipe]});
   });
 
   afterEach(() => { setDelayProjection(false); });
@@ -315,6 +315,29 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => {
       }
     });
 
+    it('should be able to act as child elements inside i18n block (text + pipes)', () => {
+      // Note: for some reason keeping this key inline causes clang to reformat the entire file
+      // in a very weird way. Keeping it separated like this seems to make it happy.
+      const key = '{$startTagNgTemplate}Hello {$interpolation}{$closeTagNgTemplate}' +
+          '{$startTagNgContainer}Bye {$interpolation}{$closeTagNgContainer}';
+
+      ɵi18nConfigureLocalize({
+        translations: {
+          [key]:
+              '{$startTagNgTemplate}Hej {$interpolation}{$closeTagNgTemplate}{$startTagNgContainer}Vi ses {$interpolation}{$closeTagNgContainer}'
+        }
+      });
+      const fixture = initWithTemplate(AppComp, `
+        <div i18n>
+          <ng-template tplRef>Hello {{name | uppercase}}</ng-template>
+          <ng-container>Bye {{name | uppercase}}</ng-container>
+        </div>
+      `);
+
+      const element = fixture.nativeElement.firstChild;
+      expect(element.textContent.replace(/\s+/g, ' ').trim()).toBe('Hej ANGULARVi ses ANGULAR');
+    });
+
     it('should be able to handle deep nested levels with templates', () => {
       ɵi18nConfigureLocalize({
         translations: {
@@ -1463,3 +1486,8 @@ class DirectiveWithTplRef {
   constructor(public vcRef: ViewContainerRef, public tplRef: TemplateRef<{}>) {}
   ngOnInit() { this.vcRef.createEmbeddedView(this.tplRef, {}); }
 }
+
+@Pipe({name: 'uppercase'})
+class UppercasePipe implements PipeTransform {
+  transform(value: string) { return value.toUpperCase(); }
+}
diff --git a/tools/public_api_guard/core/core.d.ts b/tools/public_api_guard/core/core.d.ts
index 12062bbcb0..eadd5601f7 100644
--- a/tools/public_api_guard/core/core.d.ts
+++ b/tools/public_api_guard/core/core.d.ts
@@ -853,6 +853,8 @@ export declare function ɵɵdisableBindings(): void;
 
 export declare function ɵɵelement(index: number, name: string, attrs?: TAttributes | null, localRefs?: string[] | null): void;
 
+export declare function ɵɵelementContainer(index: number, attrs?: TAttributes | null, localRefs?: string[] | null): void;
+
 export declare function ɵɵelementContainerEnd(): void;
 
 export declare function ɵɵelementContainerStart(index: number, attrs?: TAttributes | null, localRefs?: string[] | null): void;