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

Allow developers to disable concurrent rendering (reactRoot: false) with React 18 #36419

Closed
wants to merge 1 commit into from

Conversation

ide
Copy link

@ide ide commented Apr 24, 2022

Currently any website that uses React 18 causes shouldUseReactRoot() to return true, which forces config.experimental.reactRoot to be true and enables concurrent rendering. There are several libraries--often those that buffer up CSS used during rendering and emit it before streaming the response to the client--that rely on React's rendering to be synchronous. While those libraries will eventually need to be updated to support streaming and Suspense in some cases, they work with React 18 otherwise.

This commit lets websites disable concurrent features by explicitly setting experimental.reactRoot = false in next.config.js. If experimental.reactRoot is unspecified, then the existing default behavior still applies: reactRoot is true when using React 18+ or an experimental version and false otherwise. Production builds use the same resolved config object so future changes to this behavior will apply to both development and production.

Tested in a sample website that relies on synchronous React rendering and uses React 18. When experimental.reactRoot is false, the site renders. When it's true or not defined, the site expectedly fails to render (as was the case before this commit). Ran next build to make sure that production builds honor the same logic.

Added an integration test. Run with node run-tests.js test/integration/react-18-serial/test/index.test.js.

Bug

  • Related issues linked using fixes #number This PR is the bug report
  • Integration tests added
  • Errors have helpful link attached, see contributing.md N/A

…/React 18

Currently any website that uses React 18 causes `shouldUseReactRoot()` to return true, which forces `config.experimental.reactRoot` to be true and enables concurrent rendering. There are several libraries--often those that buffer up CSS used during rendering and emit it before streaming the response to the client--that rely on React's rendering to be synchronous. While those libraries will eventually need to be updated to support streaming and Suspense in some cases, they work with React 18 otherwise.

This commit lets websites disable concurrent features by explicitly setting `experimental.reactRoot = false` in next.config.js. If `experimental.reactRoot` is unspecified, then the existing default behavior still applies: `reactRoot` is true when using React 18+ or an experimental version and false otherwise. Production builds use the same resolved config object so future changes to this behavior will apply to both development and production.

Tested in a sample website that relies on synchronous React rendering and uses React 18. When `experimental.reactRoot` is false, the site renders. When it's true or not defined, the site expectedly fails to render (as was the case before this commit). Ran `next build` to make sure that production builds honor the same logic.
@chentsulin
Copy link
Contributor

chentsulin commented May 10, 2022

#36552 did some changes to the logic, but still not address to this problem.

In my point of view, we have so many controls: config.experimental.reactRoot, shouldUseReactRoot, hasReactRoot, and process.env.__NEXT_REACT_ROOT, but neither one of them can actually disable reactRoot in React 18. This's confusing.

@chentsulin
Copy link
Contributor

@huozhi Do you have any thoughts/suggestions on this?

@huozhi
Copy link
Member

huozhi commented May 10, 2022

Thanks for the PR! I filed #36792 to let document getInitialProps to be buffer and return the complete html, which is aligned with the previous behavior. It should support the case like css extraction from html.

The reason that next.js opt-in streaming rendering by default and don't wanna let user to disable reactRoot manually is because features like Suspense or React.lazy in SSR requires to work under streaming rendering to work properly

@ide ide closed this May 10, 2022
@ide ide deleted the disable-react-root branch May 10, 2022 16:55
kodiakhq bot pushed a commit that referenced this pull request May 11, 2022
When getInitialProps is customized with react 18, since gIP requires to return `html` as doc property which could be used by  user-land customization, we do blocking-rendering there and passdown the `html` to document

Fixes #36675
Closes #36419
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants