ktmud commented on pull request #11002: URL: https://github.com/apache/incubator-superset/pull/11002#issuecomment-697154594
> I agree for the most part. Functional components + hooks work quite differently than class based components. For new components it absolutely makes sense to _prefer_ functional components, especially for new components. I just wanted to point out that refactoring class components to be function components requires rethinking the entire implementation and can result in unexpected bugs that are difficult to diagnose. It is a significantly more complex task than say converting a component from jsx to tsx. 100% agree. I'd also like to point out sometimes converting to TypeScript is also not as easy as one might think. It's one thing to change a file name and add a bunch of primitive types, it's another to add proper strong typing that really gives future developers peace of mind and smart intellisense. I'm all in favor of stability and not doing refactoring for refactoring's sake. But if you can improve code quality along the way and are confident about your changes, by all means please do it. It'll be a win for everyone in the long run. I think right now it's just people are still not used to the best practices and common patterns with React hooks, so things can be a little brittle when hooks are used incorrectly. Enabling [the react/hooks ESLint rule](https://github.com/apache/incubator-superset/pull/11006) should help mitigate this more or less. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
