preactjs / eslint-config-preact

Unopinionated baseline ESLint config for Preact and Preact CLI codebases.
https://npm.im/eslint-config-preact
MIT License
95 stars 16 forks source link

Loosen jsx-bind rule to allow functions #2

Closed marvinhagemeister closed 4 years ago

marvinhagemeister commented 4 years ago

This PR loosens the rule regarding inline functions for event handlers. Personally I've found that the difference is hardly measurable on any devices I own. Plus in Preact's case we just swap an object pointer during diff.

<Foo onClick={() => alert("1337")} />
<Foo onClick={function () { alert("1337") }} />
developit commented 4 years ago

I mostly agree with this change, though it does cause memoization to fail, which can have unfortunate impacts on diff performance. I wonder if we should just downgrade this to a warning?

I do use inline handlers all the time, so I end up just ignoring the warnings from this rule, which is probably silly.

marvinhagemeister commented 4 years ago

While we can ignore warnings, I think it's much more confusing for newcomers who may not understand why the warning is there in the first place. Memoization can be a problem and in the code-bases I've seen use memoization explicitly (via useMemo) or something else.