refactor(oxfmt): allow missing lint config#20243
refactor(oxfmt): allow missing lint config#20243camc314 wants to merge 1 commit intoc/03-11-oxlint_extract_vite_lint_config_in_js_loaderfrom
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
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.
Pull request overview
Refactors oxfmt’s Vite config handling so that a vite.config.ts without an fmt field no longer errors, and instead falls back to the default oxfmt configuration (improving usability for Vite default setups).
Changes:
- Updates CLI test and snapshot to expect success when
vite.config.tshas nofmtfield. - Moves Vite
.fmtextraction from Rust config resolution into the JS config loader, defaulting to{}when missing. - Removes the Rust-side error path that required
fmtto be present invite.config.ts.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| apps/oxfmt/test/cli/vite_config/vite_config.test.ts | Renames the test to assert fallback behavior when fmt is missing. |
| apps/oxfmt/test/cli/vite_config/snapshots/vite_config.test.ts.snap | Updates snapshot to reflect exit code/output for successful fallback behavior. |
| apps/oxfmt/src/core/config.rs | Removes Rust-side Vite .fmt field extraction/error path. |
| apps/oxfmt/src-js/cli/js_config.ts | Adds Vite-specific .fmt extraction with {} fallback and validation. |
You can also share your feedback on Copilot code review. Take the survey.
| .map_err(|_| { | ||
| format!( | ||
| "{}\nEnsure the file has a valid default export of a JSON-serializable configuration object.", | ||
| path.display() | ||
| ) |
There was a problem hiding this comment.
With the Vite .fmt validation now happening in the JS loader, any JS-side error details are currently lost: create_js_config_loader maps all JS errors to a generic string and this call site also discards the error (map_err(|_| ...)). This will make misconfigurations like a non-object fmt field hard to diagnose. Consider preserving and surfacing the underlying error message (e.g., map to err.to_string() in the loader and include it in this map_err).
79f3537 to
38ed0bb
Compare
38ed0bb to
07d2329
Compare
without this, users using vite with the default config encounter an error when using oxfmt (same as the PR above this)