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.

Sign up for GitHub

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

Jump to bottom

useId stuck in infinite loop with streaming render #406

Open
1 task done
f0x52 opened this issue Jan 8, 2025 · 4 comments · May be fixed by #408
Open
1 task done

useId stuck in infinite loop with streaming render #406

f0x52 opened this issue Jan 8, 2025 · 4 comments · May be fixed by #408

Comments

@f0x52
Copy link

f0x52 commented Jan 8, 2025 •
edited
Loading

  • Check if updating to the latest Preact version resolves the issue

Describe the bug
I encountered an infinite loop in the useId hook, when trying to debug a different issue relating to useId/Suspense.

<div>
  <Suspense fallback="suspense-1">
    <ComponentOne/>
  </Suspense>
  <Suspense fallback="suspense-1">
    <ComponentTwo/>
  </Suspense>
</div>

With this component tree, where ComponentOne and ComponentTwo both load a lazy() component, as soon as rendering resumes on the first Suspense boundary, the useId hook inside ComponentOne loops infinitely.

To Reproduce
https://codesandbox.io/p/devbox/preact-useid-infinite-loop-4clmdc?workspaceId=ws_Hyb2mUcrQ6TVf2LBce8QH7

Launching the run/debug configuration 'Run program' will try to do a server-side streaming render with two Suspense boundaries, both of which have a component that logs the output of the useId hook, and then loads a lazy() wrapped component.

As debug output you will see:

render ComponentOne
  ComponentOne useId: P0-0
render ComponentTwo
  ComponentTwo useId: P0-1
resolved async component
render ComponentOne

After which the process freezes (and Codesandbox pops up a 'CPU usage 100%' in the top bar). If you pause the debug, you can see it's stuck in an infinite while loop in useHook, because root._parent is a circular reference.
https://github.com/preactjs/preact/blob/1226aae23826e0d703b520cb764749e04f571043/hooks/src/index.js#L431-L433

Expected behavior
Render finished, expected output

render ComponentOne
  ComponentOne useId: P0-0
render ComponentTwo
  ComponentTwo useId: P0-1
resolved async component
render ComponentOne
  ComponentOne useId: P0-0
resolved async component
render ComponentTwo
  ComponentTwo useId: P0-1
@JoviDeCroock
Copy link
Member

I am going to move this issue to the proper repository (preact-render-to-string)

@JoviDeCroock JoviDeCroock transferred this issue from preactjs/preact Jan 8, 2025
@f0x52 f0x52 mentioned this issue Jan 8, 2025
Open
@f0x52
Copy link
Author

f0x52 commented Jan 8, 2025

With renderToStringAsync this code does work

render ComponentOne
  ComponentOne useId: P0-0
render ComponentTwo
  ComponentTwo useId: P0-1
resolved async component
resolved async component

render output: <div>lazy-loadedlazy-loaded</div>

Interesting difference is that ComponentOne/Two don't get re-rendered at all, which would also circumvent getting stuck in the loop.

@f0x52
Copy link
Author

f0x52 commented Jan 8, 2025

Think I chased it down to the following code:
the renderer.onError callback renderChild argument sets the vnode that whose render the promise was caught in as the parent (5th arg to _renderToString)

preact-render-to-string/src/index.js

Lines 483 to 493 in 2da38a7

let res = renderer.onError(error, vnode, (child) =>
_renderToString(
child,
context,
isSvgMode,
selectValue,
vnode,
asyncMode,
renderer
)
);

The error handler walks up to find the Suspense boundary vnode

preact-render-to-string/src/lib/chunked.js

Lines 54 to 60 in 2da38a7

// walk up to the Suspense boundary
while ((vnode = vnode[PARENT])) {
let component = vnode[COMPONENT];
if (component && component[CHILD_DID_SUSPEND]) {
break;
}
}

And when the promise resolves, renderChild re-renders the Suspense boundary children (vnode.props.children)

preact-render-to-string/src/lib/chunked.js

Line 78 in 2da38a7

const child = renderChild(vnode.props.children);

Rendering them with the parent set to the vnode that originally threw, which is guaranteed to be one of the children of the vnode.props.children, so this will always cause infinite recursion when the useId hook attempts to walk up the tree.

I think the correct approach would be rendering vnode.props.children with that vnode (Suspense boundary) set as the parent, probably passing it to the renderChild callback as another argument

@f0x52 f0x52 linked a pull request Jan 8, 2025 that will close this issue
Draft
@f0x52
Copy link
Author

f0x52 commented Jan 8, 2025

The reason this wasn't an issue for #407 nor the Suspense/lazy example in the docs is because they throw while rendering the direct children of the Suspense element, which means the Suspense component is the vnode that caught the error, so it's actually the correct parent for re-rendering the Suspense's children.
renderToStringAsync awaits the thrown promise and continues rendering from the component that threw (no bubbling up to a Suspense boundary) so also doesn't encounter this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use suspense boundary vnode as parent for subtree re-render f0x52/preact-render-to-string
2 participants
@JoviDeCroock @f0x52