Only start listener after some condition?

[cmdcolin]

I am not sure if this is a great use case but sometimes we have code like this

useEffect(() => {
    if(mouseDown) {
        window.addEventListener('mouseup',...) // make sure to capture the mouseup even if it is outside of the target div
    }
    return () => window.removeEventListener('mouseup',...)
},[mouseDown])

<div onMouseDown={() => setMouseDown(true)}/>

Do you thing it's worth it to have a conditional use-event-listener for a case such as this? Or do you think code like this is maybe an antipattern

[srmagura]

This seems like it could be an antipattern. Wouldn't it be simpler if window always had a mouseup event handler? If mouseup occurs but mouseDown == false, the mouseup handler would just do nothing.

If I am missing something about your use case, feel free to explain.

[cmdcolin]

The basic idea would be to avoid maybe having unnecessary mouseup handlers if there are many elements on the screen, but also sort of avoid having unnecessary logic in my mouseup handler to make it relevant only when a sort of "click and drag" is happening

This sometimes occurs if I have like say, a slider

<------->

I start by clicking on the dashes

Then I quickly drag my mouse to the right and it goes fully off the slider but I still want to capture the mouse up, so I add a global event handler to make this work

[cmdcolin]

The use case I have isn't specifically for slider but you can see slider app using this type of thing here https://github.com/mui-org/material-ui/blob/master/packages/material-ui/src/Slider/Slider.js

[cmdcolin]

It might be outside the scope of this repo but I was just thinking...if there was a way to simplify this type of logic, it would be pretty nice

[srmagura]

Cool, I see what you are trying to accomplish now. You really just want a more straightforward way to write the code and you want peace of mind that there is no performance degradation due to having many unnecessary mouseup handlers.

It would be nice if you could do:

const [mouseDown, setMouseDown] = useState(false)

function mouseUpHandler() { ... }

useEventListener('mouseup', mouseDown ? mouseUpHandler : undefined)

return <div onMouseDown={() => setMouseDown(true)}/>

Is this how you want it to work? (this doesn't work currently)

I was going to go ahead and implement this new feature, but now I'm concerned about a race condition. The mouseup event could occur before the React component rerenders, in which cause mouseUpHandler would not be called. Even if this works in today's React, it might stop working when React starts doing async rendering. What are you thoughts here?

[cmdcolin]

It does seem like these is a danger in using conditionals in hooks like this. It seems like maybe it would be ideal to have a dependency array type thing a la useeffect

useEventListener('mouseup', () => {
    if(mouseDown) {
        mouseUpHandler()
    }
}, [mouseDown])

Note: haven't really fully thought it through...just copying the useEffect style of thought :)

[srmagura]

useEventListener always uses the latest event handler function you provide, so I don't see the benefit of passing a dependency array.

Adding this feature seems deceptively complicated because of potential race conditions. I'm going to close the issue for now since there is an easy (if not ideal) workaround. If you can think of good way to make useEventListener conditional, we can discuss further.

[cmdcolin]

Just in case anyone comes across this, my original case (adding a global mouse up and mouse move handler after a mouse down) is actually nicely solved by pointerEvents (just setPointerCapture and then pointMove and pointerUp do not have to be global, see https://codesandbox.io/s/ox5lx949oq)