fix(scope-manager): reconcile type-value distinction for implicit globals across libs #11715

lilnasy · 2025-10-23T16:34:04Z

PR Checklist

Addresses an existing open issue: fixes Scope Manager fails to pick up references to implicit globals #11714
That issue was marked as accepting prs
Steps in Contributing were taken

Overview

To fix the issue, implicit variables are mutated as the global scope is being populated.

I admittedly did not spend much time thinking of a proper solution. Though it was tricky to pinpoint, the fix is simple. I imagine the "correct" location for the fix is a matter of maintainer preference.

Direct commits from maintainers welcome.

typescript-eslint · 2025-10-23T16:34:07Z

Thanks for the PR, @lilnasy!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint.

netlify · 2025-10-23T16:34:09Z

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	`381f374`
🔍 Latest deploy log	https://app.netlify.com/projects/typescript-eslint/deploys/691cc5cb2055780008dd5de6
😎 Deploy Preview	https://deploy-preview-11715--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 99 (🟢 up 2 from production) Accessibility: 97 (no change from production) Best Practices: 100 (no change from production) SEO: 92 (no change from production) PWA: 80 (no change from production) View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

nx-cloud · 2025-10-23T16:35:35Z

🤖 Nx Cloud AI Fix Eligible

An automatically generated fix could have helped fix failing tasks for this run, but Self-healing CI is disabled for this workspace. Visit workspace settings to enable it and get automatic fixes in future runs.

_{To disable these notifications, a workspace admin can disable them in workspace settings.}

View your CI Pipeline Execution ↗ for commit 381f374

Command	Status	Duration	Result
`nx run-many -t lint`	❌ Failed	3m 21s	View ↗
`nx test eslint-plugin --coverage=false`	✅ Succeeded	5m 23s	View ↗
`nx run-many -t typecheck`	✅ Succeeded	1m 59s	View ↗
`nx test eslint-plugin-internal --coverage=false`	✅ Succeeded	12s	View ↗
`nx test typescript-estree --coverage=false`	✅ Succeeded	2s	View ↗
`nx run integration-tests:test`	✅ Succeeded	4s	View ↗
`nx run types:build`	✅ Succeeded	5s	View ↗
`nx run generate-configs`	✅ Succeeded	7s	View ↗
`Additional runs (29)`	✅ Succeeded	...	View ↗

☁️ Nx Cloud last updated this comment at 2025-11-18 19:27:48 UTC

codecov · 2025-10-23T17:44:01Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.55%. Comparing base (28cf803) to head (381f374).
⚠️ Report is 26 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #11715   +/-   ##
=======================================
  Coverage   90.55%   90.55%           
=======================================
  Files         522      522           
  Lines       53063    53073   +10     
  Branches     8838     8842    +4     
=======================================
+ Hits        48049    48059   +10     
- Misses       4999     5000    +1     
+ Partials       15       14    -1

Flag	Coverage Δ
unittest	`90.55% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
packages/scope-manager/src/scope/ScopeBase.ts	`96.04% <100.00%> (+0.12%)`	⬆️
.../scope-manager/src/variable/ImplicitLibVariable.ts	`87.50% <100.00%> (ø)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

bradzacher

instead of mutating the variable here -- could we instead do it higher.

eg:

In the GlobalScope class (which calls ScopeBase.defineVariable):

typescript-eslint/packages/scope-manager/src/scope/GlobalScope.ts

Lines 68 to 79 in ea2ee6b

 public defineImplicitVariable( 

 name: string, 

 options: ImplicitLibVariableOptions, 

 ): void { 

 this.defineVariable( 

 new ImplicitLibVariable(this, name, options), 

 this.set, 

 this.variables, 

 null, 

 null, 

 ); 

 }

Or in the Referencer class (which calls GlobalScope.defineImplicitVariable):

typescript-eslint/packages/scope-manager/src/referencer/Referencer.ts

Lines 72 to 75 in ea2ee6b

 // This loop is guaranteed to see each included lib exactly once 

 for (const [name, variable] of lib.variables) { 

 globalScope.defineImplicitVariable(name, variable); 

 }

I think the latter would be better as we could pre-aggregate the variable definitions to deduplicate and merge them before even calling defineImplicitVariable.

lilnasy · 2025-11-08T03:49:08Z

@bradzacher I'm thinking it would be even better to go one level higher.

The first year-wise occurrence of the globals I glimpsed at seems to always be TYPE_VALUE. If that holds true for all, we could simply ensure the iteration is in chronological order at runtime and it would work. The test could be made more comprehensive to prevent regressions.

lilnasy · 2025-11-10T18:31:59Z

I have a branch where I flatten the lib references at codegen time instead of at runtime, fixing the bug right there. It increases the package size a little but reduces ScopeManager instantiation time.

I can move forward with whichever approach maintainers prefer.

- Part of #14827 - Fixes the bug noticed in a review in #14890 (comment) - Originally reported as typescript-eslint/typescript-eslint#11714 - Originally fixed in typescript-eslint/typescript-eslint#11715 - Patching in oxlint because neither the bug nor the fix has been reviewed in the `typescript-eslint` org.

…bals across libs

bradzacher · 2025-11-24T15:02:43Z

I believe the behaviour that we will need to add as part of the eslint v10 upgrades will provide you the behaviour you want.

This behaviour will resolve all references to globals as part of the analysis rather than leaving them all open.

JoshuaKGoldberg · 2025-11-28T17:48:49Z

@bradzacher so just confirming, is there any change we should make here? Is #11714 a no-op/wontfix given the upcoming ESLint v10 changes?

Linking for visibility: #11762 tracks updating scope-manager for compatibility with ESLint v10.

lilnasy · 2025-11-28T18:50:06Z

I believe the behaviour that we will need to add as part of the eslint v10 upgrades will provide you the behaviour you want.

I agree.

Is #11714 a no-op/wontfix given the upcoming ESLint v10 changes?

@JoshuaKGoldberg I think that depends on what you meant in the comment here: #11762 (comment). Could you clarify? Can @typescript-eslint/scope-manager remain compatible with both ESLint v8 and v10?

JoshuaKGoldberg · 2025-11-28T19:34:28Z

We haven't looked deeply into it yet, but I really hope so. Dropping support for v8 and v9 would be a breaking change that we're not ready to release a new major version for anytime soon.

bradzacher · 2025-11-29T04:34:06Z

Well technically by being compatible with v10 we're also compatible with v8/v9 - cos eslint will just have nothing to do.

lilnasy · 2025-11-29T06:12:50Z

Thanks, I thought that might be the case. Fixing this as part of ESLint 10 compat seems appropriate. Oxlint will be able to remove the patch we made to @typescript-eslint/scope-manager then (if it happens before our rust reimplementation).

) - Part of oxc-project#14827 - Fixes the bug noticed in a review in oxc-project#14890 (comment) - Originally reported as typescript-eslint/typescript-eslint#11714 - Originally fixed in typescript-eslint/typescript-eslint#11715 - Patching in oxlint because neither the bug nor the fix has been reviewed in the `typescript-eslint` org.

lilnasy changed the title ~~fix(scope manager): reconcile type-value distinction for implicit globals across libs~~ fix(scope-manager): reconcile type-value distinction for implicit globals across libs Oct 23, 2025

lilnasy force-pushed the main branch from dfc9368 to cae9f5a Compare November 1, 2025 21:28

lilnasy mentioned this pull request Nov 1, 2025

fix(linter): patch @typescript-eslint/scope manager oxc-project/oxc#15214

Merged

bradzacher requested changes Nov 4, 2025

View reviewed changes

lilnasy force-pushed the main branch from 191576b to b3876a9 Compare November 8, 2025 02:20

lilnasy force-pushed the main branch from b3876a9 to d78359d Compare November 10, 2025 18:22

lilnasy added 2 commits November 19, 2025 00:45

fix(scope manager): reconcile type-value distinction for implicit glo…

7577887

…bals across libs

fix test failures

381f374

lilnasy force-pushed the main branch from d78359d to 381f374 Compare November 18, 2025 19:15

JoshuaKGoldberg requested a review from bradzacher November 24, 2025 13:58

JoshuaKGoldberg added the triage Waiting for team members to take a look label Nov 28, 2025

lilnasy closed this Nov 29, 2025

	public defineImplicitVariable(
	name: string,
	options: ImplicitLibVariableOptions,
	): void {
	this.defineVariable(
	new ImplicitLibVariable(this, name, options),
	this.set,
	this.variables,
	null,
	null,
	);
	}

	// This loop is guaranteed to see each included lib exactly once
	for (const [name, variable] of lib.variables) {
	globalScope.defineImplicitVariable(name, variable);
	}

Uh oh!

fix(scope-manager): reconcile type-value distinction for implicit globals across libs #11715

fix(scope-manager): reconcile type-value distinction for implicit globals across libs #11715

Conversation

lilnasy commented Oct 23, 2025

PR Checklist

Overview

Uh oh!

typescript-eslint bot commented Oct 23, 2025

Uh oh!

netlify bot commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for typescript-eslint ready!

Uh oh!

nx-cloud bot commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🤖 Nx Cloud AI Fix Eligible

Uh oh!

codecov bot commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bradzacher left a comment

Choose a reason for hiding this comment

Uh oh!

lilnasy commented Nov 8, 2025

Uh oh!

lilnasy commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bradzacher commented Nov 24, 2025

Uh oh!

JoshuaKGoldberg commented Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lilnasy commented Nov 28, 2025

Uh oh!

JoshuaKGoldberg commented Nov 28, 2025

Uh oh!

bradzacher commented Nov 29, 2025

Uh oh!

lilnasy commented Nov 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

netlify bot commented Oct 23, 2025 •

edited

Loading

nx-cloud bot commented Oct 23, 2025 •

edited

Loading

codecov bot commented Oct 23, 2025 •

edited

Loading

lilnasy commented Nov 10, 2025 •

edited

Loading

JoshuaKGoldberg commented Nov 28, 2025 •

edited

Loading