fix(ivy): remove nested placeholders with i18n (#28595)

PR Close #28595

packages/core/src/render3/i18n.ts

@@ -614,6 +614,16 @@ export function i18nEnd(): void {
   i18nEndFirstPass(tView);
 }
 
+function findLastNode(node: TNode): TNode {
+  while (node.next) {
+    node = node.next;
+  }
+  if (node.child) {
+    return findLastNode(node.child);
+  }
+  return node;
+}
+
 /**
  * See `i18nEnd` above.
  */
@@ -629,12 +639,17 @@ function i18nEndFirstPass(tView: TView) {
 
   // The last placeholder that was added before `i18nEnd`
   const previousOrParentTNode = getPreviousOrParentTNode();
-  const visitedPlaceholders = readCreateOpCodes(rootIndex, tI18n.create, tI18n.icus, viewData);
+  const visitedNodes = readCreateOpCodes(rootIndex, tI18n.create, tI18n.icus, viewData);
 
-  // Remove deleted placeholders
-  // The last placeholder that was added before `i18nEnd` is `previousOrParentTNode`
-  for (let i = rootIndex + 1; i <= previousOrParentTNode.index - HEADER_OFFSET; i++) {
-    if (visitedPlaceholders.indexOf(i) === -1) {
+  // Find the last node that was added before `i18nEnd`
+  let lastCreatedNode = previousOrParentTNode;
+  if (lastCreatedNode.child) {
+    lastCreatedNode = findLastNode(lastCreatedNode.child);
+  }
+
+  // Remove deleted nodes
+  for (let i = rootIndex + 1; i <= lastCreatedNode.index - HEADER_OFFSET; i++) {
+    if (visitedNodes.indexOf(i) === -1) {
       removeNode(i, viewData);
     }
   }
@@ -646,7 +661,7 @@ function readCreateOpCodes(
   const renderer = getLView()[RENDERER];
   let currentTNode: TNode|null = null;
   let previousTNode: TNode|null = null;
-  const visitedPlaceholders: number[] = [];
+  const visitedNodes: number[] = [];
   for (let i = 0; i < createOpCodes.length; i++) {
     const opCode = createOpCodes[i];
     if (typeof opCode == 'string') {
@@ -655,6 +670,7 @@ function readCreateOpCodes(
       ngDevMode && ngDevMode.rendererCreateTextNode++;
       previousTNode = currentTNode;
       currentTNode = createNodeAtIndex(textNodeIndex, TNodeType.Element, textRNode, null, null);
+      visitedNodes.push(textNodeIndex);
       setIsParent(false);
     } else if (typeof opCode == 'number') {
       switch (opCode & I18nMutateOpCode.MASK_OPCODE) {
@@ -677,7 +693,7 @@ function readCreateOpCodes(
           break;
         case I18nMutateOpCode.Select:
           const nodeIndex = opCode >>> I18nMutateOpCode.SHIFT_REF;
-          visitedPlaceholders.push(nodeIndex);
+          visitedNodes.push(nodeIndex);
           previousTNode = currentTNode;
           currentTNode = getTNode(nodeIndex, viewData);
           if (currentTNode) {
@@ -715,6 +731,7 @@ function readCreateOpCodes(
           previousTNode = currentTNode;
           currentTNode =
               createNodeAtIndex(commentNodeIndex, TNodeType.IcuContainer, commentRNode, null, null);
+          visitedNodes.push(commentNodeIndex);
           attachPatchData(commentRNode, viewData);
           (currentTNode as TIcuContainerNode).activeCaseIndex = null;
           // We will add the case nodes later, during the update phase
@@ -731,6 +748,7 @@ function readCreateOpCodes(
           previousTNode = currentTNode;
           currentTNode = createNodeAtIndex(
               elementNodeIndex, TNodeType.Element, elementRNode, tagNameValue, null);
+          visitedNodes.push(elementNodeIndex);
           break;
         default:
           throw new Error(`Unable to determine the type of mutate operation for "${opCode}"`);
@@ -740,7 +758,7 @@ function readCreateOpCodes(
 
   setIsParent(false);
 
-  return visitedPlaceholders;
+  return visitedNodes;
 }
 
 function readUpdateOpCodes(

packages/core/test/i18n_integration_spec.ts

@@ -6,10 +6,10 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Component, Directive, TemplateRef, ViewContainerRef} from '@angular/core';
+import {Component, ContentChild, ContentChildren, Directive, QueryList, TemplateRef, ViewChild, ViewContainerRef} from '@angular/core';
 import {TestBed} from '@angular/core/testing';
 import {expect} from '@angular/platform-browser/testing/src/matchers';
-import {fixmeIvy, onlyInIvy, polyfillGoogGetMsg} from '@angular/private/testing';
+import {onlyInIvy, polyfillGoogGetMsg} from '@angular/private/testing';
 
 @Directive({
   selector: '[tplRef]',
@@ -59,7 +59,9 @@ const TRANSLATIONS: any = {
   '{VAR_SELECT, select, 10 {10 - {$startBoldText}ten{$closeBoldText}} 20 {20 - {$startItalicText}twenty{$closeItalicText}} other {{$startTagDiv}{$startUnderlinedText}other{$closeUnderlinedText}{$closeTagDiv}}}':
       '{VAR_SELECT, select, 10 {10 - {$startBoldText}dix{$closeBoldText}} 20 {20 - {$startItalicText}vingt{$closeItalicText}} other {{$startTagDiv}{$startUnderlinedText}autres{$closeUnderlinedText}{$closeTagDiv}}}',
   '{VAR_SELECT_2, select, 10 {ten - {VAR_SELECT, select, 1 {one} 2 {two} other {more than two}}} 20 {twenty - {VAR_SELECT_1, select, 1 {one} 2 {two} other {more than two}}} other {other}}':
-      '{VAR_SELECT_2, select, 10 {dix - {VAR_SELECT, select, 1 {un} 2 {deux} other {plus que deux}}} 20 {vingt - {VAR_SELECT_1, select, 1 {un} 2 {deux} other {plus que deux}}} other {autres}}'
+      '{VAR_SELECT_2, select, 10 {dix - {VAR_SELECT, select, 1 {un} 2 {deux} other {plus que deux}}} 20 {vingt - {VAR_SELECT_1, select, 1 {un} 2 {deux} other {plus que deux}}} other {autres}}',
+  '{$startTagNgTemplate}{$startTagDiv_1}{$startTagDiv}{$startTagSpan}Content{$closeTagSpan}{$closeTagDiv}{$closeTagDiv}{$closeTagNgTemplate}':
+      '{$startTagNgTemplate}Contenu{$closeTagNgTemplate}'
 };
 
 const getFixtureWithOverrides = (overrides = {}) => {
@@ -514,4 +516,69 @@ onlyInIvy('Ivy i18n logic').describe('i18n', function() {
       expect(element).toHaveText('vingt');
     });
   });
+
+  describe('queries', () => {
+    function toHtml(element: Element): string {
+      return element.innerHTML.replace(/\sng-reflect-\S*="[^"]*"/g, '')
+          .replace(/<!--bindings=\{(\W.*\W\s*)?\}-->/g, '');
+    }
+
+    it('detached nodes should still be part of query', () => {
+      const template = `
+          <div-query #q i18n>
+            <ng-template>
+              <div>
+                <div *ngIf="visible">
+                  <span text="1">Content</span>
+                </div>
+              </div>
+            </ng-template>
+          </div-query>
+        `;
+
+      @Directive({selector: '[text]', inputs: ['text'], exportAs: 'textDir'})
+      class TextDirective {
+        // TODO(issue/24571): remove '!'.
+        text !: string;
+        constructor() {}
+      }
+
+      @Component({selector: 'div-query', template: '<ng-container #vc></ng-container>'})
+      class DivQuery {
+        // TODO(issue/24571): remove '!'.
+        @ContentChild(TemplateRef) template !: TemplateRef<any>;
+
+        // TODO(issue/24571): remove '!'.
+        @ViewChild('vc', {read: ViewContainerRef})
+        vc !: ViewContainerRef;
+
+        // TODO(issue/24571): remove '!'.
+        @ContentChildren(TextDirective, {descendants: true})
+        query !: QueryList<TextDirective>;
+
+        create() { this.vc.createEmbeddedView(this.template); }
+
+        destroy() { this.vc.clear(); }
+      }
+
+      TestBed.configureTestingModule({declarations: [TextDirective, DivQuery]});
+      const fixture = getFixtureWithOverrides({template});
+      const q = fixture.debugElement.children[0].references.q;
+      expect(q.query.length).toEqual(0);
+
+      // Create embedded view
+      q.create();
+      fixture.detectChanges();
+      expect(q.query.length).toEqual(1);
+      expect(toHtml(fixture.nativeElement))
+          .toEqual(`<div-query><!--ng-container-->Contenu<!--container--></div-query>`);
+
+      // Disable ng-if
+      fixture.componentInstance.visible = false;
+      fixture.detectChanges();
+      expect(q.query.length).toEqual(0);
+      expect(toHtml(fixture.nativeElement))
+          .toEqual(`<div-query><!--ng-container-->Contenu<!--container--></div-query>`);
+    });
+  });
 });

packages/core/test/render3/i18n_spec.ts

@@ -1771,7 +1771,7 @@ describe('Runtime i18n', () => {
             selectors: [['parent']],
             directives: [Child],
             factory: () => new Parent(),
-            consts: 2,
+            consts: 3,
             vars: 0,
             template: (rf: RenderFlags, cmp: Parent) => {
               if (rf & RenderFlags.Create) {