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

Support React 18 #2800

Closed
snowystinger opened this issue Feb 2, 2022 · 7 comments · Fixed by #3108
Closed

Support React 18 #2800

snowystinger opened this issue Feb 2, 2022 · 7 comments · Fixed by #3108
Assignees

Comments

@snowystinger
Copy link
Member

🙋 Feature Request

We'd like to formally support React 18.

🤔 Expected Behavior

One thing we'll need to figure out is, for any of our containers (ie dialogs), do we need an API to mount the new/old way so we don't accidentally poison the downstream tree which may not be ready yet.

See this comment in particular #2136 (comment)

😯 Current Behavior

💁 Possible Solution

See Context.

🔦 Context

Starting and exploration work has been done here:
#2526
https://github.com/adobe/react-spectrum/tree/react-18
#2136

@curran
Copy link

curran commented Apr 8, 2022

Strong agree.

FWIW I tried upgrading an app with an SSR component to React 18 and see the following error, which may (or may not) be related to this ticket. Note the third line of the stack trace coming from @react-aria/ssr/src/SSRProvider.tsx:

Error: Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:
1. You might have mismatching versions of React and the renderer (such as React DOM)
2. You might be breaking the Rules of Hooks
3. You might have more than one copy of React in the same app
See https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem.
    at resolveDispatcher (/home/curran/repos/vizhub/vizhub-v3/node_modules/react/cjs/react.development.js:1476:13)
    at Object.useContext (/home/curran/repos/vizhub/vizhub-v3/node_modules/react/cjs/react.development.js:1484:20)
    at $dac019021ac61f1f$export$9f8ac96af4b1b2ae (/home/curran/repos/vizhub/vizhub-v3/node_modules/@react-aria/ssr/dist/packages/@react-aria/ssr/src/SSRProvider.tsx:51:13)
    at renderWithHooks (/home/curran/repos/vizhub/vizhub-v3/vizhub-app/node_modules/react-dom/cjs/react-dom-server-legacy.node.development.js:5461:16)
    at renderIndeterminateComponent (/home/curran/repos/vizhub/vizhub-v3/vizhub-app/node_modules/react-dom/cjs/react-dom-server-legacy.node.development.js:5534:15)
    at renderElement (/home/curran/repos/vizhub/vizhub-v3/vizhub-app/node_modules/react-dom/cjs/react-dom-server-legacy.node.development.js:5749:7)
    at renderNodeDestructive (/home/curran/repos/vizhub/vizhub-v3/vizhub-app/node_modules/react-dom/cjs/react-dom-server-legacy.node.development.js:5888:11)
    at retryTask (/home/curran/repos/vizhub/vizhub-v3/vizhub-app/node_modules/react-dom/cjs/react-dom-server-legacy.node.development.js:6260:5)
    at performWork (/home/curran/repos/vizhub/vizhub-v3/vizhub-app/node_modules/react-dom/cjs/react-dom-server-legacy.node.development.js:6307:7)
    at /home/curran/repos/vizhub/vizhub-v3/vizhub-app/node_modules/react-dom/cjs/react-dom-server-legacy.node.development.js:6628:12

I wonder what would happen if 18 is whitelisted here https://github.com/adobe/react-spectrum/blob/main/packages/%40react-aria/ssr/package.json#L23

@snowystinger
Copy link
Member Author

I think your suggestion is because you suspect multiple copies of React?
A peer dependency should only give a warning when you do your install, it shouldn't affect the actual resolution of the version installed. You can verify this by looking in your node_modules or asking yarn/npm to list/why you have a dependency installed. It should only return one copy. Would you mind checking?

We'll expand that list of supported versions when we pick up this issue. Right now we know that some of our components have some bugs when used in a concurrent tree, so we can't in good faith expand it just yet.

@curran
Copy link

curran commented Apr 11, 2022

Sorry, it was a wonky node_modules issue. After deleting node_modules and package-lock and reinstalling, that error goes away. Nevermind!

Right now we know that some of our components have some bugs when used in a concurrent tree, so we can't in good faith expand it just yet.

That makes sense. Do you happen to know which components have bugs when used in a concurrent tree?

@snowystinger
Copy link
Member Author

Unfortunately, it's a bit more of a suspicion than knowing which ones have bugs. We have issues tracking our components that have problems in strict mode, we assume those are the ones that may have bugs in concurrent mode.
strict mode

Most of the work we need to do is outlined in #2136 as mentioned in the description of this issue. You can see there that pretty much everything rendered correctly when we tried out the alpha a few months ago, but there are still some other things to test out. If you try it in your app and find any issues, please report them :)

@curran
Copy link

curran commented Apr 11, 2022

Will do. Thanks!

@m5r
Copy link

m5r commented May 21, 2022

Nicely done @devongovett, thanks!
What's the best way for me to know when the next version will be released? I found this but was wondering if there is something better?

@devongovett
Copy link
Member

We will be testing this on Tuesday and hopefully releasing shortly after if all goes well.

The releases page is the best place, and I usually announce them on Twitter as well.

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 a pull request may close this issue.

4 participants