streamich / react-use

React Hooks — 👍
http://streamich.github.io/react-use
The Unlicense
41.77k stars 3.15k forks source link

`useDeepCompareEffect` should not show the warning when the values are not primitive but `null`/`undefined` at the moment #2227

Open italodeandra opened 2 years ago

italodeandra commented 2 years ago

I have this code that uses useDeepCompareEffect and when I have on the deps a value that is not a primitive value but at the moment it is null or undefined, it shows the following message:

`useDeepCompareEffect` should not be used with dependencies that are all primitive values. Use React.useEffect instead.

In my opinion this message should be removed for null and undefined values.

stoickeyboard commented 2 years ago

Super late but useDeepCompareEffectNoCheck was created for this use case.

From the README:

NOTE: Only use this if your values are objects or arrays that contain objects. Otherwise, you should just use React.useEffect. In case of "polymorphic" values (eg: sometimes object, sometimes a boolean), use useDeepCompareEffectNoCheck, but do it at your own risk, as maybe there can be better approaches to the problem.

italodeandra commented 2 years ago

Hey @HummingBird24. Thank you for the reply but in my opinion these are separate cases.

I agree that changing something from object to boolean can cause problems. But changing something from null or undefined shouldn't. Otherwise we would also have something like useEffectNoCheck from the React's own API.

But that is just my opinion, I'm not even using the hook anymore currently so I can't investigate.

stoickeyboard commented 2 years ago

@italodeandra I agree with you actually. It's normal to expect an object variable to possibly be null or undefined but not a boolean. So yeah although useDeepCompareEffectNoCheck gets around the problem, I don't think it should be required. It also confuses things with the naming.

xixixao commented 11 months ago

useDeepCompareEffectNoCheck doesn't exist anymore and useCustomCompareEffect has the same check inside, so there's no way to get around this besides rolling your own implementation.