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

Bug: Suspense | client component should have a queue #25964

Closed
xiel opened this issue Jan 5, 2023 · 6 comments · Fixed by #26232
Closed

Bug: Suspense | client component should have a queue #25964

xiel opened this issue Jan 5, 2023 · 6 comments · Fixed by #26232

Comments

@xiel
Copy link

xiel commented Jan 5, 2023

React version: 18.2.0
Next version: 13.1.1

Steps To Reproduce

  1. Suspend using a (fake) promise in a client component in app dir (next)
  2. Try to useState after the use() call
'use client';

import { use, useState } from 'react';

const testPromise = new Promise((resolve) => {
  setTimeout(() => {
    resolve('use test promise');
  });
});

export default function Page() {
  use(testPromise);
  useState(0);

  return <h1>Test</h1>;
}

Link to code example:
https://codesandbox.io/s/github/xiel/app-playground/tree/suspense-error/?from-embed=&file=/app/page.tsx

The current behavior

Suspending for a promise in a client component and using state/reducer after results in errors during hydration:

react-dom.development.js?9d87:94 Warning: An error occurred during hydration. The server HTML was replaced with client 

react-dom.development.js?9d87:94 Warning: You are accessing "digest" from the errorInfo object passed to onRecoverableError. 

on-recoverable-error.js?eb92:17 Uncaught Error: Should have a queue. This is likely a bug in React. Please file an issue.

The expected behavior

No error during hydration.

@xiel xiel added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Jan 5, 2023
@xiel xiel changed the title Bug: Suspense in client component should have a queue Bug: Suspense | client component should have a queue Jan 5, 2023
@govind15496
Copy link

Hello, can you explain what is happening ?

@eps1lon
Copy link
Collaborator

eps1lon commented Jan 7, 2023

@xiel Could you also open this against vercel/next.js. They probably know best how to create a minimal repro that just uses React code.

@xiel
Copy link
Author

xiel commented Jan 7, 2023

opened issue in next repo: vercel/next.js#44686

feel free to close this issue, if you think it is only relevant to next.js

@eps1lon eps1lon added Type: Bug and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels Jan 18, 2023
@preslavle
Copy link

Hi there, any update on when this issue is going to get fixed? We are trying to integrate use() in some of our client components, but got blocked due to this.

And a higher level question, how far we are from use() hook becoming more stable?

@sophiebits
Copy link
Collaborator

This happens because we end up calling the update version of useState rather than the mount version since React knows this is not a new Fiber. It seems like we do want to retain the past state of the Hooks we have already called, but we also need to accommodate the case that we are calling a Hook for the first time.

@acdlite WDYT? Right now updateMemo is resilient to receiving an empty workInProgressHook (and you have a test for that one!) but updateReducer is not. I saw this comment:

// use as a base. When we reach the end of the base list, we must switch to
// the dispatcher used for mounts.

Is updating the dispatcher mid-render still your thinking? Is there a good place to do that? (Once the next hook is called, we are already in an update-only path…) Or I guess we could make updateWorkInProgress return null in this case and have all the update hooks handle that possibility?

@acdlite
Copy link
Collaborator

acdlite commented Feb 24, 2023

Yeah the plan is to detect when it runs out of hooks from the previous attempt and switch to the mount dispatcher.

This bug only happens with use during initial mount that suspends because that's the only case where we reuse a partial list of hooks.

I'll submit a fix soon, have a few other things I need to do first. If someone else wants to give it a go in the meantime I'll happily review the PR, though it is a bit of a tricky one.

sophiebits added a commit to sophiebits/react that referenced this issue Feb 24, 2023
When resuming a suspended render, there may be more Hooks to be called that weren't seen the previous time through. Make sure to switch to the mount dispatcher when calling use() if the next Hook call should be treated as a mount.

Fixes facebook#25964.
sophiebits added a commit to sophiebits/react that referenced this issue Feb 24, 2023
When resuming a suspended render, there may be more Hooks to be called that weren't seen the previous time through. Make sure to switch to the mount dispatcher when calling use() if the next Hook call should be treated as a mount.

Fixes facebook#25964.
sophiebits added a commit to sophiebits/react that referenced this issue Feb 24, 2023
When resuming a suspended render, there may be more Hooks to be called that weren't seen the previous time through. Make sure to switch to the mount dispatcher when calling use() if the next Hook call should be treated as a mount.

Fixes facebook#25964.
sophiebits added a commit that referenced this issue Feb 24, 2023
When resuming a suspended render, there may be more Hooks to be called
that weren't seen the previous time through. Make sure to switch to the
mount dispatcher when calling use() if the next Hook call should be
treated as a mount.

Fixes #25964.
github-actions bot pushed a commit that referenced this issue Feb 24, 2023
When resuming a suspended render, there may be more Hooks to be called
that weren't seen the previous time through. Make sure to switch to the
mount dispatcher when calling use() if the next Hook call should be
treated as a mount.

Fixes #25964.

DiffTrain build for [a8f971b](a8f971b)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants