0
(function() {
  var tasks = /* Some awesome content stored here */
  var render = '';
  for(var x = 0; x < questions.length; x+= 1) {
  render += '<p onclick="changeText(' + x +')">question[i]</p>'
 }
    function changeText(x) {
      // some fancy stuff happens here with a certain x
    }
})();

Why do I get the error below when trying to use an IIFE but this works just fine if I get rid of the IIFE? Isn't my changeText() in the same scope as everything else?

How do I fix this?

Uncaught ReferenceError: changeText is not defined
    at HTMLElement.onclick

1 Answers1

-1

A function declaration creates a variable (with a value that is that function) in the current scope.

onclick attributes only have access to their own scope and the global scope.

The function inside the IIFE is out of scope.

The quick and dirty solution is to make changeText a global.

The clean solution, OTOH:

This is one more reason that you should avoid using onclick attributes.

Don't generate HTML by smashing strings together. Generate your DOM directly and use addEventListener.

for(var x = 0; x < questions.length; x+= 1) {
    var p = document.createElement("p");
    p.appendChild(document.createTextNode("question[i]");
    p.setAttribute("data-x", x);
    p.addEventListener("click", function() { changeText(this.dataset.x); });  
    something.appendChild(p);
 }

NB: Paragraphs are not designed to be interactive controls. Adding click events to them makes them inaccessible to people who aren't (or can't) using a pointing device. You should use something designed to be clicked on (like a <button>) instead.

Quentin
  • 914,110
  • 126
  • 1,211
  • 1,335
  • _"The clean solution"_: I disagree. You shouldn't create that click handler in the loop. – Cerbrus Sep 26 '17 at 09:18
  • @Cerbrus Can you please explain why is it wrong to reate that click handler in the loop? Is this a matter of aesthetics or does it affect your code in anyway buy doing something like this? –  Sep 26 '17 at 09:40
  • @user19380947: If you've got 50 questions, you're creating 50 duplicates of that function. Just create that function _once_, outside of the loop, and pass it to `addEventListener`. – Cerbrus Sep 26 '17 at 09:42
  • @Cerbrus If you put it outside of the loop, only one event listener will be created which is for the last paragraph. Anyway to fix that and create a listener for each paragraph that will allow you to change the function without being repetitive like in the code above? –  Sep 26 '17 at 11:32
  • This: `function() { changeText(this.dataset.x); }` has to be moved out of the loop. Not the whole `addEventListener` call. – Cerbrus Sep 26 '17 at 11:42