Skip to content

fix(oxfmt): Skip vite.config.ts which fails to import#20326

Merged
graphite-app[bot] merged 1 commit intomainfrom
03-13-fix_oxfmt_skip_vite.config.ts_which_fails_to_import
Mar 13, 2026
Merged

fix(oxfmt): Skip vite.config.ts which fails to import#20326
graphite-app[bot] merged 1 commit intomainfrom
03-13-fix_oxfmt_skip_vite.config.ts_which_fails_to_import

Conversation

@leaysgur
Copy link
Member

@leaysgur leaysgur commented Mar 13, 2026

If oxfmt.config.ts fails to load, it is the user's responsibility, so reporting it as an error is correct.

However, this does not apply to vite.config.ts, so we ignore it.
Perhaps the most common scenario is when you encounter a vite.config.ts file that has a valid export default {} within a project configured with type: commonjs. Even this alone will cause import() to fail.


Also refactored to report error message from JS.

Copilot AI review requested due to automatic review settings March 13, 2026 05:11
@github-actions github-actions bot added A-cli Area - CLI A-formatter Area - Formatter labels Mar 13, 2026
Copy link
Member Author

leaysgur commented Mar 13, 2026


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent changes, fast-track this PR to the front of 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.

Copy link
Contributor

Copilot AI left a 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 improves Oxfmt’s JS/TS config handling by (1) skipping auto-discovered vite.config.ts files that fail to import/evaluate and (2) surfacing more actionable error details when config loading fails.

Changes:

  • Skip auto-discovered vite.config.ts when load_js_config() errors, continuing config search instead of failing.
  • Preserve and propagate the underlying NAPI/JS error “reason” into Rust-side config load errors.
  • Simplify JS-side thrown error messages and update CLI snapshot expectations accordingly.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
apps/oxfmt/src/core/config.rs Skip failing vite.config.ts during auto-discovery; include underlying JS error text in config load errors.
apps/oxfmt/src/core/js_config.rs Return NAPI error reason instead of a generic "failed to load" string.
apps/oxfmt/src-js/cli/js_config.ts Standardize JS error messages (path now provided by Rust-side context).
apps/oxfmt/test/cli/js_config/snapshots/js_config.test.ts.snap Update snapshots to reflect the more detailed error output.

You can also share your feedback on Copilot code review. Take the survey.

@leaysgur leaysgur added the 0-merge Merge with Graphite Merge Queue label Mar 13, 2026
Copy link
Member Author

leaysgur commented Mar 13, 2026

Merge activity

@leaysgur leaysgur removed the 0-merge Merge with Graphite Merge Queue label Mar 13, 2026
graphite-app bot pushed a commit that referenced this pull request Mar 13, 2026
If oxfmt.config.ts fails to load, it is the user's responsibility, so reporting it as an error is correct.
However, this does not apply to vite.config.ts, so we ignore it.

Also refactored to report error message from JS.
@graphite-app graphite-app bot force-pushed the 03-13-fix_oxfmt_skip_vite.config.ts_which_fails_to_import branch from a382562 to d345ed0 Compare March 13, 2026 05:24
If oxfmt.config.ts fails to load, it is the user's responsibility, so reporting it as an error is correct.
However, this does not apply to vite.config.ts, so we ignore it.

Also refactored to report error message from JS.
@leaysgur leaysgur force-pushed the 03-13-fix_oxfmt_skip_vite.config.ts_which_fails_to_import branch from d345ed0 to 68fb0d0 Compare March 13, 2026 05:25
@leaysgur leaysgur added the 0-merge Merge with Graphite Merge Queue label Mar 13, 2026
@graphite-app graphite-app bot merged commit 68fb0d0 into main Mar 13, 2026
19 checks passed
@graphite-app graphite-app bot deleted the 03-13-fix_oxfmt_skip_vite.config.ts_which_fails_to_import branch March 13, 2026 05:37
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Mar 13, 2026
leaysgur added a commit that referenced this pull request Mar 16, 2026
graphite-app bot pushed a commit that referenced this pull request Mar 16, 2026
Current status:

- oxfmt: #20326 was merged
- oxlint: After discussion in #20327, it was decided NOT to merge

First, correct this inconsistency.

Since #20326 includes a change that displays errors from the JS side as-is, we will keep that part.
camc314 pushed a commit that referenced this pull request Mar 16, 2026
# Oxlint
### 🚀 Features

- c95951f linter/plugins: Implement `sourceCode.markVariableAsUsed` (#20357) (overlookmotel)
- 7a2a7d0 linter: Implement `n/handle-callback-err` rule (#19616) (Mikhail Baev)

### 🐛 Bug Fixes

- f8fbd6e linter/plugins: Remove `hashbang` property from AST (#20365) (overlookmotel)
- 6eb5b01 linter/prefer-await-to-then: Ignore Promise static methods (#20347) (camc314)
- a4b61f7 linter: Remove `defineConfig` check (#20308) (camc314)
- 3ad7f53 linter/explicit-module-boundary-types: False positive with satisfies expr (#20309) (camc314)
- f547401 linter/no-unused-private-class-members: Treat switch discriminants as read (#20307) (camc314)
- 1c07b3b diagnostics: Handle `WouldBlock` in stdout writes to prevent panic (#20295) (Boshen)

### ⚡ Performance

- e4f7248 linter: Remove unnecessary clone of owned String in drain loop (#20388) (Boshen)
- 4a67f1d linter: Eliminate Vec allocation in disable directive matching (#20387) (Boshen)
- 618a598 linter/plugins: Add fast path for files with no comments (#20366) (overlookmotel)
- b0125c5 linter/plugins: Deserialize comments without AST (#20364) (overlookmotel)
- 9cd612f linter/plugins: Recycle comment objects (#20362) (overlookmotel)
- bf442f8 linter/plugins: Cheaper `Token` creation (#20360) (overlookmotel)
- 5474d0a semantic: V8-style walk-up reference resolution (#20292) (Boshen)
- 7946eba linter/plugins: Avoid arguments spread and temp array when merging (#20318) (overlookmotel)
- fc7cf8a linter/plugins: Pre-define less CFG merger functions (#20317) (overlookmotel)
- 3b9eb28 linter/plugins: Streamline getting/creating visit fn mergers (#20319) (overlookmotel)
- f04e850 linter/plugins: Inline binary search functions into call sites (#20312) (overlookmotel)
- fe24afe linter/plugins: Apply replace globals TSDown plugin to JS files (#20305) (overlookmotel)
- 77cdacc linter/plugins: Use array buffer views for tokens (#20301) (overlookmotel)
- 910c941 linter/plugins: Reorder branches in `getTokenByRangeStart` (#20296) (overlookmotel)
- af7674c linter/tokens: Avoid extra token value allocation (#20013) (camc314)

### 📚 Documentation

- 24490b5 linter: Improve formatting for 80ish rules' docs. (#20411) (connorshea)
- 3383523 linter: Improve `--tsconfig` flag docs (#20342) (camc314)
# Oxfmt
### 🚀 Features

- d22c443 oxfmt: Export `OxfmtConfig` type (#20275) (leaysgur)
- a11ecff oxfmt/lsp: Respect `angular` language id as `.component.html` file (#20242) (Sysix)

### 🐛 Bug Fixes

- ce65099 formatter: Preserve parentheses around as expression before private field access (#20419) (bab)
- f908742 oxfmt: Revert #20326 partially (#20413) (leaysgur)
- 4ef93ea formatter: Honor trailing ignore comments after list separators (#19925) (Andreas Lubbe)
- 68fb0d0 oxfmt: Skip vite.config.ts which fails to import (#20326) (leaysgur)
- 88ee826 oxfmt: Handle literalline for script-in-vue (#20130) (leaysgur)
- 1c07b3b diagnostics: Handle `WouldBlock` in stdout writes to prevent panic (#20295) (Boshen)

Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cli Area - CLI A-formatter Area - Formatter C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants