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

fix: compatibility with react rc 2 #35108

Merged
merged 8 commits into from
Mar 15, 2022
Merged

fix: compatibility with react rc 2 #35108

merged 8 commits into from
Mar 15, 2022

Conversation

wardpeet
Copy link
Contributor

@wardpeet wardpeet commented Mar 11, 2022

Description

Fixes support for React 18 latest RC.

  • Renamed onCompleteAll to onAllReady
  • Add HAS_REACT_18 flag to webpack to import the correct react-dom functions
  • Fixed warnings in gatsby-plugin-image

Related Issues

Fixes #35095
Fixes #35083
Fixes #35094

[sc-47335]

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Mar 11, 2022
@wardpeet wardpeet added topic: core Relates to Gatsby's core (e.g. page loading, reporter, state machine) topic: frontend Relates to frontend issues (e.g. reach/router, gatsby-link, page-loading, asset-loading, navigation) and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Mar 11, 2022
@wardpeet wardpeet marked this pull request as draft March 11, 2022 14:35
imjoshin
imjoshin previously approved these changes Mar 11, 2022
Copy link
Contributor

@imjoshin imjoshin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, but I'd recommend getting another set of eyes for approval. 😄

One thought for potential future refactor: is there any way we could pull out the renderer/hydrator logic into a common factory-type class? This subject is a bit in-the-weeds-y and I could see more duplicated logic with new plugins in the future.

@wardpeet wardpeet marked this pull request as ready for review March 15, 2022 07:07
@wardpeet wardpeet changed the title fix/react18 rc support fix: compatibility with react rc 2 Mar 15, 2022
Copy link
Contributor

@LekoArts LekoArts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two smaller things.

Also, what's up with the empty file packages/gatsby/cache-dir/react-dom-switcher.js


root.render(Component)

return root
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm missing some context here, but why is with React 18 the reactRender returning something, with older version it's void?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because Root doesn't exist in React < 18 and we need the root so we only call createRoot once.

…er.tsx

Co-authored-by: Lennart <lekoarts@gmail.com>
LekoArts
LekoArts previously approved these changes Mar 15, 2022
@wardpeet wardpeet added bot: merge on green Gatsbot will merge these PRs automatically when all tests passes and removed bot: merge on green Gatsbot will merge these PRs automatically when all tests passes labels Mar 15, 2022
@wardpeet wardpeet merged commit 0c61265 into master Mar 15, 2022
@wardpeet wardpeet deleted the fix/react18-rc-support branch March 15, 2022 17:40
@mwskwong
Copy link

mwskwong commented Mar 16, 2022

This merge seems to have introduced a new issue.
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: core Relates to Gatsby's core (e.g. page loading, reporter, state machine) topic: frontend Relates to frontend issues (e.g. reach/router, gatsby-link, page-loading, asset-loading, navigation)
Projects
None yet
4 participants