eslevels icon indicating copy to clipboard operation
eslevels copied to clipboard

Level detection incorrect for named function expressions

Open getify opened this issue 7 years ago • 6 comments

function foo() {
	var y = 3;
	var w = function baz(){
		var p = 2;
	};
}

In that code, the identifier baz should be in the same scope as var p (aka, level 2), since identifiers of named function expressions are always in their own function's scope (and read-only), not in the containing scope.

However, I tried this on the demo page (http://mazurov.github.io/eslevels-demo/) and it's incorrectly identifying it in the outer scope (level 1), the same as var y.

screen shot 2017-11-29 at 8 44 31 am

getify avatar Nov 29 '17 14:11 getify

The names of NFEs are in an intermediate scope, not the scope of their bodies.

top-level -> foo body/params -> special scope for just the baz name -> baz body/params

michaelficarra avatar Nov 29 '17 16:11 michaelficarra

@michaelficarra OK, that is technically true by spec.

But while you can construct obscure scenarios where the differences in those scopes matter, in the vast majority of cases, I believe baz would be interpreted by readers/authors of code as "in the scope of itself (its own function)". I can't really imagine practical scenarios where es-levels would need to represent an intermediate scope, especially since there's literally only one identifier in that scope.

screen shot 2017-11-29 at 1 29 46 pm

Importantly, it's most definitely not in the outer scope, and suggesting that it is -- as es-levels does right now -- is very much misleading/incorrect.

getify avatar Nov 29 '17 19:11 getify

@getify It is important to understand this distinction when analysing your second example (copied/simplified below)

var a = function baz() { var baz; return baz; };
a();

If these variables were both declared in the same scope, the function would return itself. But instead, the variable declared in the body shadows the one in the function name scope. The distinction is important to average users.

michaelficarra avatar Nov 29 '17 21:11 michaelficarra

@michaelficarra I don't run across people doing that kind of shadowing very often, but that's also because most people rarely use named function expressions (even though I'm always preaching that they should!).

I'm not saying the information/distinction is unimportant, just that most of the time that extra level (and different color) would probably be irrelevant.

Maybe a compromise would be that es-levels would identify (and colorize) it as belonging to the inner scope UNLESS it's shadowed, in which case it gets a different color to make it clear in those cases?

getify avatar Nov 29 '17 21:11 getify

@getify Telling a partial story about the scope might be confusing, as users might be surprised that changing variable names makes scopes appear/disappear.

michaelficarra avatar Nov 29 '17 21:11 michaelficarra

I would like to re-emphasize a point I made earlier in this thread:

Importantly, it's most definitely not in the outer scope, and suggesting that it is -- as es-levels does right now -- is very much misleading/incorrect.

That may be lost in the weeds here with the esoteric discussion of whether there needs to be a color for the intermediate scope. I still disagree with @michaelficarra on that, but there should be no question that the current behavior -- implying it's part of the enclosing scope -- is most definitely wrong, and should be fixed.

I don't want the intermediate color (except in the exceptional case of shadowing a function expression's name), and I think that will confuse more users... but it would still be better than what it does now, which is flatly inaccurate.

getify avatar Feb 23 '18 12:02 getify