-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
feat: Add support for React 19 Canary #1294
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit b90d8cc:
|
@@ -30,7 +30,7 @@ jobs: | |||
fail-fast: false | |||
matrix: | |||
node: [14, 16, 18] | |||
react: [latest, canary, experimental] | |||
react: ['18.x', latest, canary, experimental] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added now so that we automatically continue testing React 18.x once React 19 is released.
@@ -73,6 +73,7 @@ export interface RenderOptions< | |||
*/ | |||
hydrate?: boolean | |||
/** | |||
* Only works if used with React 18. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also add this to our docs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -207,6 +207,14 @@ function render( | |||
wrapper, | |||
} = {}, | |||
) { | |||
if (legacyRoot && typeof ReactDOM.render !== 'function') { | |||
const error = new Error( | |||
'`legacyRoot: true` is not supported in this version of React. Please use React 18 instead.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this message could be more encouraging update, rather than encouraging tech debt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Do you have a suggestion for a better warning?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My thoughts:
legacyRoot
is only relevant for earlier versions of React- If you're testing with React 19, there's a high chance you are also USING React 19, then it'd be best to just remove it
- If you're testing with React 19 but somehow you are NOT using React 19 yourself (?!), you may need help with migration.
Given that, my take:
Warning: legacyRoot is no longer supported in React 19. If your app runs React 19 or later, you should remove this flag. If your app runs React 18 or earlier, and you need help with upgrading your app, visit https://reactjs.org/link/switch-to-createroot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense: #1301. The link is not that applicable since you wouldn't need to change any callsites of render
from RTL.
Closes #1270
We can release React 19 support as a SemVer MINOR. Even though
legacyRoot
is no longer supported, it's not really a RTL breaking change but a React breaking change that's handled in their SemVer MAJOR release. It would only be a RTL breaking change if React would be a direct dependency not a peer dependency as it is now.