-
-
Notifications
You must be signed in to change notification settings - Fork 714
perf(linter/plugins): use singleton object for ScopeManager
#15332
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
perf(linter/plugins): use singleton object for ScopeManager
#15332
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the ScopeManager implementation from a class-based approach to a singleton frozen object pattern, improving performance and memory efficiency.
Key Changes:
- Converted
ScopeManagerfrom a class to a frozen singleton object (SCOPE_MANAGER) - Moved scope manager state management from
source_code.tstoscope.tsfor better encapsulation - Implemented lazy initialization of the underlying TS-ESLint scope manager
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| apps/oxlint/src-js/plugins/source_code.ts | Updated imports and removed local scope manager state, delegating to the new SCOPE_MANAGER singleton |
| apps/oxlint/src-js/plugins/scope.ts | Refactored ScopeManager class to a frozen singleton object with lazy initialization and added reset functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
75a6a13 to
728ea43
Compare
Merge activity
|
Follow-on after #14890. That PR implemented `ScopeManager` class. Instead of a class, which an instance of is created for each file, use a singleton object, with the underlying TS-ESLint scope manager stored in a top-level var. Methods/getters of `SCOPE_MANAGER` lazily initialize the TS-ESLint scope manager as required, and cache it. This pattern is the same as we use for `SourceCode`, enabled by the fact that only 1 file is linted at a time. This has several advantages: 1. Reduce object creation - less GC pressure. 2. Property accesses don't need to go up prototype chain, as they would for instances of a class. 3. No need for private properties, which are somewhat expensive to access.
728ea43 to
4c0ba92
Compare

Follow-on after #14890.
That PR implemented
ScopeManagerclass. Instead of a class, which an instance of is created for each file, use a singleton object, with the underlying TS-ESLint scope manager stored in a top-level var. Methods/getters ofSCOPE_MANAGERlazily initialize the TS-ESLint scope manager as required, and cache it.This pattern is the same as we use for
SourceCode, enabled by the fact that only 1 file is linted at a time. This has several advantages: