Skip to content

Fixed types to make the library compatible with React 18 #233

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

Closed
wants to merge 1 commit into from

Conversation

MartaGolabek
Copy link

Since React 18, 'children' isn't an implicit prop any more, one has to set it manually:
https://reactjs.org/blog/2022/03/08/react-18-upgrade-guide.html#updates-to-typescript-definitions
This hasn't been reflected in the react-swipeable-library's types yet. This is why, I have enhanced SwipeableListItem props with children property.

@przemyslawzalewski
Copy link
Member

Thank you for your contribution @MartaGolabek. While your proposal is entirely correct, we will stick to the one implemented via #230 as the children are explicitly required by the inner prop-types validation (#230 (comment)). PropsWithChildren would mark them as optional.

Moreover, there were recent issues with the PropsWithChildren - better to avoid going into that pit facebook/react#24304 😉

@MartaGolabek
Copy link
Author

@przemyslawzalewski thank you for the answer! I didn't see the other pull request. I am happy when one of the solutions is merged and I can keep using react-swipeable-library without a monkey patch ;-) Do you know when #230 is going to be merged?

@przemyslawzalewski
Copy link
Member

@MartaGolabek The PR is now merged, we will push a release soon afterward 👍

@przemyslawzalewski
Copy link
Member

@MartaGolabek The children type adjustments and peer dependency support for React 18 have been just released via https://github.com/sandstreamdev/react-swipeable-list/releases/tag/v1.0.2 🚀
I hope you can avoid the monkey-patching now 🙏

@MartaGolabek
Copy link
Author

@przemyslawzalewski thank you very much!

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.

3 participants