-
Notifications
You must be signed in to change notification settings - Fork 41
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
Consider removing prop-types dependency #95
Comments
Ideally we would move off of react-smooth. Its too much to manage and difficult because it isn't in TS. The problem is, if we were to move to TS prop-types is the only thing that tells us what is supposed to be what as none of the current maintainers have ever really touched this code except to fix some small things. I'll take a look if you want to open a PR but there might be some hesitation |
i'm not sure I understand what you mean by that... as about TS/JS i didn't notice at first that project isn't fully in typescript... what is a state of test coverage for the project? I mean if a new person like me, that isn't familiar with codebase itself, would like to rewrite JS to TS and add types to bundle... can I be sure I didn't break anything only by running tests? |
By the statement I mean we would use a different library for animation, one we don't maintain. Before this week there was only like 3 tests and it was 100% JS. I added a whole bunch of tests and coverage is now at about 90%, but no you cannot guarantee you didn't break anything by just running them unfortunately. |
prop-types
are marked as deprecated as of 2017 and in the upcoming React 19 validation failure will start to be silently ignored. Official React docs suggest replacing it with TypeScript interface to still benefit from build-time checks and reduce the overall bundle size. Please consider removing this lib from the list of dependencies. If you are OK with, I'll be glad to contribute the fixhttps://react.dev/blog/2024/04/25/react-19-upgrade-guide#removed-proptypes-and-defaultprops
The text was updated successfully, but these errors were encountered: