From a5c57c7484f1dc3afab4ece4e969a4a7308cdeca Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 24 Apr 2024 21:17:37 +0200 Subject: [PATCH] fix(core): resolve error for multiple component instances that use fallback content (#55478) 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 --- .../src/template/pipeline/src/ingest.ts | 4 +- .../src/render3/instructions/projection.ts | 30 ++++--- packages/core/test/acceptance/content_spec.ts | 90 +++++++++++++++++++ .../platform-server/test/hydration_spec.ts | 6 +- 4 files changed, 116 insertions(+), 14 deletions(-) diff --git a/packages/compiler/src/template/pipeline/src/ingest.ts b/packages/compiler/src/template/pipeline/src/ingest.ts index 156bad117cd..e5a3b54b101 100644 --- 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 index 95ad07873e3..56176c57cfb 100644 --- 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, - 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 index 1b7f00d6700..573451ecf4f 100644 --- 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: ``, + standalone: true, + imports: [Fallback], + }) + class Projection { + } + + @Component({ + standalone: true, + imports: [Projection], + template: `Hello`, + }) + class App { + } + + const fixture = TestBed.createComponent(App); + expect(creationCount).toBe(0); + expect(getElementHtml(fixture.nativeElement)).toContain(`Hello`); + }); + + 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: `Fallback`, + standalone: true, + }) + class Projection { + } + + @Component({ + standalone: true, + imports: [Projection], + template: ` + Content + + ` + }) + class App { + } + + const fixture = TestBed.createComponent(App); + expect(getElementHtml(fixture.nativeElement)) + .toContain('ContentFallback'); + }); + + 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: `Fallback`, + standalone: true, + }) + class Projection { + } + + @Component({ + standalone: true, + imports: [Projection], + template: ` + + Content + ` + }) + class App { + } + + const fixture = TestBed.createComponent(App); + expect(getElementHtml(fixture.nativeElement)) + .toContain('FallbackContent'); + }); }); }); diff --git a/packages/platform-server/test/hydration_spec.ts b/packages/platform-server/test/hydration_spec.ts index c4dbc39837f..b1dc17a383e 100644 --- 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 override
'); + expect(content).toContain('Header slot:
Header override
'); expect(content).toContain('Main slot:
Main fallback
'); - expect(content).toContain('Footer slot:

Footer override 321

'); + expect(content).toContain( + 'Footer slot: ', + ); expect(content).toContain('Wildcard fallback'); }); });