fix(core): resolve error for multiple component instances that use fallback content (#55478)

crisbeto · AndrewKushnir · commit a5c57c7484f1 · 2024-04-25T09:04:01.000-07:00
Currently fallback content for `ng-content` gets declared and rendered out in one go. This breaks down if multiple instances of the same component are used where one doesn't render the fallback content while the other one does, because the `TNode` for the content has to be created during the first creation pass. These changes resolve the issue by always _declaring_ the template, but only rendering it if the slot is empty. Fixes #55466. PR Close #55478
diff --git a/packages/compiler/src/template/pipeline/src/ingest.ts b/packages/compiler/src/template/pipeline/src/ingest.ts
@@ -357,10 +357,11 @@ function ingestContent(unit: ViewCompilationUnit, content: t.Content): void {
     throw Error(`Unhandled i18n metadata type for element: ${content.i18n.constructor.name}`);
   }
 
-  const id = unit.job.allocateXrefId();
   let fallbackView: ViewCompilationUnit | null = null;
 
   // Don't capture default content that's only made up of empty text nodes and comments.
+  // Note that we process the default content before the projection in order to match the
+  // insertion order at runtime.
   if (
     content.children.some(
       (child) =>
@@ -372,6 +373,7 @@ function ingestContent(unit: ViewCompilationUnit, content: t.Content): void {
     ingestNodes(fallbackView, content.children);
   }
 
+  const id = unit.job.allocateXrefId();
   const op = ir.createProjectionOp(
     id,
     content.selector,
diff --git a/packages/core/src/render3/instructions/projection.ts b/packages/core/src/render3/instructions/projection.ts
@@ -7,7 +7,7 @@
  */
 import {findMatchingDehydratedView} from '../../hydration/views';
 import {newArray} from '../../util/array_utils';
-import {assertLContainer} from '../assert';
+import {assertLContainer, assertTNode} from '../assert';
 import {ComponentTemplate} from '../interfaces/definition';
 import {TAttributes, TElementNode, TNode, TNodeFlags, TNodeType} from '../interfaces/node';
 import {ProjectionSlots} from '../interfaces/projection';
@@ -132,6 +132,17 @@ export function ɵɵprojection(
     fallbackVars?: number): void {
   const lView = getLView();
   const tView = getTView();
+  const fallbackIndex = fallbackTemplateFn ? nodeIndex + 1 : null;
+
+  // Fallback content needs to be declared no matter whether the slot is empty since different
+  // instances of the component may or may not insert it. Also it needs to be declare *before*
+  // the projection node in order to work correctly with hydration.
+  if (fallbackIndex !== null) {
+    declareTemplate(
+        lView, tView, fallbackIndex, fallbackTemplateFn!, fallbackDecls!, fallbackVars!, null,
+        attrs);
+  }
+
   const tProjectionNode =
       getOrCreateTNode(tView, HEADER_OFFSET + nodeIndex, TNodeType.Projection, null, attrs || null);
 
@@ -149,9 +160,8 @@ export function ɵɵprojection(
   const componentHostNode = lView[DECLARATION_COMPONENT_VIEW][T_HOST] as TElementNode;
   const isEmpty = componentHostNode.projection![tProjectionNode.projection] === null;
 
-  if (isEmpty && fallbackTemplateFn) {
-    insertFallbackContent(
-        lView, tView, nodeIndex, fallbackTemplateFn, fallbackDecls!, fallbackVars!, attrs);
+  if (isEmpty && fallbackIndex !== null) {
+    insertFallbackContent(lView, tView, fallbackIndex);
   } else if (
       isNodeCreationMode &&
       (tProjectionNode.flags & TNodeFlags.isDetached) !== TNodeFlags.isDetached) {
@@ -161,13 +171,11 @@ export function ɵɵprojection(
 }
 
 /** Inserts the fallback content of a projection slot. Assumes there's no projected content. */
-function insertFallbackContent(
-    lView: LView, tView: TView, projectionIndex: number, templateFn: ComponentTemplate<unknown>,
-    decls: number, vars: number, attrs: TAttributes|undefined) {
-  const fallbackIndex = projectionIndex + 1;
-  const fallbackTNode =
-      declareTemplate(lView, tView, fallbackIndex, templateFn, decls, vars, null, attrs);
-  const fallbackLContainer = lView[HEADER_OFFSET + fallbackIndex];
+function insertFallbackContent(lView: LView, tView: TView, fallbackIndex: number) {
+  const adjustedIndex = HEADER_OFFSET + fallbackIndex;
+  const fallbackTNode = tView.data[adjustedIndex] as TNode;
+  const fallbackLContainer = lView[adjustedIndex];
+  ngDevMode && assertTNode(fallbackTNode);
   ngDevMode && assertLContainer(fallbackLContainer);
 
   const dehydratedView = findMatchingDehydratedView(fallbackLContainer, fallbackTNode.tView!.ssrId);
diff --git a/packages/core/test/acceptance/content_spec.ts b/packages/core/test/acceptance/content_spec.ts
@@ -1808,5 +1808,95 @@ describe('projection', () => {
       expect(content).toContain('Outer header override');
       expect(content).toContain('Outer footer fallback');
     });
+
+    it('should not instantiate directives inside the fallback content', () => {
+      let creationCount = 0;
+
+      @Component({
+        selector: 'fallback',
+        standalone: true,
+        template: 'Fallback',
+      })
+      class Fallback {
+        constructor() {
+          creationCount++;
+        }
+      }
+
+      @Component({
+        selector: 'projection',
+        template: `<ng-content><fallback/></ng-content>`,
+        standalone: true,
+        imports: [Fallback],
+      })
+      class Projection {
+      }
+
+      @Component({
+        standalone: true,
+        imports: [Projection],
+        template: `<projection>Hello</projection>`,
+      })
+      class App {
+      }
+
+      const fixture = TestBed.createComponent(App);
+      expect(creationCount).toBe(0);
+      expect(getElementHtml(fixture.nativeElement)).toContain(`<projection>Hello</projection>`);
+    });
+
+    it('should render the fallback content when an instance of a component that uses ' +
+           'fallback content is declared after one that does not',
+       () => {
+         @Component({
+           selector: 'projection',
+           template: `<ng-content>Fallback</ng-content>`,
+           standalone: true,
+         })
+         class Projection {
+         }
+
+         @Component({
+           standalone: true,
+           imports: [Projection],
+           template: `
+            <projection>Content</projection>
+            <projection/>
+          `
+         })
+         class App {
+         }
+
+         const fixture = TestBed.createComponent(App);
+         expect(getElementHtml(fixture.nativeElement))
+             .toContain('<projection>Content</projection><projection>Fallback</projection>');
+       });
+
+    it('should render the fallback content when an instance of a component that uses ' +
+           'fallback content is declared before one that does not',
+       () => {
+         @Component({
+           selector: 'projection',
+           template: `<ng-content>Fallback</ng-content>`,
+           standalone: true,
+         })
+         class Projection {
+         }
+
+         @Component({
+           standalone: true,
+           imports: [Projection],
+           template: `
+            <projection/>
+            <projection>Content</projection>
+          `
+         })
+         class App {
+         }
+
+         const fixture = TestBed.createComponent(App);
+         expect(getElementHtml(fixture.nativeElement))
+             .toContain('<projection>Fallback</projection><projection>Content</projection>');
+       });
   });
 });
diff --git a/packages/platform-server/test/hydration_spec.ts b/packages/platform-server/test/hydration_spec.ts
@@ -5987,9 +5987,11 @@ describe('platform-server hydration integration', () => {
         const content = clientRootNode.innerHTML;
         verifyAllNodesClaimedForHydration(clientRootNode);
         verifyClientAndSSRContentsMatch(ssrContents, clientRootNode);
-        expect(content).toContain('Header slot: <header>Header override</header>');
+        expect(content).toContain('Header slot: <!--container--><header>Header override</header>');
         expect(content).toContain('Main slot: <main>Main fallback</main>');
-        expect(content).toContain('Footer slot: <footer><h1>Footer override 321</h1></footer>');
+        expect(content).toContain(
+          'Footer slot: <!--container--><footer><h1>Footer override 321</h1></footer>',
+        );
         expect(content).toContain('Wildcard fallback');
       });
     });
