Hacker News Reader

Comment by rjeli | Hacker News Reader

I don't like the explanation as a set of piecewise cases. this just refers to the current object context. this refers to the window at the top level because you're operating within `window`. There aren't "simple functions" and "object constructor functions", just functions, and the `new` keyword executes the function within the context of a new object. Event listeners are not a special case, it's just that the callback is executed from the context of the event -- which makes the difference between `function (e) { this }` and `e => this` much easier to understand.

View on HN

I agree. The key to grokking this is the difference between a function's execution context and its lexical scope. If you don't explain that I don't think you've explained anything, and if you do then there's no need to think about separate cases like the article suggests.

If you think of javascript code like an english sentence, then the function is the verb, the context (this) is the subject and the scope (arguments, closed over variables) are the nouns.

s/nouns/objects/g (then the analogy works for me)

The unfortunate thing about the lengths people go to in order to try to "fix" `this` in event handlers is that it wouldn't be a problem if people just implemented the event listener interface correctly.

PSA Don't do this:

  proto.foo = function() {
    /* ... */
    var self = this;
    element.addEventListener("click", (function(event) {
      self.iHaveAnUnhealthyObsessionWithLambdas(event.target);
    }));
  }

  proto.iHaveAnUnhealthyObsessionWithLamdas = function(clicked) {
    /* do something with clicked element `clicked` */
  }

Do this instead:

  proto.foo = function() {
    /* ... */
    element.addEventListener("click", this);
  }

  proto.handleEvent = function(event) {
    /* do something with clicked element `event.target` */
  }

But then if you're listening to multiple events, won't you have to try to disambiguate after the fact in handleEvent()?

That's the fun part, because here you can code your event handling to read something more like a state machine. Here's a quick example in bogus code, adding and removing event listeners as we go along (and without keeping reference to all the callback functions):

  class ElementDragger {
    constructor(elm) {
      elm.addEventListener('mousedown', this);
    }
    handleEvent(e) {
      const elm = e.target;
      switch(e.type) {
        case 'mousedown':
          elm.removeEventListener('mousedown', this).
          elm.addEventListener('mousemove', this);
          elm.addEventListener('mouseup', this);
          break;
        case 'mousemove':
          elm.style.left = e.clientX + 'px';
          break;
        case 'mouseup':
          elm.removeEventListener('mousedown', this);
          elm.removeEventListener('mouseup', this);
          elm.addEventListener('mousedown', this);
          break;
      }
    }
  }

That seems complicated, fragile, and bad for performance in a DOM. Event bindings are slow (or at least, used to be).

I don't know the purpose unbinding mousedown on mousedown - it only fires once per mousedown doesn't it?

Why not just stick with `elm.addEventListener('mousedown', this.handleMouseDown.bind(this))`? Where's the benefit other than making things more verbose?

Bind is less performant, because bind creates a new function with new context. At the framework level, bind is avoided at all costs. @wiredearp's approach is correct, because it only adds a single event listener, everything else is conditional.

If you think the above code has bad performance because "event bindings are slow" then your preferred form using `bind` is going to have all of that and more; it sidesteps nothing (except extremely fast switch case matching) and only adds overhead, both in runtime and memory costs.

> Where's the benefit other than making things more verbose?

To be clear what you're asking, are you saying that:

    elm.addEventListener('mousedown', this);

... is more verbose than:

    elm.addEventListener('mousedown', this.handleMouseDown.bind(this))

No, that a switch statement is more verbose than a single function.

Switch statements in OOP are a sign that something is wrong.

It's however easy to read and that feels good if not right, since the common solutions to this imagined problem require substantial mental overhead as seen on https://hackernoon.com/rethinking-javascript-eliminate-the-s.... To promote "single functions" we could:

  handleEvent(e) {
    this[e.type](e, e.target);
  }
  mousemove(e, elm) {
    elm.style.left = e.clientX + 'px';
  }

... but then the discussion would almost certainly revolve around this questionable decision instead of the original topic.

Switch statements are fine for simple tasks like the one in the example, especially earlier in a project where you do not know what you need in the future. You can always switch it to the correct pattern in version 2 as that way its better understood what hidden needs the code in question has, thus making it easy to pick a good OOP patern to replace it.

Then again, my jugment might be colored by working on codebases that are >21 years old.

> No, that a switch statement is more verbose than a single function.

You're vacillating and applying an inconsistent standard. The switch statement is used where multiple listeners are being attached, therefore it wouldn't work with your "single function". You'd need multiple functions.

Give any example passing functions directly as the callback to `addEventListener` to demonstrate your case, please. The equivalent version that simply implements DOMEventListener will be less verbose, be lighter on resources (both with memory use and with runtime), and have no need for any this-binding workaround weirdness.

If you plan to remove that event listener later on, you would need to go full verbose.

  this._callback = this.handleMouseDown.bind(this);
  elm.addEventListener('mousedown', this._callback);
  elm.removeEventListener('mousedown', this._callback);
  this._callback = null;

Compared to:

  elm.addEventListener('mousedown', this);
  elm.removeEventListener('mousedown', this);

@richthegeek I removed that event listener in my example to illustrate how easy it is, there was otherwise no good reason for it.

Better yet element.addEventListener("click", self.handleClick.bind(this))

This is less performant, because bind creates a new function with new context. At the framework level, bind is avoided at all costs.

The overhead is not important, if you bind once. It becomes problem, if you do it too often, like in react render function.

Today I learned

In general, a good practice is to avoid anonymous functions for the listener else it is not possible to remove the event later.

I guess the function could still be anonymous, but you will at least need to keep it assigned to some variable or property name. If you make use of the `handleEvent` pattern that @carussell suggested, you can however avoid having to keep track of all these callback functions since the listener can always be removed by passing `this` a second time:

['click', 'focus', 'blur'].forEach(type => elm.removeEventListener(type, this));

The problem is that the approach is different depending on context. You are making use of the assumption that the addEventListener function knows to call the "handleEvent" method of the object in the second argument, and give it a proper "this" object. That's a big assumption, which should be documented in the code.

The code using "self" doesn't have this problem; it's more self-contained so to speak (no pun intended).

Agreed. Much prefer 'var self = this' style. Keep the model transparent for future maintenance.

One time I don't is in Jasmine, where they explicitly state that 'this' is shared across all 'beforeEach', 'it' and 'afterEach' calls. Which backs up your first point.

I thought self had gone out of favor due to its relatively new global purpose.

ok, call it something else. I meant to say, save away the current ref to 'this'.

This is where I've landed with the JavaScript that I write. It keeps things relatively sane, at least better than the other approaches I've tried.

> That's a big assumption

No, it's not. At all. It's part of the DOM Level 2 standard that specifies what `addEventListener` does and has been for going on 20 years.

https://www.w3.org/TR/DOM-Level-2-Events/events.html#Events-...

Well I don't see it used in a lot of code, so in that respect I think it is obscure. Also, using the "self" approach allows one to lexically bind to the enclosing scope, which aligns nicely with how most functional programmers think.

> Well I don't see it used in a lot of code, so in that respect I think it is obscure.

You just moved the goalposts.

> I don't like the explanation as a set of piecewise cases

We don't like it and it's redundant. One should learn the principles and return to this article to read it as a quiz and find out whether he got it or not.

Books like the excellent Javascript Allonge (free to read on leanpub) do a great job on explaining the context. If you need more hardcore stuff, "You don't know JS" will fit you better.

(Disclaimer: I'm not affiliated with any of the recommended books or their authors or leanpub or whatever.They just helped me a lot; and still do from time to time)

Agreed. It’s extremely poorly explained and quite misleading

> this just refers to the current object context

Exactly.

"If you can't explain it simply, you don't understand it well enough."

Albert Einstein

As an event listener "function(e) {this}" is not the same as "(e) => this". With the regular function "this" points to the element on which the event listener was attached to while with the arrow function "this" points to the context (i.e. "this") of its lexical environment.

That's what the OP was saying.

This refers the to the object left of the .

Reply on news.ycombinator.com