Skip to content

Change Request: Track implicit globals #653

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
1 of 4 tasks
nzakas opened this issue May 14, 2025 · 4 comments · Fixed by #654
Closed
1 of 4 tasks

Change Request: Track implicit globals #653

nzakas opened this issue May 14, 2025 · 4 comments · Fixed by #654

Comments

@nzakas
Copy link
Member

nzakas commented May 14, 2025

Which packages would you like to change?

  • espree
  • eslint-scope
  • eslint-visitor-keys

What problem do you want to solve?

Split off from: eslint/eslint#19695 (comment)

Right now, implicit globals aren't actually treated as globals. Instead, they're treated as unresolved. Example:

function test() {
  implicitGlobal = 123; // creates an implicit global in JS, unresolved in eslint-scope
  implicitGlobal;  // no relationship to implicitGlobal in the previous line in eslint-scope
}

That means we aren't able to track implicit globals like we do explicit globals.

What do you think is the correct solution?

We should consider whether assignment to an implicit global should create a global variable declaration in eslint-scope.

Participation

  • I am willing to submit a pull request for this change.

Additional comments

No response

@snitin315
Copy link
Contributor

I think yes, we should create a global variable declaration in eslint-scope for implicit globals, that's how just JS works.

@snitin315 snitin315 moved this from Needs Triage to Feedback Needed in Triage May 20, 2025
@nzakas
Copy link
Member Author

nzakas commented May 20, 2025

Ping @eslint/eslint-team

@mdjermanovic
Copy link
Member

mdjermanovic commented May 20, 2025

We should consider whether assignment to an implicit global should create a global variable declaration in eslint-scope

I think it shouldn't, for several reasons:

  1. Implicit globals are a legacy feature that should be avoided (e.g., it doesn't work in strict mode), so I think there's not much value in tracking something that should ideally be removed from the code. Also, nowadays it's rare to write non-strict code, which is the only code where this can work.
  2. This would be a breaking change for consumers, including ESLint. Given the previous point, I think making a breaking change isn't justified.
  3. These variables can already be found in global scope's implicit if anyone needs them.
  4. Unlike declared variables, which are always created in the scope, these variables are created at the time when the assignment is executed, so they may or may not exist. That might be the reason why they were not already tracked in the global scope.

@nzakas
Copy link
Member Author

nzakas commented May 21, 2025

  • These variables can already be found in global scope's implicit if anyone needs them.

I didn't realize these were tracked separately. In that case, I agree, we don't need to make any changes.

We should document this in the README for safekeeping.

nzakas added a commit that referenced this issue May 21, 2025
@snitin315 snitin315 moved this from Feedback Needed to Implementing in Triage May 21, 2025
@github-project-automation github-project-automation bot moved this from Implementing to Complete in Triage May 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Complete
Development

Successfully merging a pull request may close this issue.

3 participants