search

nytimes / react-tracking

🎯 Declarative tracking for React apps.
https://open.nytimes.com/introducing-react-tracking-declarative-tracking-for-react-apps-2c76706bb79a
Other
1.88k stars 123 forks source link

Add eslint react hooks plugin #143

Closed tizmagik closed 4 years ago

tizmagik commented 4 years ago

Figured we should probably pull in eslint-plugin-react-hooks since we're making heavy use of them now.

At first I applied the auto-fix for all issues, but that broke one of the e2e tests: does not cause unnecessary updates due to context changes -- and that's a critical one for performance reasons. So I had to selectively eslint-disable the tracking dependency in the hooks dependency array for a couple of the hooks. Not sure if there's a more idiomatic way around that?

Admittedly, I haven't personally used Hooks too much myself, so I'm definitely keen to hear your feedback on this, @bgergen if you could take a look when you get a chance 🙏

bgergen commented 4 years ago

Yeah, there may be a more idiomatically correct way to write some of these functions so that we're being fully "honest" about their dependencies. I'll do some digging and let you know what I find.