0

I am creating a JavaScript component which I am creating instances of based on jQuery results however, the DOM element which I pass into the constructor, although populated when I step through the loop in the calling code, is undefined when passed to the constructor.

Here's my class and constructor...

export default class DeleteButton {

    /**
     * Creates an instance of DeleteButton.
     * 
     * @param {object} element The DOM element to make into a delete button.
     * 
     * @memberOf DeleteButton
     */
    constructor(element) {        
        this.id = element.getAttribute("data-id");
        if (!this.id) throw new Error("The 'data-id' attribute is required.");

        this.deleteUri = element.getAttribute("data-delete-uri");
        if (!this.deleteUri) throw new Error("The 'data-delete-uri' attribute is required.");

        $(element).click(this.confirmRemove);
    }

    confirmRemove() { // does something }
}

and here's the calling code (This is a component manager that handles when to load components based on URLs / DOM state etc)...

export default class JsComponentManager {

    constructor(onLoader) {
        this._loader = onLoader;
        this.select = {
            deleteButtons: () => $(".js-delete-button")
        }
        this.result = 0;
    }

    bindComponents() {
        const paths = new PathManager();
        let $deleteButtons = this.select.deleteButtons()
        if ($deleteButtons.length > 0) {
            this._loader.add(this.renderDeleteButtons, $deleteButtons);
        }
    }

    renderDeleteButtons($elements) {
        $elements.each(() => {
            document.DeleteButtons = document.DeleteButtons || [];
            document.DeleteButtons.push(new DeleteButton(this));
        });
    }
}

This uses the following loader function to ensure that items are loaded...

/**
 * Adds an event to the onload queue.
 * 
 * @param {function} func The function to add to the queue.
 * @param {any} param1 The first (optional) parameter to the function.
 * @param {any} param2 The second (optional) parameter to the function.
 */
var AddLoadEvent = function (func, param1, param2) {
    var oldonload = window.onload;
    if (typeof window.onload !== "function") {
        window.onload = () => { func(param1, param2); };
    } else {
        window.onload = () => {
            if (oldonload) { oldonload(); }
            func(param1, param2);
        };
    }
};

module.exports = {
    add: AddLoadEvent
};

The onload management code seems to be running fine and, stepping through, code execustion is completely as expected until document.DeleteButtons.push(new DeleteButton(this)); - 'this' here is the DOM element, as I would expect, but as soon as the debugger steps into the controller the value is undefined.

Is this some odd scoping pain I've walked into?

Keith Jackson
  • 3,078
  • 4
  • 38
  • 66
  • *"but as soon as the debugger steps into the controller the value is undefined."* Are you saying `element` is `undefined` in the `DeleteButton` constructor? Not sure I follow what the problem is. However, what you are passing to `DeleteButton` is an instance of `JsComponentManager`, not an element. See http://stackoverflow.com/q/34361379/218196 . – Felix Kling Feb 22 '17 at 16:49
  • No - I'm not (and I know it's weird) 'this' in jQuery each is the element itself. See my answer below for more details. Essentially jQuery each is a total horribleness that doesn't seem to be able to pass the element out of itself due to limited scope. – Keith Jackson Feb 22 '17 at 17:14
  • Please have a look at the question I linked to. – Felix Kling Feb 22 '17 at 17:18
  • You cannot blame jQuery for not working with arrow functions. jQuery was created way before arrow functions existed. – Felix Kling Feb 22 '17 at 17:24
  • I had looked it over and I was aware of this - jQuery's .each method must do some rebinding to set the value of 'this' to match the element in the collection of jQuery objects. I think it must be resetting this in the parent call which is then picked up as the master 'this' value up the stack by the arrow function. It's very very odd. – Keith Jackson Feb 22 '17 at 17:24
  • You are overcomplicating things. `jQuery.each` simply calls the function with `callback.call(element)`. There is no "resetting" of `this`. It only sets the `this` value of the function when it calls it. If you didn't know about `.call` or `.apply`, have a look at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/apply . – Felix Kling Feb 22 '17 at 17:25
  • Indeed, but I don't think the arrow function is what is preventing it from passing out it's values. – Keith Jackson Feb 22 '17 at 17:26
  • It is exactly that! Arrow functions don't have their own `this`, hence jQuery cannot set the function's `this` value. Please see my answer and any other documentation about arrow functions. – Felix Kling Feb 22 '17 at 17:28
  • The value of 'element' (which in a JS forEach loop is a parameter) goes into 'this' when you use jQuery .each. – Keith Jackson Feb 22 '17 at 17:28
  • Exactly. But arrow functions *resolve `this` lexically*! Therefore the *caller* of the function (jQuery) *cannot set the value `this`*. – Felix Kling Feb 22 '17 at 17:29
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/136383/discussion-between-keith-jackson-and-felix-kling). – Keith Jackson Feb 22 '17 at 17:34

2 Answers2

2
renderDeleteButtons($elements) {
    $elements.each(() => {
        document.DeleteButtons = document.DeleteButtons || [];
        document.DeleteButtons.push(new DeleteButton(this));
    });
}

doesn't do what you think it does. jQuery relies on being able to set the this value of the callback function. But arrow functions don't have their own this, so jQuery cannot set the this value.

Inside the arrow function this will refer to whatever this refers to in renderDeleteButtons, which likely is an instance of JsComponentManager.

If you pass a function to another function and that function has to set the this value, you cannot use an arrow function. Use a function expression instead:

renderDeleteButtons($elements) {
    $elements.each(function() {
        document.DeleteButtons = document.DeleteButtons || [];
        document.DeleteButtons.push(new DeleteButton(this));
    });
}

See also: Arrow function vs function declaration / expressions: Are they equivalent / exchangeable?


Maybe this helps to demonstrate the difference between an arrow function and a function declaration/expression:

// our library:
function each(values, callback) {
  for (var i = 0; i < values.length; i++) {
    // we use `.call` to explicitly set the value of `this` inside `callback`
    callback.call(values[i]);
  }
}


// Function declaration/expression
var obj = {
  someMethod() {
    "use strict";
    each([1,2,3], function() {
      console.log('function declaration:', this);
    });
  },
};

// Because we use a function expression, `each` is able to set the value of `this`
// so this will log the values 1, 2, 3
obj.someMethod();

// Arrow function
obj = {
  someMethod() {
    each([1,2,3], () => {
      "use strict";
      console.log('arrow function:', this);
    });
  },
};

// `this` is resolved lexically; whatever `each` sets is ignored
// this will log the value of `obj` (the value of `this` inside `someMethod`)
obj.someMethod();
Community
  • 1
  • 1
Felix Kling
  • 795,719
  • 175
  • 1,089
  • 1,143
0

I've now got this working by abandonning jQuery.each (which seems to have serious scoping issues passing the element from the array to anything else due to the way it messes with 'this'). I solved this by using a JS forEach call instead as follows. Discovering jQuery's makeArray method was the key. This is similar to what I had started with originally but was banging my head against forEach not working on a jQuery object...

renderDeleteButtons($elements) {
    $.makeArray($elements).forEach((el) => {
        document.DeleteButtons = document.DeleteButtons || [];
        document.DeleteButtons.push(new DeleteButton(el));
    });
}

It also doesn't hurt my sensibilities by doing weird stuff with 'this' (for Felix)

See Felix's extra info on lexical scoping with 'this' at Arrow function vs function declaration / expressions: Are they equivalent / exchangeable?

Community
  • 1
  • 1
Keith Jackson
  • 3,078
  • 4
  • 38
  • 66