'this' in callback bound as closure via apply becoming undefined if called in specific order

Question

I have a JS function where I try to use this which is supposed to be the button clicked to trigger the function:

function myAction( greeting ) { console.log( greeting + ", the button is :" + this ); this.remove(); }

I then have several buttons on my page, let's say two (this really even happens with only two buttons on the page; I've tested it):

<button id="button-one" class="click-me">Click Me</button>
<button id="button-two" class="click-me">Click Me</button>

From my experience, when I want to use event listener callbacks which require the use of the concerned element as this plus other function arguments; if we have several buttons sharing for example the same class; the best way to reach this is to bind that callback as a closure (to capture the iterated element as this + apply (to deliver the concerned this element):

var myButtons = document.getElementsByClassName( "click-me" );
var amountOfButtons = myButtons.length;
for ( var i = 0; i < amountOfButtons; i++ ) {
    myButtons[i].addEventListener( "click", (
      function(i) {
        return function() { myAction.apply( myButtons[i], ["hello"] ); };
      }(i))
    );
  }

I never had any problem with this approach; it always worked. Is it actually the wrong approach?

Because, what I'm facing now is that, when I click on #button-one, and then click on #button-two, this actually becomes undefined in strict mode and the window in non-strict mode; on the second click. When I do it the other way around, this always sticks to be the accordingly clicked button. It somehow seems to update the myButtons collection, such that when the first button gets removed via the first call, the 1 index becomes unavailable in the collection, as there's only one remaining button; the other one being removed. At least that's my theory. But I've never faced this issue before in similar code cases? How shall I code what I want to happen then?

EDIT

Thanks to all of you; and special thanks to @Nenad Vracar, as his solution led to the minimum needed edits which perfectly work, by simply changing:

var myButtons = document.getElementsByClassName( "click-me" );
var amountOfButtons = myButtons.length;
for ( var i = 0; i < amountOfButtons; i++ ) {
    myButtons[i].addEventListener( "click", (
      function(i) {
        return function() { myAction.apply( myButtons[i], ["hello"] ); };
      }(i))
    );
  }

To:

var myButtons = document.getElementsByClassName( "click-me" );
var amountOfButtons = myButtons.length;
for ( var i = 0; i < amountOfButtons; i++ ) {
    myButtons[i].addEventListener( "click", function() {
      myAction.apply( this, ["hello"] );
    });
  }

Simple smart solution, I could literally not see the wood for the trees! thanks again!

Note. Event delegation was not really an option here, as the DOM structure of the page / the elements needing the attachment is somewhat complicated, and I generally prefer to attach the function to the elements needing it. Also, I prefer to not change the code of the function to be a closure itself, to not make changes to the function code itself. Hence the reason why I've decided to use this solution.

Answer 1

You could use slightly different approach where you use querySelectorAll and then you can use forEach loop on that. And inside the click event handler you don't need that extra closure and this inside the myAction will be the this inside the click event handler since you passed it as thisArg parameter to apply.

function myAction(greeting) {
  console.log(greeting + ", the button is :" + this.textContent);
  this.remove();
}

document
  .querySelectorAll(".click-me")
  .forEach(function(btn) {
    btn.addEventListener('click', function() {
      myAction.apply(this, ['Hello'])
    })
  })

<button id="button-one" class="click-me">One</button>
<button id="button-two" class="click-me">Two</button>

Answer 2

Your approach right now seems very complex. Here are two suggestions to simplify the process.

Higher order function event handler

If you re-define myAction as a function that returns the event listener, you can pass arguments directly. Therefore, there would be no need to wrap it in another function when assigning the handler:

function myAction( greeting ) { //take input
  return function() { //return the function to act as event listener
    console.log( greeting + ", the button is :" + this ); this.remove(); 
  }
}

var myButtons = document.getElementsByClassName( "click-me" );
var amountOfButtons = myButtons.length;

for ( var i = 0; i < amountOfButtons; i++ ) {
    //pass a variable to be used in the listener
    myButtons[i].addEventListener( "click", myAction( "hello" ) );
}

<button id="button-one" class="click-me">Click Me</button>
<button id="button-two" class="click-me">Click Me</button>

Use event delegation

This is the preferred approach. Instead of adding and event listener to each of the elements, you can add a single event listener to a common ancestor in the DOM tree. Then you only need one function.

Worth noting that the event listener will receive events from all children, so it needs to filter out irrelevant ones. You can make a simple wrapper to check if the target target is correct, then execute the decorated handler:

function myAction( greeting ) { 
  return function(event) { //take an event 
    var el = event.target; //extract the element from it
    console.log( greeting + ", the button is :" + el ); el.remove(); //use the element
  }
}

/*
 * Convert a handler to a delegate to only work on a sub-section of elements
 * @param {string} filterSelector - the handler will only be triggered
 *   for elements matching this selector
 * @param {Function} handler - the handler to wrap
 *
 * @return {Function} a function to serve as a delegated event handler
 */
function delegate( filterSelector, handler ) {
  return function ( event ) {
    //only fire for explicitly desired targets
    if (event.target.matches( filterSelector ) ) {
      //forward call to the handler
      handler.apply( this, arguments );
    }
  }
}


var delegatedHandler = delegate( ".click-me", myAction( "hello" ) );

document.body //attach higher up in the DOM
  .addEventListener( "click", delegatedHandler );

<button id="button-one" class="click-me">Click Me</button>
<button id="button-one" class="dummy">I should stay</button>
<button id="button-two" class="click-me">Click Me</button>

See Element#matches() for potential polyfills if you're working with older browsers.

See also:
What is DOM Event delegation?
Event binding on dynamically created elements?