feat: improve useClickAway() hook #394
Conversation
add deps for skip useless event sub/unsub
| @@ -8,10 +8,14 @@ const useClickAway = ( | |||
| onClickAway: (event: KeyboardEvent) => void, | |||
| events: string[] = defaultEvents | |||
| ) => { | |||
| const savedCallback = useRef(onClickAway); | |||
There was a problem hiding this comment.
Instead of saving the callback inside the useClickAway hook, isn't it better if consumer do it manually?
const onClickAway = useCallback(() => { /* ... */ });
useClickAway(ref, onClickAway);There was a problem hiding this comment.
I don't want to write extra code on every useClickAway usage
and this approach is less effective
There was a problem hiding this comment.
may I ask what is the purpos of using the useRef here ?
why not simply use the callback ?
| @@ -21,7 +25,7 @@ const useClickAway = ( | |||
| off(document, eventName, handler); | |||
| } | |||
| }; | |||
| }); | |||
| }, [events, ref]); | |||
There was a problem hiding this comment.
events is an array, I though array cannot be a dep, maybe it should be like this:
[...events, ref]There was a problem hiding this comment.
you're right 🤗
There was a problem hiding this comment.
sorry, I'm trying to figure it out myself.
And you approach is totally wrong, with spread operator in deps react writes this warnings
Warning: The final argument passed to useEffect changed size between renders. The order and size of this array must remain constant.
Previous: [1, 2, 3, 1, 1, 1]
Incoming: [1, 2, 3]
in App
There was a problem hiding this comment.
Could useEffect be changed to useDeepCompareEffect?
There was a problem hiding this comment.
Why? useEffect works fine with array!
See codesandbox example
There was a problem hiding this comment.
I guess you are right. I was thinking if events is provided inline, it will be a new array every re-render.
useClickAway(ref, fn, ['mousedown', 'touchstart'])User needs to make sure events is memoized.
# [9.7.0](v9.6.0...v9.7.0) (2019-06-19) ### Features * improve useClickAway() hook ([#394](#394)) ([c60df19](c60df19))
|
🎉 This PR is included in version 9.7.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
No description provided.