eslint-scope icon indicating copy to clipboard operation
eslint-scope copied to clipboard

Question: how come function references on the global scope are not closed

Open bradzacher opened this issue 5 years ago • 6 comments

https://github.com/eslint/eslint-scope/blob/dbddf14d5771b21b5da704213e4508c660ca1c64/tests/references.js#L211-L263

I'm working on trying to understand this codebase better. But I can't understand why it works this way:

function a() {}
a();

To me it seems clear that the reference that's created as part of the call should resolve directly to the function declaration, but it's not - it remains as an unresolved reference that's added to the global scope's through list.

This would cause the no-unused-vars ESLint rule to error on the function declaration, were it not for this code in the linter.

It looks like it works seemingly by chance? ESLint augments the global scope with more variables, and then forcefully resolves any through references against the global variable set, which includes the function declaration as well as the augmented variables.

Is anyone able to explain why this works this way?

Or a better question - when closing the global scope, why doesn't it attempt to resolve all through references against the variables defined in the global scope?

bradzacher avatar May 06 '20 05:05 bradzacher

Thank you for your question.

....but, I'm not sure why it's so. I guess that it's as-is from the original package escope.

Maybe... it couldn't assume declarations in global scope make variables, because browsers may ignore declarations.

var top = function() {}
top() // ERROR: 'top' is not a function

data:text/html,<script>var top = function(){}; top();</script>

mysticatea avatar May 15 '20 12:05 mysticatea

Interesting. Is this something that should be fixed?

I believe at least in sourceType: module mode, this shouldn't work this way, right?

bradzacher avatar May 15 '20 17:05 bradzacher

Yes, data:text/html,<script type="module">var top = function(){}; top();</script> works fine.

Also, new knownGlobals: string[] option may be good to make variables and resolve references.

mysticatea avatar May 16 '20 05:05 mysticatea

is it only var declarations that shouldn't be closed?

Doing some testing it looks like every single other case works as expected and throws this error: Identifier 'top' has already been declared

It's only var top that is ignored and throws top is not a function

bradzacher avatar May 17 '20 21:05 bradzacher

@mysticatea what's the verdict here? Is there something to fix or no?

nzakas avatar Jan 26 '21 02:01 nzakas

@mysticatea is this something you want to address?

nzakas avatar Apr 30 '21 00:04 nzakas