Last week I looked over some JavaScript a colleague of mine had written, and I noticed an interesting mistake. Interesting because I knew that I had myself tripped over this at some point, and seeing it in someone else’s code showed me that it’s indeed a tricky problem more people are struggling with, so I thought it might be worth sharing.
(Also I wrote a very lengthy issue for them, describing the problem and how to fix it, which was the basis for this post.)
Here’s the first part of the code in question (we’re talking about plain ES6 in the browser, no frameworks. Also this is not the original code, I removed everything unrelated to the point I’m trying to make):
Ⓐ
// For completeness, imagine these being something sensible:
let elements = document.querySelectorAll(/* ... */)
function mouseenterHandler(element) {
// ...
}
// This is the interesting part:
elements.forEach(element => {
element.addEventListener('mouseenter', () => {
mouseenterHandler(element)
})
})
Some DOM elements are queried for and event listeners are being attached to each.
And then, somewhere further down, in a teardown routine:
Ⓑ
elements.forEach(element => {
element.removeEventListener('mouseenter', () => {
mouseenterHandler(element)
})
})
This, apparently, is an attempt to unregister the event listeners by calling removeEventListener
with equivalent anonymous functions as arguments.
The problem
() => { this.mouseenterHandler(element) }
in Ⓐ is an anonymous function that we retain no reference to (i.e., it is not saved in a variable or given a name in any way).
() => { this.mouseenterHandler(element) }
in Ⓑ is an equivalent anonymous function. The important thing to note here is that they’re equivalent, but not the same.
How JavaScript compares functions
Functions in JavaScript are objects, and, like all other objects, they’re compared by reference. What that means is that JavaScript has no way to determine the equivalence of two functions.
Y u no serialise them?
Now we might think, JavaScript can already serialise functions, why doesn’t it simply compare them by their string representation?
let f1 = (x) => { return x + 1 }
let f2 = (x) => { return x + 1 }
console.log(f1.toString()) // '(x) => { return x + 1 }'
console.log(f2.toString()) // '(x) => { return x + 1 }'
// ... so:
console.log(f1.toString() === f2.toString()) // true - yay!?
But let’s consider this slightly different, although arguably equivalent function:
function f3(x) {
return x + 1
}
console.log(f3.toString()) // 'function f3(x) {\n return x + 1\n}'
It is quite obvious that f1.toString() === f3.toString()
and f2.toString() === f3.toString()
will be false, even though it’s also trivial that f1(x) === f3(x)
and f2(x) === f3(x)
will be true for any given x in [Number.MIN_SAFE_INTEGER
, Number.MAX_SAFE_INTEGER - 1
] (and actually for many other values of x
, too).
So this method would only work for functions that are written out in exactly the same way.
How it’s actually done
In JavaScript, there are essentially three* basic data types that are immutable, which is a fancy comp-sci way of saying they sort of behave like things do in pen-and-paper mathematics. One of them is the Number
type. In maths, there is only one number 𝟐. It makes no sense to talk about this 𝟐 here and that 𝟐 over there. We can write out the character «𝟐» as often as we’d like to, but every one of them will still be a reference to the same single number 𝟐. It works the same way in JavaScript:
let a = 2
let b = 2
console.log(a === b) // true
The other two* basic data types in JS are String
and Boolean
. That’s why we were able to compare the string representations of f1
, f2
and f3
by equivalence.
Everything else in JavaScript is compared by reference. Everytime we write []
, we create a new array, which is not the same one as the next time we write []
, everytime we write {}
we create a new object, and everytime we write () => {}
, we create a new function.
(Strictly speaking, it’s not every time we write []
, {}
, or () => {}
, it’s every time one of these is evaluated. Which is actually a big difference. Imagine a function like function makeObj () { return {} }
—everytime we’d call makeObj()
, it’d return a fresh new object.)
In other words,
console.log([] === []) // false
console.log({} === {}) // false
console.log((() => {}) === (() => {})) // false, too!
but
let a1 = []
let a2 = a1
console.log(a2 === a1) // true
let o1 = {}
let o2 = o1
console.log(o2 === o1) // true
let f4 = () => {}
let f5 = f4
console.log(f5 === f4) // also true
What’s it got to do with our eventListeners
Imagine that for each element in the DOM, JavaScript creates an array in which to keep all the mouseenter listeners, like this:
let myElementMouseenterListeners = []
And every time we add an event listener, like
myElement.addEventListener('mouseenter', () => { console.log('yay') })
JavaScript internally just adds it to the array:
let myListenerToAdd = () => { console.log('yay') }
myElementMouseenterListeners.push(myListenerToAdd)
console.log(myElementMouseenterListeners) // [ [Function] ]
When the 'mouseenter'
event occurs, JS will call every function in the array:
let myMouseenterEvent = new MouseEvent('mouseenter')
myElementMouseenterListeners.forEach(mouseenterListener => {
mouseenterListener(myMouseenterEvent)
})
And when we try to remove an event listener, JavaScript will iterate over the array of event listeners, comparing each function in there to the one we are trying to remove, and if it happens to be the same, remove it from the array.
Imagine we do:
myElement.removeEventListener('mouseenter', () => { console.log('yay') })
And JavaScript does:
let myListenerToRemove = () => { console.log('yay') }
for (let i = 0; i < myElementMouseenterListeners.length; i++) {
if (myElementMouseenterListeners[i] === myListenerToRemove) {
myElementMouseenterListeners.splice(i, 1)
break
}
}
console.log(myElementMouseenterListeners) // still [ [Function] ]
That means, when the loop arrives at the listener we added in the beginning, it will compare it to the one we supplied to removeEventListener
, so what happens is basically this:
() => { console.log('yay') } === () => { console.log('yay') }
Which, as we examined before, evaluates to false.
What this means is that code like
element.removeEventListener('eventname', () => { console.log('event handled') })
calling removeEventListener
with an anonymous function that is just newly created in that very moment as second argument, can never have any effect. It will, instead, fail silently.
What we need to do instead (possible solutions)
For removeEventListener
to have any effect, we’ll have to supply a reference to a function that we actually registered before via addEventListener
.
Generally, something like
let element = document.querySelector(/* ... */)
function mouseenterHandler() {
// ...
}
element.addEventListener('mouseenter', mouseenterHandler)
element.removeEventListener('mouseenter', mouseenterHandler)
will work, because we’re using a reference to the same function everywhere, so when removeEventListener
is called, it will find out which function to remove by comparing it like this.mouseenterHandler === this.mouseenterHandler
, which we can see is trivially true.
Now the “problem” is that our actual mouseenterHandler
is generalised - it takes an element as a parameter. Which is certainly a better solution than writing a new mouseenterHandler
function for each element we’re going to work with! But now we have to get the parameter in there somehow, and wrapping the call to mouseenterHandler
in an anonymous function won’t work here, as I verbously showed above.
Solution 1: create specialised versions of the event handlers for each element
We could create specialised versions of mouseenterHandler
that already have the extra argument baked in right after we populate elements
. Something like:
let elements = document.querySelectorAll(/* ... */)
let enhancedElements = []
elements.forEach(element => {
enhancedElements.push({
element,
mouseenterHandler() { mouseenterHandler(element) },
})
}
And then, change the code adding the handlers into
Ⓐ
enhancedElements.forEach(ee => {
ee.element.addEventListener('mouseenter', ee.mouseenterHandler)
})
and the removal, respectively, into
Ⓑ
enhancedElements.forEach(ee => {
ee.element.removeEventListener('mouseenter', ee.mouseenterHandler)
})
This will work, but it will also create an extra object and function per element, which may not be a problem if there aren’t that many elements, but still, there’s a more elegant way …
Solution 2: change our event handlers to work with the arguments they already get
The browser will call our event handlers with the event as the first argument. And an event is just an object with a number of properties, one of them being event.target
, which is a reference to the element on which the event occurred. So, why not change our event handlers to use that, so we don’t have to shoehorn the element in there manually?
If our mouseenterHandler, for example, looked like this:
mouseenterHandler(element) {
element.classList.add(/* ... */)
}
We could just change it to use event.target
instead:
mouseenterHandler(event) {
event.target.classList.add(/* ... */)
}
Or use destructuring right in the parameter list, so we don’t have to repeat the event.
part:
mouseenterHandler({ target }) {
target.classList.add(/* ... */)
}
With this solution, we can leave the let elements = document.querySelectorAll(/* ... */)
line how it is; no extra objects or functions needed, we’ll just have to change Ⓐ into:
elements.forEach(element => {
element.addEventListener('mouseenter', mouseenterHandler)
})
And Ⓑ, accordingly:
elements.forEach(element => {
element.removeEventListener('mouseenter', mouseenterHandler)
})
Our event handlers are now “universal” and can be used unchanged with any element.
* I lied. undefined
is a type, too.
Thank you for reading! This is my first post here on dev.to, and also I’m not a native English speaker, so general feedback regarding style, helpfulness, etc. would be appreciated :)
Top comments (19)
Thanks for your post. Very interesting reading for all kind of developers.
In the past I used to use event.target a lot. Nowadays I'm trying to get more familiarized with the 'this' shortcut.
The way you capture the event in the callback is the same, but inside the function I use to write:
mouseenterHandler(e) {
this.classList.add(/* ... */)
}
There are some considerations for you to have in mind when you use the 'this' inside a function. In my cases, I try to make sure that I'm dealing with something that can be accessed and handled with no problems (most of the time, it's an element within the DOM. I am still in the early begining of learning JS, I don't iterate over functions or some other 'complex' stuff). So for adding and managing event handlers or for iterate over the node list querySelectorAll returns, I'm kinda comfortable using 'this'
Any thoughts, recomendations or advices?
Thanks in advance
stackoverflow.com/a/21667010/7435195
You can find there insightful explanation regarding a problem you might face when using
this
Following that I'd recommend to read the links in the other comments.
Event bubbling is facinating! :)
Thank you for the comment. I wasn’t aware of the exact behaviour of
this
in event handlers.I tend not to use mechanisms like this because they make less explicit what I’m trying to do. Also, they cannot be used with ES6 arrow functions, because they always keep the
this
binding from their lexical context.@fgiraldi Kyle Simpson’s third “You Don’t Know JS” book (which can be read on GitHub for free) explains this in a lot of detail: github.com/getify/You-Dont-Know-JS...
Hi,
I got here with a problem that I have and this discussion is the closest to what I have found so far.
I have this:
document.addEventListener("wheel", event => { preventOnWheel(event) }, { passive : false });
I want to remove the event listener with:
element.onmouseout = function() {
document.removeEventListener("wheel", event => { preventOnWheel(event) }, { passive : false });
}
It is not working. Can you please help?
Hi,
you have exactly the problem that I tried to describe here :)
You'll have to save your actual event listener function (the
event => { preventOnWheel(event) }
) part in a variable so you can callremoveEventListener
with that exact same function afterwards; something like this should work:Actually your
preventOnWheel
function already seems to be a single-argument function that you could use as an eventListener directly – no need to wrap it in an extra function, so the above could be simplified to something like this:Also, you're mixing "old-style" event listener registration via assignment to
element.onmouseout
with the neweraddEventListener
/removeEventListener
API, and you're mixing arrow functions (() => {}
) with inlinefunction() {}
, so, if I may suggest a few more changes:Hope it works!
Hi Emanuel,
Thank you so much for your fast reply and descriptive answer.
Some background of the problem:
I have a image slider which changes images on wheel event. While you use the wheel and you are inside the element the wheel scroll is blocked with event.preventDefault and it changes the images. When you leave the element with the mouse, the wheel scroll is unblocked with event.preventDefault = false.
I did it using "old-style" and it worked well until Google in Chrome 73 decided to make the wheel event listener passive by default, and now the event.preventDefault does not work and there is no way to add {passive:false} except rewriting those functions in addEventListener style.
So I was trying to just make it work, and not make any unnecessary changes to the other code.
I will try your suggestions and I will write you the result.
Thanks again.
It worked!
Thanks again!
Glad to hear that it worked!
And sorry for the stylistic comments, I assumed you had written the whole code just now. Thank you for the additional explanations!
Hi. Did we maybe exchange a few emails last week?
I'm not sure if I get the point. What would the benefit of having a
setInterval
-like API be? You'd still have to keep an object – in that case the number returned fromsetInterval
– in scope, or pass it around somehow, to the place where you're doing the unregistering. Changes nothing IMHO.Anyway I think it's a good idea to keep the places where you're adding and removing your event listeners close together in your source file. Here's a pattern I often use with classes:
Sorry, I probably thought that was obvious, could have pointed it out more clearly. OTOH, explaining how scope works would have been clearly out of scope for this article.
To be clear, the solution I presented where I'm putting the event listeners in the array is there because I thought it was a minimal and hopefully easily understandable way to make this work. And while it works indeed, I would not recommend or use this in real code, because I think it's unnecessarily complicated.
Instead, I would either use the last solution shown (the one with the generalised event handler function that works unchanged for every element) or event delegation. And I'd probably use/recommend the former only in cases with very few elements and where no elements are added/removed or where I don't even care about removing the event listeners because they are used for the entire remaining lifetime of the app. In the vast majority of other cases I'd use event delegation, which was already mentioned in the other comments and which isn't the subject of the original post.
The post is about the fact that you have to keep a reference to the same single function and repeat that, both when adding and removing the event listener. It is already a very long text about a very specific thing. As you made clear now, you were having issues around the topics of closures and scope, which I think isn't the fault of my article. A single text cannot be concerned with everything.
If you are the one who I think you are, I even emailed you another complete code example, which I regret, because I think you were, and still are, rude.
Emanuel,
Let me be clear about two things. One of which is my identity; I am most certainly not the person who you think I am. We have never conversed before. The second is that my post was in no way disparaging what you wrote. I am little surprised you think that I was being rude in any way, it was certainly not my intention. You've clearly spent a lot of time and effort on this document/tutorial, and that can only be applauded.
My post was a mix between discussing the differences between interval/timeout and events, and some observational and anecdotal comments about my own experiences. Any questions raised were not done so with criticism.
We could have a much deeper conversation about this, but after your response I feel the best course of action would be for this thread to be deleted (I have deleted my comments but that leaves these child posts).
I do feel a little disappointed that our first exchange should result in such a misinterpretation of tone, but please believe me when I say I wish you all the best for the future.. :)
--- Edit:
To everyone else:
I have to say it's quite telling that Emanuel took just a few hours to write a diatribe accusing me of being someone else and using a tone in their response that assumed there was some sort of history of abuse.
However 5 days later they have failed to offer up any apology, or even acknowledge their error. This was after I gave them a considerable opportunity to do so without losing face.
If any prospective employers are reading this, please take note of this sort of behaviour.
To Emanuel:
In hindsight you will see that your response was insecure and petulant without any provocation. I just hope that hindsight happens sooner rather than later.
Signed up to dev.to just to say thank you! I haven't tried your solution yet but it gave me hope. Pulled my hair out trying to figure out the problem in my code; now it all makes sense.
It worked! By listing the handlers in an array; I'm now able to reference and use them properly during
addEventListener()
andremoveEventListener()
. Now I can even loop through an array of events and register them.Glad to hear you got it working :)
I'm pretty sure your code could be simpler though, let me know if you'd like some suggestions.
Happy to see your suggestion! Please show me how you can simplify it
Good read!
Actually, I always use "Solution 2" as you stated above. IMO, EventListener makes sense only if we are interacting with
event
s.This is the post I've been looking for. Thank you.
The second solution you mentioned is called event bubbling. check this article for in depth explanation.
I think you’re referring to the fact that some events “bubble up” the DOM (
mouseenter
, for example!), which means they can not only be observed on the source element where they occurred, but on any parent element, too. So you don’t have to register event listeners on every element that you want to catch the events on, because a single listener on a common parent element will catch them all.If you still need to know which particular element the event originated from, you can look that up in
event.target
, a property of the event object that I used in the last paragraphs of my post, too. Afaik, the browser mechanism of propagating events up the DOM is called event bubbling, while the technique of catching events on a parent element is called event delegation. The benefits of event delegation are that fewer event listeners are needed and that you don't have to remove or register event listeners as the DOM nodes emitting the events are added or removed, as long as the parent element you're listening on stays the same.My post, however, is not about event delegation–note how even in the last code example in my second solution, I’m still adding an event listener to every single element in the collection. It’s about an even more basic insight that I think comes maybe one or two steps before using event delegation: that event handlers can be designed in general terms to work on any element, because they are receiving the event.target as an argument.
I think this is quite basic stuff and I’m belaboring very elementary points in much detail here. If you work with event handling in JavaScript every day and know this stuff inside out, it may be hard to recognise the insight that enables us to go from the first to the second solution, because it's so little. I only started to think about it in so much detail because I tried to understand what my colleague was thinking and wanted to explain to them what was going on. Also, I agree that a logical next step to further improve the code from the second solution would certainly be to use event delegation. But I neither discussed event delegation in my post, nor did I use it in the solution I presented.