search

recharts / react-smooth

react animation
MIT License
277 stars 38 forks source link

Consider removing prop-types dependency #95

Open vorant94 opened 1 month ago

vorant94 commented 1 month ago

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 fix

https://react.dev/blog/2024/04/25/react-19-upgrade-guide#removed-proptypes-and-defaultprops

ckifer commented 1 week ago

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

vorant94 commented 2 days ago

we would move off of react-smooth

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?

ckifer commented 2 days ago

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.