Skip to content

fix(oxlint,oxfmt): Check env.VITE_PLUS_VERSION for Vite+#20417

Open
leaysgur wants to merge 2 commits intomainfrom
03-16-fix_oxlint_oxfmt_check_env.vite_plus_version_for_vite_
Open

fix(oxlint,oxfmt): Check env.VITE_PLUS_VERSION for Vite+#20417
leaysgur wants to merge 2 commits intomainfrom
03-16-fix_oxlint_oxfmt_check_env.vite_plus_version_for_vite_

Conversation

@leaysgur
Copy link
Member

@leaysgur leaysgur commented Mar 16, 2026

Fixes #20416

  • Both: Update test utils to accept env vars
  • Oxlint: Also disable nested config for Vite+

Vite+ PR: voidzero-dev/vite-plus#946
(We don't need to worry about the release timing, since Vite+ locks our version.)

@github-actions github-actions bot added A-linter Area - Linter A-cli Area - CLI A-formatter Area - Formatter A-linter-plugins Area - Linter JS plugins labels Mar 16, 2026
Copy link
Member Author

leaysgur commented Mar 16, 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.

@github-actions github-actions bot added the C-bug Category - Bug label Mar 16, 2026
@leaysgur leaysgur changed the title fix(oxlint,oxfmt): Check env.VITE_PLUS_VERSION for Vite+ fix(oxlint,oxfmt): Check env.VITE_PLUS_VERSION for Vite+ Mar 16, 2026
@leaysgur leaysgur force-pushed the 03-16-fix_oxlint_oxfmt_check_env.vite_plus_version_for_vite_ branch 4 times, most recently from 8099731 to b8fa6a8 Compare March 16, 2026 08:49
@leaysgur leaysgur marked this pull request as ready for review March 16, 2026 08:49
@leaysgur leaysgur requested a review from camc314 as a code owner March 16, 2026 08:49
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.

Can you explain how this works with LSP?

I think users have to manually configure the env var? This will break support when users are using 1x VP workspace & 1x non-VP workspace, since env vars are singletons across the process. Futhermore, I think this increases the barrier to entry, in the same way that requireing a CLI flag to work out which config file would be.

@leaysgur
Copy link
Member Author

leaysgur commented Mar 16, 2026

I think users have to manually configure the env var?

This is false, since Vite+'s bin wrapper handle this.

https://github.com/voidzero-dev/vite-plus/pull/946/changes


This will break support when users are using 1x VP workspace & 1x non-VP workspace, since env vars are singletons across the process.

This is yes, and no.

But this may be a trade-off... Do we break Oxc users to support Vite+, or do we protect one and break the other?

Personally, I think this approach is safer, as long as it works if the relationship between the workspace and the LSP is properly maintained. At least for now, this will only affect Vite+ users, whereas before, both Oxc and Vite+ users were having issues.

But if we do oxc-project/oxc-vscode#145, this plan will work without worries.

@graphite-app graphite-app bot changed the base branch from 03-13-fix_oxlint_oxfmt_suppress_stdout_while_import_jsconfig_for_lsp to graphite-base/20417 March 16, 2026 10:11
@graphite-app graphite-app bot force-pushed the 03-16-fix_oxlint_oxfmt_check_env.vite_plus_version_for_vite_ branch from b8fa6a8 to ba64c12 Compare March 16, 2026 10:28
@graphite-app graphite-app bot force-pushed the graphite-base/20417 branch from 2ec8349 to 89ce30b Compare March 16, 2026 10:28
@graphite-app graphite-app bot changed the base branch from graphite-base/20417 to main March 16, 2026 10:28
@graphite-app graphite-app bot force-pushed the 03-16-fix_oxlint_oxfmt_check_env.vite_plus_version_for_vite_ branch from ba64c12 to f75b58c Compare March 16, 2026 10:29
@Sysix
Copy link
Member

Sysix commented Mar 16, 2026

But if we do oxc-project/oxc-vscode#145, this plan will work without worries.

This only will "fix integration" with VS Code.
Copying from oxc-project/oxc-vscode#145 (comment)

We should also be careful about other IDEs and how they integrate with LSP and our server. "Fixing" it on the IDE side will force us to test one more scenario how oxlint --lsp and oxfmt --lsp interact with an editor. Would love to see a consistent across the editors 🫶

I added a notice in the issue, that the "root" problem could also affect non js/ts config.

@leaysgur
Copy link
Member Author

In the current situation,

  • merely touching vite.config.ts causes an error
  • And to load vite.config.ts without error in most cases, a vite implementation is essential
  • However, it is unlikely that Oxapps will add vite as dependency

Given this premise, I believe the only thing Oxapps can do is to make it possible to completely ignore vite.config.ts via a switch.

In that case, enabling the Vite+ mode switch becomes a responsibility of the extension, and I believe that is a necessary cost for the feature of supporting Vite+.

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.

oxlint,oxfmt: Support vite.config.ts only when its usage is explicitly indicated

3 participants