Home > front end >  Functions declared within loops referencing an outer scoped variable may lead to confusing
Functions declared within loops referencing an outer scoped variable may lead to confusing

Time:11-30

This is a jshint warning question.How can I solve this problem?

var comment_btn=document.querySelector('.comment_button');
var comment_ul=document.querySelector('.comment_ul');
var comment_text=document.querySelector('#comment');

comment_btn.onclick = function(){
    var comment_li = document.createElement('li');
    comment_li.className = 'comment_li';
    if(comment_text.value != '') {
        comment_li.innerHTML = comment_text.value   "<a class='comment_a' href='javascript:;'>Delete</a>";
        comment_ul.insertBefore(comment_li,comment_ul.children[0]);
        var del = document.querySelectorAll('.comment_a');
        for (var i = 0; i < del.length; i  ) {
            del[i].onclick = function() {
                comment_ul.removeChild(this.parentNode);
            };
        }
    }
    else {
        alert('Please input!');
    }
};

Warning:
Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (comment_ul) (W083)jshint(W083)

I really can't think of a solution,please help me.

CodePudding user response:

Every time you add a new li, you are selecting EVERY delete anchor and adding another click event to it.

You should not be looping at all. You should just be selecting the anchor in the li you create and attach the event to that.

const comment_ul = document.querySelector("ul");
const comment_text = document.querySelector("#comment_text");


comment_btn.addEventListener('click', function() {
  if (!comment_text.value.length) {
    alert('Please input!');
    return;
  }

  var comment_li = document.createElement('li');

  comment_li.className = 'comment_li';

  comment_li.innerHTML = comment_text.value   "<a class='comment_a' href='javascript:;'>Delete</a>";

  comment_ul.insertBefore(comment_li, comment_ul.children[0]);

  var del = comment_li.querySelector('.comment_a');
  del.addEventListener('click', function(evt) {
    evt.preventDefault();
    comment_ul.removeChild(this.parentNode);
  });

});
<ul></ul>

<input type="text" id="comment_text" />
<button id="comment_btn">Add</button>

CodePudding user response:

The function you assign to onclick inside the loop doesn't care about any value that changes during the course of the loop. It only uses comment_url (defined before the loop) and this.

Create the function before the loop and assign it to a variable.

Copy it to onclick inside the loop.

CodePudding user response:

This is a warning which will appear if you have a loop and you have a variable declared with var, which can sometimes cause problems. Assuming you aren't reassigning comment_ul anywhere, the problems aren't relevant to your situation, so all you need to do is make the linter happy.

One approach would be to declare the variable with const instead (best to declare variables with const when possible, and let when not).

const comment_ul = document.querySelector('.comment_ul');

Another would be to use forEach instead of for(....

That said, your code looks buggy at the moment - you're adding an event listener to every button every time comment_btn is clicked. Instead of doing that, consider adding only a single listener, ever - use event delegation. If the clicked element or one of its ancestors is a .comment_a, remove its parent.

commentContainer.addEventListener('click', (event) => {
  const a = event.target.closest('.comment_a');
  if (a) {
    a.parentElement.remove();
  }
});
  • Related