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

Add support for React 18 server rendering #1409

Merged
merged 4 commits into from Dec 24, 2021

Conversation

kylemellander
Copy link
Contributor

@kylemellander kylemellander commented Dec 15, 2021

With React 18, the client-side rendering process has been updated. To prepare the way for supporting React 18's features, the new way of rendering needs to be adopted.

This change creates some helpers that support both methods of rendering and hydrating on the client to start the path forward to supporting React 18.

NOTE: Because there is no current support for testing against React versions, I am forgoing adding extra tests for this. If there is an approach to doing this, we can add that.


This change is Reviewable

in React 18, the API for rendering and hydrating elements is updating.  This updates the syntax to support both syntax so that users of both APIs can still work with the same interface.
@kylemellander
Copy link
Contributor Author

#1408

@justin808
Copy link
Member

@kylemellander Big thanks for trying this out!

reactwg/react-18#5

Please try to use React.version rather than just checking if the function exists to determine what API to use.

https://www.peterbe.com/plog/display-current-react-version

Copy link
Member

@justin808 justin808 left a comment

Choose a reason for hiding this comment

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

Please use React.version for conditional logic.

Copy link
Member

@justin808 justin808 left a comment

Choose a reason for hiding this comment

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

Please check if the @ts-expect-error messages are still needed.

@@ -150,7 +152,8 @@ function render(el: Element, railsContext: RailsContext): void {
}

// Hydrate if available and was server rendered
const shouldHydrate = !!ReactDOM.hydrate && !!domNode.innerHTML;
// @ts-expect-error potentially present if React 18 or greater
Copy link
Member

Choose a reason for hiding this comment

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

Please check if the @ts-expect-error messages are still needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, these are still needed. We could upgrade the types library to the react beta to get around having to do this.

Copy link
Member

Choose a reason for hiding this comment

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

Pros and cons of doing that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the pros are:

  1. Using an API that is up-to-date with the latest React.

The cons are a couple:

  1. It would be depending on beta code for our types
  2. It could potentially have removed deprecated types that are no longer used in React 18. This could be seen as a positive as well because it would alert us to changes in the API that we might be using.

My recommendation would be to ignore it for now until it gets out of beta and then upgrade the types.


export default function reactHydrate(domNode: Element, reactElement: ReactElement): void | Element | Component {
if (supportsReactCreateRoot) {
// @ts-expect-error potentially present if React 18 or greater
Copy link
Member

Choose a reason for hiding this comment

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

Please check if the @ts-expect-error messages are still needed.


export default function reactRender(domNode: Element, reactElement: ReactElement): void | Element | Component {
if (supportsReactCreateRoot) {
// @ts-expect-error potentially present if React 18 or greater
Copy link
Member

Choose a reason for hiding this comment

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

Please check if the @ts-expect-error messages are still needed.

@justin808
Copy link
Member

@Judahmeek please review and merge if you approve and CI Passes.

@kylemellander Thanks for the contribution! Please consider joining us on our Slack for React and Rails.

@justin808 justin808 merged commit 922e19c into shakacode:master Dec 24, 2021
@kylemellander kylemellander deleted the react-18-prep branch December 31, 2021 05:51
@justin808
Copy link
Member

We're getting this error now:

Warning: You are importing createRoot from “react-dom” which is not supported. You should instead import it from “react-dom/client”.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants