Today I spent a number of hours with a coworker trying to figure out why a dropdown component was unmounting on certain clicks.
There is a cool date-range picker built by another coworker that allows the user to select two different days like so:
Our QA had discovered a bug where clicking a day in June when looking at May caused the entire thing to close.
What? Why is that happening?
We had solved a similar problem a few months ago so we spent a minute tracking slack and git history to try and see what we had changed. Eventually my coworker discovered the problem: there was an event that was happening on click of document in which we were checking to see if the event's target was a child node and closing if not the case.
componentDidMount() {
document.addEventListener('click', this.handlePageClick);
}
handlePageClick = e => {
if (!this.myDiv.contains(e.target)) {
this.hideChild();
}
};
But myDiv does contain the target..? Well yes, in every case except the May example. You'll notice in the May example there is an extra row. That row disappears as soon as you click a day in June because our component is unmounting those nodes before the document click event listener is running. Watch what happens when I disable the click event.
If the document click event listener was active, it'd try and find the DOM node that is no longer there on that 6th row.
There are a few ways we could have approached this.
- have a ghost 6th row that is always present but only sometimes visible
- delay the removing of the row until the document click event fire finished (callback, timeout?)
- add a property to the event.nativeEvent object that we could check for from the document event listener to conditionally unmount
We eventually decided to pursue event bubbling. It didn't make sense to have any click events bubbling out of the popup, so why not kill them? Problem is: they already were killed. We saw this all over the code:
<div onClick={e => e.stopPropagation()}>
So why then was it still firing? Why isn't event.stopPropagation() working?!
I came to the conclusion that the document event listener was indifferent to any children clicks after drafting a codesandbox that showed the document event listener firing regardless of stopPropagation.
But then my coworker tweeked w3school's sandbox to show me I was wrong. So I was then scratching my head.
Then it hit me. React's SyntheticEvent. I remembered that React uses a single event listener on the document. If we're adding others and trying to stop propagation, that's probably meaningless to other document event listeners because they're on the same node.
I learned that the workaround is to use window.addEventListener() instead which solved our problem instantly. Too many hours to learn a hard lesson: review and master the React docs.
componentDidMount() {
// document.addEventListener('click', this.handlePageClick);
window.addEventListener('click', this.handlePageClick);
}
Top comments (4)
That's a bad idea to use native event listeners instead of synthetic react listeners.
You also need to keep eyes on the cleanup (i,e. for you case you may forget to delete event listener from window and introduce a memory leak).
Instead you can use event.nativeEvent.stopImmediatePropagation() to stop event propagation and prevent events being bubbled to parent component.
event.stopPropagation();event.nativeEvent.stopImmediatePropagation();
I wanted to add that I just solved my case. I spent hours figuring why my higher level event blocks a nested one. Even with e.stopPropagation() . And suddenly It became clear - I was using innerHTML :) So I had one global event on body and one more on h1 and for a test I was adding an element in body next way -
body.innerHTML = body.innerHTML + "<my-element></my-element>";. That's what happens when we are following bad practices)))how would i go about doing this with functional components?
meet the same problem, thank you for solve my problem