Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use suspense boundary vnode as parent for subtree re-render #408

Merged
merged 6 commits into from
Jan 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/friendly-numbers-hang.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'preact-render-to-string': patch
---

Fix issue where subtree re-render for Suspense boundaries caused a circular reference in the VNode's parent
4 changes: 2 additions & 2 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -480,13 +480,13 @@ function _renderToString(
return str;
} catch (error) {
if (!asyncMode && renderer && renderer.onError) {
let res = renderer.onError(error, vnode, (child) =>
let res = renderer.onError(error, vnode, (child, parent) =>
_renderToString(
child,
context,
isSvgMode,
selectValue,
vnode,
parent,
asyncMode,
renderer
)
Expand Down
4 changes: 2 additions & 2 deletions src/internal.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ComponentChildren, VNode } from 'preact';
import { ComponentChildren, ComponentChild, VNode } from 'preact';

interface Suspended {
id: string;
Expand All @@ -15,7 +15,7 @@ interface RendererErrorHandler {
this: RendererState,
error: any,
vnode: VNode<{ fallback: any }>,
renderChild: (child: ComponentChildren) => string
renderChild: (child: ComponentChildren, parent: ComponentChild) => string
): string | undefined;
}

Expand Down
2 changes: 1 addition & 1 deletion src/lib/chunked.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ function handleError(error, vnode, renderChild) {
const promise = error.then(
() => {
if (abortSignal && abortSignal.aborted) return;
const child = renderChild(vnode.props.children);
const child = renderChild(vnode.props.children, vnode);
if (child) this.onWrite(createSubtree(id, child));
},
// TODO: Abort and send hydration code snippet to client
Expand Down
83 changes: 83 additions & 0 deletions test/compat/render-chunked.test.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { h } from 'preact';
import { expect } from 'chai';
import { Suspense } from 'preact/compat';
import { useId } from 'preact/hooks';
import { renderToChunks } from '../../src/lib/chunked';
import { createSubtree, createInitScript } from '../../src/lib/client';
import { createSuspender } from '../utils';
import { VNODE, PARENT } from '../../src/lib/constants';

describe('renderToChunks', () => {
it('should render non-suspended JSX in one go', async () => {
Expand Down Expand Up @@ -66,4 +68,85 @@ describe('renderToChunks', () => {
'</div>'
]);
});

it('should encounter no circular references when rendering a suspense boundary subtree', async () => {
const { Suspender, suspended } = createSuspender();

const visited = new Set();
let circular = false;

function CircularReferenceCheck() {
let root = this[VNODE];
while (root !== null && root[PARENT] !== null) {
if (visited.has(root)) {
// Can't throw an error here, _catchError handler will also loop infinitely
circular = true;
break;
}
visited.add(root);
root = root[PARENT];
}
return <p>it works</p>;
}

const result = [];
const promise = renderToChunks(
<div>
<Suspense fallback="loading...">
<Suspender>
<CircularReferenceCheck />
</Suspender>
</Suspense>
</div>,
{ onWrite: (s) => result.push(s) }
);

suspended.resolve();
await promise;

if (circular) {
throw new Error('CircularReference');
}

expect(result).to.deep.equal([
'<div><!--preact-island:16-->loading...<!--/preact-island:16--></div>',
'<div hidden>',
createInitScript(1),
createSubtree('16', '<p>it works</p>'),
'</div>'
]);
});

it('should support using useId hooks inside a suspense boundary', async () => {
const { Suspender, suspended } = createSuspender();
f0x52 marked this conversation as resolved.
Show resolved Hide resolved

function ComponentWithId() {
const id = useId();
return <p>id: {id}</p>;
}

const result = [];
const promise = renderToChunks(
<div>
<ComponentWithId />
<Suspense fallback="loading...">
<Suspender>
<ComponentWithId />
</Suspender>
</Suspense>
</div>,
{ onWrite: (s) => result.push(s) }
);

suspended.resolve();
await promise;

expect(result).to.deep.equal([
'<div><p>id: P0-0</p><!--preact-island:24-->loading...<!--/preact-island:24--></div>',
'<div hidden>',
createInitScript(1),
createSubtree('24', '<p>id: P0-0</p>'),
'</div>'
]);
});
});
4 changes: 2 additions & 2 deletions test/compat/stream-node.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,10 @@ describe('renderToPipeableStream', () => {
const result = await sink.promise;

expect(result).to.deep.equal([
'<div><!--preact-island:17-->loading...<!--/preact-island:17--></div>',
'<div><!--preact-island:33-->loading...<!--/preact-island:33--></div>',
'<div hidden>',
createInitScript(),
createSubtree('17', '<p>it works</p>'),
createSubtree('33', '<p>it works</p>'),
'</div>'
]);
});
Expand Down
4 changes: 2 additions & 2 deletions test/compat/stream.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,10 @@ describe('renderToReadableStream', () => {
const result = await sink.promise;

expect(result).to.deep.equal([
'<div><!--preact-island:24-->loading...<!--/preact-island:24--></div>',
'<div><!--preact-island:40-->loading...<!--/preact-island:40--></div>',
'<div hidden>',
createInitScript(),
createSubtree('24', '<p>it works</p>'),
createSubtree('40', '<p>it works</p>'),
'</div>'
]);
});
Expand Down
Loading