-
-
Notifications
You must be signed in to change notification settings - Fork 633
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
Update React to v19 #1677
Update React to v19 #1677
Conversation
WalkthroughThe pull request introduces modifications to the Changes
Possibly related PRs
Suggested Reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
package.json (1)
18-19
: Consider adding React 19 migration guide to documentation.Since this is a major version upgrade, consider adding migration notes or updating the documentation to help users transition to React 19.
Would you like me to help create a migration guide that covers the breaking changes and required updates?
Also applies to: 42-43
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (2)
node_package/tests/jest.setup.js
(1 hunks)package.json
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: rspec-package-tests (newest)
- GitHub Check: build
- GitHub Check: rspec-package-tests (oldest)
🔇 Additional comments (3)
node_package/tests/jest.setup.js (2)
8-11
: LGTM! Error handling follows consistent pattern.The error handling for MessageChannel follows the same pattern as TextEncoder, which helps identify when jsdom adds native support.
19-34
: Verify MessageChannel mock compatibility with React 19.The MessageChannel mock implementation looks correct for basic functionality. However, since this is specifically for React 19 upgrade, we should verify it supports all required features.
Run the following to check for any MessageChannel-related test failures:
✅ Verification successful
MessageChannel mock implementation verified for React 19 compatibility
The current implementation provides the necessary functionality for React 19's scheduling needs, focusing on the core message passing capability required for the test environment. The mock follows community best practices and is sufficient for the codebase's usage patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for MessageChannel usage in tests and verify no failures # Check for any existing MessageChannel usage in tests rg -l "MessageChannel" "node_package/tests" # Run tests with extra logging for MessageChannel-related operations JEST_DEBUG=true yarn test --testPathPattern="node_package/tests" 2>&1 | rg -i "messagechannel"Length of output: 164
Script:
#!/bin/bash # Search for React-specific MessageChannel usage rg -l "scheduler|concurrent" "node_package" # Check for any React imports or usage patterns ast-grep --pattern 'import { $$$ } from "react"' # Look for any scheduling or effect-related test files fd -e test.js -e spec.js . node_package/testsLength of output: 496
Script:
#!/bin/bash # Check package.json for React version and dependencies cat node_package/package.json | jq '.dependencies,.devDependencies' # Look for any concurrent mode or experimental features usage rg -l "createRoot|concurrent|experimental|Suspense|useTransition" "node_package"Length of output: 343
package.json (1)
18-19
: Verify compatibility with React 19 breaking changes.React 19 introduces breaking changes. We should verify:
- Usage of deprecated features
- Changes in event handling
- Updates to concurrent mode APIs
Run these checks:
Also applies to: 42-43
✅ Verification successful
React 19 compatibility verification completed - no issues found
The codebase appears clean of deprecated React features and breaking changes. The only React-specific patterns found are
setState
calls in test and documentation files, which should be reviewed but are not critical for production code.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for potential React 19 breaking changes # Check for deprecated lifecycle methods ast-grep --pattern 'componentWillMount|componentWillReceiveProps|componentWillUpdate' # Check for event pooling usage (removed in React 17) rg -l "e.persist\(\)" # Check for findDOMNode usage (deprecated) rg -l "findDOMNode" # Check for string refs (deprecated) ast-grep --pattern 'ref: "$_"'Length of output: 164
Script:
#!/bin/bash # Description: Additional React patterns verification # Check React import patterns and component definitions ast-grep --pattern 'import * from "react"' # Check for legacy context types ast-grep --pattern 'childContextTypes|getChildContext' # Check for React.FC usage ast-grep --pattern 'React.FC' # Check for class components (might need migration) ast-grep --pattern 'class $_ extends React.Component' # Check for setState usage patterns rg "setState\(" -A 2 # Look for context usage rg "React.createContext|useContext" -A 2Length of output: 1053
package.json
Outdated
"@types/react": "^19.0.7", | ||
"@types/react-dom": "^19.0.3", |
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.
Update peer dependencies to match React 19.
While the React dependencies have been updated to v19, the peer dependencies still specify ">= 16". This should be updated to reflect the major version change.
Apply this diff:
"peerDependencies": {
"js-yaml": ">= 3.0.0",
- "react": ">= 16",
- "react-dom": ">= 16"
+ "react": ">= 19",
+ "react-dom": ">= 19"
},
Also applies to: 42-43
5215872
to
d7666db
Compare
"@types/react": "^18.3.18", | ||
"@types/react-dom": "^18.3.5", |
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.
Keep these versions while we need to support older React versions.
a6ee185
to
16dccd3
Compare
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.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 1
🧹 Nitpick comments (1)
spec/dummy/rescript.json (1)
12-18
: Consider documenting migration steps.Since these changes are part of a React 19 upgrade, it would be helpful to document the ReScript configuration changes in the migration guide or CHANGELOG.
Consider adding a section in the documentation about updating ReScript configuration for React 19 compatibility, including:
- Suffix change from
.bs.js
to.res.js
- Module system update from
es6
toesmodule
- Any potential breaking changes or required updates in ReScript components
🛑 Comments failed to post (1)
spec/dummy/rescript.json (1)
12-12: 💡 Codebase verification
Update imports to match new
.res.js
suffix configurationThe change to
.res.js
suffix will break existing imports that still use.bs.js
. Updates needed in:
spec/dummy/client/app/startup/HelloWorldReScript.jsx
spec/dummy/client/app/packs/rescript-components.js
- Update the comment in rescript-components.js that documents the
.bs.js
convention🔗 Analysis chain
Verify ReScript suffix convention change.
The change from
.bs.js
to.res.js
aligns with ReScript's newer naming conventions. However, let's verify this is the recommended approach with React 19.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are any remaining .bs.js files that need updating fd -e bs.js # Check ReScript configuration in other parts of the project fd -e json -x grep -l "bs.js" {}Length of output: 48
Script:
#!/bin/bash # Search for ReScript configuration files fd rescript.json # Search for any ReScript or React documentation files fd -e md -e mdx -x grep -l -i "rescript.*react" {} # Look for any ReScript-related patterns in JSON files fd -e json -x grep -l "rescript" {} # Search for any references to file extensions in configuration rg "\.(res|bs)\." -A 2 -B 2Length of output: 1403
16dccd3
to
e0fbe43
Compare
@@ -2,7 +2,7 @@ | |||
"compilerOptions": { | |||
"allowJs": true, | |||
"esModuleInterop": true, | |||
"jsx": "react", | |||
"jsx": "react-jsx", |
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.
This should only affect the tests, as we don't have JSX anywhere in our package itself. Otherwise this would force requiring React 17.
f31cf3e
to
b2b5444
Compare
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
spec/dummy/.gitignore (1)
34-37
: Consider grouping all ReScript-related patterns together.For better maintainability, consider grouping all ReScript-related ignore patterns in a single section:
#Ignore ReScript build files /.merlin /lib/bs/ /.bsb.lock -*.res.js +# ReScript generated files +*.res.js
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
spec/dummy/yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (11)
.prettierignore
(1 hunks)node_package/tests/jest.setup.js
(1 hunks)package.json
(2 hunks)spec/dummy/.gitignore
(1 hunks)spec/dummy/.prettierignore
(1 hunks)spec/dummy/babel.config.js
(1 hunks)spec/dummy/client/app/packs/rescript-components.js
(1 hunks)spec/dummy/client/app/startup/HelloWorldReScript.jsx
(1 hunks)spec/dummy/package.json
(3 hunks)spec/dummy/rescript.json
(1 hunks)tsconfig.json
(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- spec/dummy/.prettierignore
- .prettierignore
- spec/dummy/client/app/startup/HelloWorldReScript.jsx
- spec/dummy/client/app/packs/rescript-components.js
🚧 Files skipped from review as they are similar to previous changes (6)
- spec/dummy/babel.config.js
- tsconfig.json
- spec/dummy/rescript.json
- package.json
- node_package/tests/jest.setup.js
- spec/dummy/package.json
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: build-dummy-app-webpack-test-bundles (newest)
- GitHub Check: rspec-package-tests (newest)
- GitHub Check: build
- GitHub Check: rspec-package-tests (oldest)
- GitHub Check: build-dummy-app-webpack-test-bundles (oldest)
🔇 Additional comments (1)
spec/dummy/.gitignore (1)
37-37
: Verify completeness of ReScript ignore patterns.The change from
.bs.js
to.res.js
aligns with the ReScript updates. However, let's ensure all ReScript-generated files are properly ignored.✅ Verification successful
ReScript ignore patterns are correctly configured ✅
The
.res.js
pattern inspec/dummy/.gitignore
correctly covers the compiled output for the single ReScript component (HelloWorldReScript.res
) present in the codebase. No additional patterns are needed.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining .bs.js patterns and verify ReScript-related patterns # Check for any remaining .bs.js patterns in gitignore files echo "Checking for remaining .bs.js patterns in gitignore files:" fd -t f -e gitignore -x grep -l "bs.js" {} # Check for .res.js patterns in gitignore files echo "Checking for .res.js patterns in gitignore files:" fd -t f -e gitignore -x grep -l "res.js" {} # Look for any ReScript files in the codebase to ensure we're not missing any patterns echo "Checking for ReScript-related files:" fd -e res -e resiLength of output: 485
Summary
Update React to v19 (both in the main
devDependencies
and inspec/dummy
).Maybe make separate spec/dummy for earliest and latest supported React versions?
Pull Request checklist
Update documentationUpdate CHANGELOG fileOther Information
This change is
Summary by CodeRabbit
Summary by CodeRabbit
Release Notes
Chores
react-proptypes
Tests
MessageChannel
polyfill in the test environmentStyle
.prettierignore
and.gitignore
to reflect new file extensions for generated ReScript components.