Skip to content

fix(oxlint): Skip vite.config.ts which fails to import#20327

Closed
leaysgur wants to merge 1 commit into03-13-fix_oxlint_oxfmt_suppress_stdout_while_import_jsconfig_for_lspfrom
03-13-fix_oxlint_skip_vite.config.ts_which_fails_to_import
Closed

fix(oxlint): Skip vite.config.ts which fails to import#20327
leaysgur wants to merge 1 commit into03-13-fix_oxlint_oxfmt_suppress_stdout_while_import_jsconfig_for_lspfrom
03-13-fix_oxlint_skip_vite.config.ts_which_fails_to_import

Conversation

@leaysgur
Copy link
Member

@leaysgur leaysgur commented Mar 13, 2026

The same as #20326

(It's fine to release this as scheduled one)

@leaysgur leaysgur requested a review from camc314 as a code owner March 13, 2026 05:11
@github-actions github-actions bot added A-linter Area - Linter A-cli Area - CLI labels Mar 13, 2026
Copy link
Member Author

leaysgur commented Mar 13, 2026

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.
Learn more


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.

@leaysgur leaysgur changed the base branch from 03-13-fix_oxfmt_skip_vite.config.ts_which_fails_to_import to graphite-base/20327 March 13, 2026 05:20
@leaysgur leaysgur force-pushed the graphite-base/20327 branch from 7a02725 to e1db09d Compare March 13, 2026 05:20
@leaysgur leaysgur force-pushed the 03-13-fix_oxlint_skip_vite.config.ts_which_fails_to_import branch from 0225bcd to 08a5dd5 Compare March 13, 2026 05:21
@leaysgur leaysgur changed the base branch from graphite-base/20327 to 03-13-fix_oxlint_oxfmt_suppress_stdout_while_import_jsconfig_for_lsp March 13, 2026 05:21
@github-actions github-actions bot added the A-linter-plugins Area - Linter JS plugins label Mar 13, 2026
Copy link
Contributor

@camc314 camc314 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced this is a good idea? Users won't see the debug log unless they have RUST_LOG=debug (basically no one), and they're going to see lint run fine, but not use their config

@leaysgur
Copy link
Member Author

Thanks for the review. I may be a bit too defensive here.

Let's keep the current behavior (error) for now and see if it becomes a real issue in practice.

(Will revert the oxfmt side later.)

@leaysgur leaysgur closed this Mar 13, 2026
@leaysgur leaysgur deleted the 03-13-fix_oxlint_skip_vite.config.ts_which_fails_to_import branch March 13, 2026 12:18
@leaysgur
Copy link
Member Author

Just in case, I'll add more likely scenarios for the record.

  • If __dirname is used in vite.config.ts even though it's a type: module project
  • Vite can transparently handle CJS/ESM via vite.resolveConfig(), so no error occurs
  • However, since we are using pure import(), this will result in an error

Users will see an error message to that effect, but they might wonder why.

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.
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 A-linter Area - Linter A-linter-plugins Area - Linter JS plugins C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants