-
Notifications
You must be signed in to change notification settings - Fork 12
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 19 #79
Conversation
Hi Thanks For this npm package, I am also facing issues while deployment due to version issues, when are you planning to release new version of this package |
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.
Thank you for your contribution @acelaya, I really appreciate it. If you could simplify the versions matrix then I'm happy to go forward and merge this.
I'll do the change, but can you approve the workflow execution? Just to make sure everything works as expected. I sort-of tested it on my own fork's actions, but it ended up failing due to not being able to upload the code coverage reports from there (which is expected), so I think it should pass here, but we don't know for sure until it's green 😅 |
Pull Request Test Coverage Report for Build 12792683108Details
💛 - Coveralls |
Thank you @acelaya |
Closes #78
This PR is primarily focused on allowing this package to be installed with React 19, by updating the peer dependency constraint to
^18.0.0 || ^19.0.0
.On top of that, this PR adds a few extra improvements:
There's an open question though: The project defines a peer dependency on react, but dev dependencies on react and react-dom. I have only updated the peer dependency, but it probably makes sense to keep those in sync.
However, when using npm, you can skip duplicating dev and peer dependencies, as it installs peer dependencies if it's not explicitly defined anywhere else. That would require defining react-dom as a peer dependency though, which probably makes sense, but I'm not sure if it would be a breaking change.