-
-
Notifications
You must be signed in to change notification settings - Fork 715
fix(oxlint): resolving JS plugin failing when extends is used
#14556
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
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. |
32cceaf to
afdab8e
Compare
CodSpeed Performance ReportMerging #14556 will not alter performanceComparing Summary
Footnotes
|
3c97e2d to
444cf06
Compare
extends is used
444cf06 to
34782ff
Compare
69c8965 to
bc14d64
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 fixes an issue where JS plugin loading fails when configuration files use the extends feature. The fix involves changing how external plugins are handled internally to track both their specifiers and the directory containing the configuration file that references them.
Key changes:
- Removed the default null value for jsPlugins in schema files to fix serialization issues
- Modified external plugin storage to include configuration directory paths alongside plugin specifiers
- Updated plugin resolution logic to use the correct directory for relative plugin paths
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tasks/website/src/linter/snapshots/schema_markdown.snap | Removes default null value from jsPlugins documentation |
| npm/oxlint/configuration_schema.json | Removes default null value from jsPlugins schema |
| crates/oxc_linter/src/snapshots/schema_json.snap | Removes default null value from jsPlugins schema |
| crates/oxc_linter/src/config/oxlintrc.rs | Adds custom serialization/deserialization and path tracking for external plugins |
| crates/oxc_linter/src/config/overrides.rs | Adds custom serialization/deserialization for external plugins in overrides |
| crates/oxc_linter/src/config/config_builder.rs | Updates plugin loading logic to use configuration directory paths |
| apps/oxlint/test/fixtures/custom_plugin_nested_config/* | Adds test fixtures for nested configuration scenario |
| apps/oxlint/test/e2e.test.ts | Adds test case for custom plugin with nested config |
| apps/oxlint/src/snapshots/* | Updates snapshots to reflect removal of default null values |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
bc14d64 to
ed624c5
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.
I'm a bit confused by the Option<PathBuf> - please see comments below.
I'm also unsure if we've introduced the possibility of same plugin getting loaded twice, because ("/foo/bar", "./plugin.js") and ("/foo/bar/qux", "../plugin.js") resolve to the same path, but do not hash the same.
But even if the latter is possible, it's an edge case, so I think we could merge this and deal with that oddity in a later PR.
ed624c5 to
26bac00
Compare
let me add a test in a follow up PR. i've got no idea. EDIT: looks like we handle this already, but i added a test in the follow up to prevent any regressions
|
6c30cc9 to
55becc5
Compare
Merge activity
|
55becc5 to
28e76ec
Compare

No description provided.