Functions declared within loops warning

JavaScript
1

How is this warning message resolved ? https://jsfiddle.net/2um4tya9/

  function createResetHandler(player) {

    const resetVideo = document.querySelectorAll(".container.active .exit");
    for (let i = 0; i < resetVideo.length; i++) {
      const resetVideoHandler = function resetVideoHandler() {
        resetVideo[i].removeEventListener("click", resetVideoHandler);
        player.destroy();
        console.log("removePlayer");
      };
      resetVideo[i].addEventListener("click", resetVideoHandler);
    }
  }

This was an attempt at fixing it, but I wrote it wrong.

ReferenceError: i is not defined

 function createResetHandler(player) {
    function resetVideoHandler() {
      player.destroy();
      console.log("resetVideoHandler");
    }
    const resetVideo = document.querySelectorAll(".container.active .exit");
    for (let i = 0; i < resetVideo.length; i++) {

      resetVideo[i].removeEventListener("click", resetVideoHandler);
    }
    resetVideo[i].addEventListener("click", resetVideoHandler);
  }

This was my 2nd attempt at fixing it, this is wrong also.

  function createResetHandler(player) {
    function resetVideoHandler() {
      player.destroy();
      console.log("resetVideoHandler");
    }
    const resetVideo = document.querySelectorAll(".container.active .exit");
    for (let i = 0; i < resetVideo.length; i++) {

      resetVideo[i].removeEventListener("click", resetVideoHandler);
      resetVideo[i].addEventListener("click", resetVideoHandler);
    }
  }

3rd attempt at fixing it.

ReferenceError: i is not defined

function createResetHandler(player) {
        function resetVideoHandler(resetVideo) {
          player.destroy();
          console.log("resetVideoHandler");
          resetVideo[i].removeEventListener("click", resetVideoHandler);
        }
        const resetVideo = document.querySelectorAll(".container.active .exit");
        for (let i = 0; i < resetVideo.length; i++) {
          resetVideo[i].addEventListener("click", resetVideoHandler);
}
        }

4th attempt at fixing it.

 function createResetHandler(player) {
    const resetVideoHandler = function resetVideoHandler(resetVideo) {
      for (let i = 0; i < resetVideo.length; i++) {
        player.destroy();
        console.log("resetVideoHandler");
        resetVideo[i].removeEventListener("click", resetVideoHandler);
      }
    }
    const resetVideo = document.querySelectorAll(".container.active .exit");
    for (let i = 0; i < resetVideo.length; i++) {
      resetVideo[i].addEventListener("click", resetVideoHandler);
    }
  }
2

Hey there @javascriptcoding5678 ! Here at line 5 you reference i

  resetVideo[i].removeEventListener("click", resetVideoHandler);

The problem is that that function is not in a for loop and therefore Javascript doesn’t actually know what i is until you get to the for loop on line 8.

I suggest trying a higher order array method such as map, filter, reduce or such as they less ikely to give you these typs of errors. (also I only skimmed the code for the issue and I havn’t seen the codebase in context so take my advice on array mehods with a grain of salt)

best,
Cy499_Studios

3

facepalm

Just re-read your code


 const resetVideoHandler = function resetVideoHandler(resetVideo) {
      for (let i = 0; i < resetVideo.length; i++) {
        player.destroy();
        console.log("resetVideoHandler");
        resetVideo[i].removeEventListener("click", resetVideoHandler);
      }
    }

Extremly sorry for my mistake but I still think you might want to give map a try. Also could it be that you have two things names resetVideo:

resetVideo[i].removeEventListener("click", resetVideoHandler);

and

const resetVideo = document.querySelectorAll(".container.active .exit");

Yeah that’s all I’ve got so far. really sorry I was not able to provide more help but hopefully it helped!

4

Here is the code where I am trying to remove the warning message.

https://jsfiddle.net/2um4tya9/

What would this be changed to?

function createResetHandler(player) {

    const resetVideo = document.querySelectorAll(".container.active .exit");
    for (let i = 0; i < resetVideo.length; i++) {
      const resetVideoHandler = function resetVideoHandler() {
        resetVideo[i].removeEventListener("click", resetVideoHandler);
        player.destroy();
        console.log("removePlayer");
      };
      resetVideo[i].addEventListener("click", resetVideoHandler);
    }
  }
5

Honestly I’m less experienced in Javascript (exremly junior dev) so I would suggest reaching out to a more senior dev on the forums such as @jwilkins.oboe or @ArielLeslie for help on this.

6

This worked https://jsfiddle.net/gx163tzd/4/

  function createResetHandler(player) {
   const resetVideo = document.querySelectorAll(".container.active .exit");

   const handler = function resetVideoHandler() {
     this.removeEventListener("click", resetVideoHandler);
     player.destroy();
     console.log("removePlayer");
   };

   for (let i = 0; i < resetVideo.length; i++) {
     resetVideo[i].addEventListener("click", handler);
   }
 }

This worked also: https://jsfiddle.net/gx163tzd/2/

 function createResetHandler(player) {
    const resetVideo = document.querySelectorAll(".container.active .exit");

    const handler = function resetVideoHandler() {
      for (let i = 0; i < resetVideo.length; i++) {
        resetVideo[i].removeEventListener("click", resetVideoHandler);
        player.destroy();
        console.log("removePlayer");
      }
    }

    for (let i = 0; i < resetVideo.length; i++) {
      resetVideo[i].addEventListener("click", handler);
    }
  }
7

One reason to not declare a function inside a loop is because JS will have to build and then destroy the function on each loop. That is unnecessarily wasteful.

And as it says, it is referring to variables from a different scope. That can be confusing.

1 Like
8

It’s just a warning coming from JSHint, I wouldn’t worry about it (issue). ESLint doesn’t seem to warn about it.

I really struggle to understand your attempts.

  1. Using the handler inside its own declaration.

  2. Using the index outside the loop.

  3. Removing the handler before using it.

  4. Using the index outside the loop.

None of that makes any sense.

Go back to using forEach, I really see no reason not to use forEach.

Or declaring resetVideoHandler before the loop should get rid of the warning as well. I would still suggest you use the options object for the removal when possible.

function createResetHandler(player) {
  function resetVideoHandler() {
    player.destroy();
  }

  const resetVideo = document.querySelectorAll(".container.active .exit");

  for (let i = 0; i < resetVideo.length; i++) {
    resetVideo[i].addEventListener('click', resetVideoHandler, {
      once: true
    });
  }
}

BTW, this is a duplicate thread. I closed the other one and be careful you have already been warned about this.

1 Like
9

How would this be written using removeEventListener instead of once:true?

function createResetHandler(player) {
  function resetVideoHandler() {
    player.destroy();
    console.log("removePlayer");
  }

  const resetVideo = document.querySelectorAll(".container.active .exit");

  for (let i = 0; i < resetVideo.length; i++) {
    resetVideo[i].addEventListener('click', resetVideoHandler, {
      once: true
    });
  }
}
10

I guess you can call removeEventListener inside the resetVideoHandler function using this as the element target.

11

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.