albertogasparin / react-magnetic-di

Dependency injection and replacement for Javascript and React components/hooks
MIT License
131 stars 7 forks source link

missing React hooks exemption for exhaustive-inject #66

Closed theKashey closed 1 year ago

theKashey commented 1 year ago

The rule contains a few react hooks - useState, useContext, useReducer - but not any other.

https://github.com/albertogasparin/react-magnetic-di/blob/master/src/eslint/rules/exhaustive-inject.js#L16

Is it a feature or a bug? I can understand if someone wants to override useEffect, but am a little concerned about useRef In fact useState is overridden the most.

However adding any exemptions to the rule may cause friction to the testing process as something you might expect to be overridden will not be.

albertogasparin commented 1 year ago

Is it a feature or a bug?

It's a feature, as I thought the defaults for replacing things like React builtin hooks should be limited. Currently the only 3 automatically added are: useState, useContext, useReducer.

You can add all others manually but I had concerns as their implementation is more iffy, so wanted to limit their exposure. Things like useRef, useMemo, useCallback, ... I feel are tricky to mock and wrong mock = wrong test. Ideally no builtin hook should be used with di, as it would be much better to defined the behaviour in a custom hook, but that might require more hardening of hook rules.

theKashey commented 1 year ago

If someone want to mock useState and have a good reason for it (there are a few good examples) - let them do it. I am more concerned with auto-adding some unrelated stuff to di.

However there is another popular problem - not adding something to di and spending some time trying to understand what went wrong and why my test is not working as expected. I've created another issue to track that - #69

albertogasparin commented 1 year ago

The rule is no longer necessary on v3