-
-
Notifications
You must be signed in to change notification settings - Fork 648
refactor(linter): introduce ContextSubHost
for cross script block analyse
#12724
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
refactor(linter): introduce ContextSubHost
for cross script block analyse
#12724
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. |
CodSpeed Instrumentation Performance ReportMerging #12724 will not alter performanceComparing Summary
Footnotes |
bf94758
to
23e3b37
Compare
23e3b37
to
646cd76
Compare
5f79f25
to
81fac36
Compare
81fac36
to
d04f456
Compare
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 linter architecture to support multi-script block analysis by introducing ContextSubHost
. This enables proper linting of Vue files and other frameworks that contain multiple script blocks within a single file, such as <script>
and <script setup>
tags in Vue components.
- Introduces
ContextSubHost
to handle individual script blocks within a file - Modifies
ContextHost
to contain multipleContextSubHost
instances and iterate through them - Adds framework-specific options to support Vue setup script detection
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
crates/oxc_linter/src/utils/jest.rs | Updates test helper to use new ContextSubHost structure |
crates/oxc_linter/src/loader/source.rs | Adds framework options support to JavaScriptSource |
crates/oxc_linter/src/loader/partial_loader/vue.rs | Detects Vue setup scripts and applies framework options |
crates/oxc_linter/src/lib.rs | Wraps linting logic in a loop to process multiple script blocks |
crates/oxc_linter/src/frameworks.rs | Defines FrameworkOptions enum for framework-specific contexts |
crates/oxc_linter/src/context/mod.rs | Updates exports and context access methods |
crates/oxc_linter/src/context/host.rs | Major refactor introducing ContextSubHost and updating ContextHost |
Comments suppressed due to low confidence (1)
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
d04f456
to
3fdf343
Compare
3fdf343
to
6a32eeb
Compare
Merge activity
|
…nalyse (#12724) Another approach to #12541 stack. Because `vue/valid-define-emit` and other rules required the semantics of the other script block. ## The Goal: `Linter.run` should run with the complete file context. In `vue` files and others, there can be multiple Semantics / Module Records. Because `vue` requires some magic compiler rules, the linter should have access to the complete file. This will also fix the offset bug in #12758. ~~**Don't** merge both stacks without rebasing.~~ I do not like the loop part. Maybe you guys have some better ideas. --- <img width="1160" height="422" alt="benchmark results" src="https://github.com/user-attachments/assets/88702a01-eeb2-4a4f-8d80-c5236d3fa8cc" />
6a32eeb
to
077abdf
Compare
Follow-on after #12724. Small perf optimization. `Cell` is cheaper than `RefCell`, because it doesn't do runtime aliasing checks. It's preferable when the contained type is small and `Copy`, as is the case here (`usize`).
Another approach to #12541 stack.
Because
vue/valid-define-emit
and other rules required the semantics of the other script block.The Goal:
Linter.run
should run with the complete file context. Invue
files and others, there can be multiple Semantics / Module Records.Because
vue
requires some magic compiler rules, the linter should have access to the complete file.This will also fix the offset bug in #12758.
Don't merge both stacks without rebasing.I do not like the loop part. Maybe you guys have some better ideas.