Skip to content

Commit

Permalink
fix(core): resolve error for multiple component instances that use fa…
Browse files Browse the repository at this point in the history
…llback 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
  • Loading branch information
crisbeto authored and AndrewKushnir committed Apr 25, 2024
1 parent 5ad2f5f commit a5c57c7
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 14 deletions.
4 changes: 3 additions & 1 deletion packages/compiler/src/template/pipeline/src/ingest.ts
Expand Up @@ -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) =>
Expand All @@ -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,
Expand Down
30 changes: 19 additions & 11 deletions packages/core/src/render3/instructions/projection.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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);

Expand All @@ -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) {
Expand All @@ -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);
Expand Down
90 changes: 90 additions & 0 deletions packages/core/test/acceptance/content_spec.ts
Expand Up @@ -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>');
});
});
});
6 changes: 4 additions & 2 deletions packages/platform-server/test/hydration_spec.ts
Expand Up @@ -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');
});
});
Expand Down

0 comments on commit a5c57c7

Please sign in to comment.