Linters used to complain any time you defined a function in a loop b/c you weren't being DRY. (I'm kinda surprised that JSHint doesn't, but JSLint doesn't appear to think this is an error any more either!)
Like older versions of linters, I'd argue that [re]defining the same function within each iteration of your loop is a bad practice any time. Because the function declaration happens within a loop, each separate function's logic is identical to the others'. That means the code can't be DRY. It also allows complex scoping setups like yours!
link.addEventListener("click", function (e) {
updateForm(link.team);
});
Normally you should pull that function out of the loop and define a single reference to it outside of the loop.
BUT... You're passing variables into your function through the backdoor with some, um, creative closure use, which is a more nuanced error [that linters apparently still care about].
Here's one way you could fix the lint that makes the importance of scoping in your initial code explicit (though keep reading!).
/*jshint esversion: 6 */
/*global updateForm, teams, table */
// Note that this function returns a function containing a closure
// that maintains one `link` per call.
function myListenerFactory(link) {
return function (e) {
updateForm(link.team);
};
}
for (let i = 0; i < teams.length; i++) {
let row = table.insertRow(-1);
//removed code that made cells under the row
let link = document.createElement("a");
link.setAttribute("href","#form");
link.team = teams[i];
// Use the same function each time. Note that it's called IMMEDIATELY,
// returning a new function for each iteration.
link.addEventListener("click", myListenerFactory(link)()); // <<< Same function reused in the loop
link.textContent = teams[i].name;
//code that adds the link and other elements into the cells of the table
}
That lints on jshint.com.
This code maintains the closure from the original by using a factory and returns one scope-wrapped function per loop iteration.
It's nice in that it clearly shows the protected scope is intentional -- you wanted a new function per iteration because scope was important -- and also limits what's in that protected scope to link and nothing else, but otherwise has no benefit.
This is why TJ says "your perfectly-valid code using link in a callback" -- your code depends on the closure, so each redefinition is, technically, different, though only by its scope. It is DRY from the start. It just doesn't look like it. It doesn't look like what it does, imo.
(That we're counting on variable hoisting to pass link to our function should be all the code smell you need to know there's probably a better solution.)
A better solution
So I'd want to push on "perfectly-valid" a bit, and I think TJ does too, by saying to avoid expando properties. It's not the easiest code to grok (nor is the lint-passing alternative, above), and that's what the linter has identified.
Let's try to find a better option.
I'm pretty sure you can get the reference you want from the e.target (unless you're supporting IE 8 or lower). Then we don't have to pass around the link at all. We can derive it from our event.
/*jshint esversion: 6 */
/*global updateForm, teams, table */
function myEventHandler(e) {
// No need to backdoor `link` when we can derive it from `e`!
updateForm(e.target.team);
}
for (let i = 0; i < teams.length; i++) {
let row = table.insertRow(-1);
//removed code that made cells under the row
let link = document.createElement("a");
link.setAttribute("href","#form");
link.team = teams[i];
// Use the same function each time
link.addEventListener("click", myEventHandler);
link.textContent = teams[i].name;
//code that adds the link and other elements into the cells of the table
}
And boom. No wacky scope, no repeated function declaration. Perfect.
Expando warning
Again, TJ's warning about avoiding "expando properties" is a very good one. Expando properties are nonstandard ones you smash onto a DOM element after the object is initialized.
These properties can be edited and/or removed without warning by other libraries fairly commonly. You'd probably be better off using data attributes...
... or, my preference, having a lookup table that's accessed by adding unique element ids to each of your links.
That is, I'm not in love with TJ's solution of keeping your initial scoping-dependent hipstery ;^D, but it works too.
Make sense?