✅ Deploy Preview for viteplus-preview canceled.

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	npm/​oxfmt@​0.1.0

View full report

@codex review

I was about to pick up #2430, found this PR already does it (nice!), and had a few empirical probes lying around that seem useful as review input. All verified against the bundled oxlint 1.79.0 / oxfmt 0.64.0 binaries on this repo's checkout.

1. PR description vs code: extension list. The description says detection covers .ts / .mts / .cts / .js / .mjs / .cjs, but the detector adds only oxlint.config.ts / oxlint.config.mts (and the oxfmt equivalents) — and the code is the correct side: the native binaries' candidate lists contain only the .ts/.mts forms, and an oxlint.config.js on disk is silently ignored (rules from it don't apply; default severities stay). Probably just worth fixing the description so nobody requests the extra extensions in review.

2. Post-migration shadowing (may deserve an info line in the summary). With both oxlint.config.ts and a vite.config.ts containing a lint block present, a direct oxlint invocation (editor integrations, plain CLI) uses oxlint.config.ts and silently ignores the lint block:

$ cat oxlint.config.ts   # rules: { 'no-debugger': 'error' }
$ cat vite.config.ts     # lint: { rules: { 'no-console': 'error' } }
$ oxlint a.js            # a.js has `debugger;` and `console.log(1)`
a.js:1:1: error eslint(no-debugger): ...        # from oxlint.config.ts
(no no-console diagnostic — the vite.config lint block is ignored)

Right after this migration the two are consistent because lint: imports the same object, so this is fine as-is. But a user who later edits the lint: value inline in vite.config.ts (e.g. lint: { ...oxlintConfig, rules: {...} }) gets a split brain: vp lint sees the edit, direct oxlint runs don't. Since the tsdown path always emits a "please manually merge …" info line, a similar one-liner here ("oxlint.config.ts remains the config used by direct oxlint invocations") might save some confusion.

3. Edge: both .oxlintrc.json and oxlint.config.ts present. oxlint itself hard-errors on that state ("Only one of .oxlintrc.json and oxlint.config.ts is allowed per directory"), so such projects are already broken — but the migrator's first-match detection picks the rc file, inlines + deletes it, and leaves oxlint.config.ts on disk unreferenced, where it then shadows the freshly inlined lint block per (2). A warning when both forms are detected would make the outcome self-explanatory. Definitely a nit.

Thanks — these are useful, and (1) was a real error in the description rather than the code.

1. Confirmed and fixed. detector.ts lists only .oxlintrc.json, .oxlintrc.jsonc, oxlint.config.ts, oxlint.config.mts (and the oxfmt equivalents), which matches what the bundled binaries actually search for. The description claimed .cts / .js / .mjs / .cjs as well, and also referred to an oxfmt.config.mjs. Both are corrected now, so the description matches the diff.

2. and 3. Both read as genuine, and I'd rather a maintainer decide whether they belong in this PR or a follow-up, since each widens the scope past what #2430 asked for. Happy to add either on request:

• (2) an info line noting oxlint.config.ts remains the config used by direct oxlint invocations, mirroring the existing "please manually merge …" line on the tsdown path;
• (3) a warning when both .oxlintrc.json and oxlint.config.ts are present, since first-match detection inlines and deletes the rc file and leaves the dynamic config on disk unreferenced.

Worth noting for (3) that oxlint hard-errors on that combination, so any project hitting it is already broken before migration — the warning would make the outcome legible rather than change it.

(not a maintainer, but the person who created the linked issue)

in the cases of 2 and 3, just my two cents of "what makes the most sense":

2. "Please manually merge..." shouldn't be the wording, it should say "We strongly recommend manually merging oxlint.config.ts immediately to ensure alignment with IDE integrations". The stronger wording compared to tsdown is intentional because tsdown doesn't have an editor integration of any kind (at least for now), so it's less important. This stronger wording should apply to oxfmt.config.ts as well.
3. Since oxlint already hard errors on multi-config situations it should make intuitive sense for vp migrate to hard error there as well. However, there can also be the case of matching others' migrations flow for this edge case. I think a review of how other migration flows handle this is a good idea, but that is outside the scope of both the issue and the PR.

Pushed fbd65ee for the Codex review. Triage of all five findings below, separating what I fixed from what I verified but deliberately left alone.

Fixed — P1 "Preserve declared Oxc package boundaries during rewrites"

Confirmed, and it is this PR's regression. The vite, vitest and tsdown passes in rewrite_import_content_full() are each gated on a SkipPackages flag; the new Oxc pass was applied unconditionally, and content_may_need_rewriting() matched oxlint/oxfmt with no skip check either. A package that declares oxlint or oxfmt in its own dependencies/peerDependencies keeps that dependency through rewritePackageJson, so its sources were rewritten to vite-plus/lint / vite-plus/fmt anyway.

The fix splits REWRITE_OXC_RULES into separate oxlint and oxfmt rule sets and gates each on its own flag, computed from peerDependencies/dependencies exactly like the existing three, at both the pre-filter and the apply site. Keeping them separate means a package that declares only one of the two still gets the other rewritten.

cargo test -p vp_migration goes 315 → 320. Verified in both directions: reverting only the new guards fails exactly the three skip tests, and test_oxc_imports_still_rewritten_when_not_declared passes either way, so the default path is unchanged.

Confirmed, not fixed here — P1 "Retain type-check defaults for dynamic lint configs" and P2 "Add Svelte rune globals"

Both are real, and they are two symptoms of one gap rather than two independent bugs. injectConfigDefaults() returns early when the top-level key already exists, and mergeDynamicConfigFile() has just inserted lint: oxlintConfig, so injectLintTypeCheckDefaults() no-ops. The same asymmetry drops two more steps Codex didn't mention: the dynamic branch also skips sanitizeMigratedOxlintConfig() and ensureVitePlusImportRuleDefaults().

The common cause: everything the JSON branch does, it does by mutating a parsed object before writing it back. When the config is an opaque imported binding there is nothing to mutate, so this can't be fixed by "merging the missing defaults around" the import — it needs either generated wrapper code at the call site, or an explicit warning that the defaults were not applied. That's a design choice with user-visible output either way, so I'd rather you pick. Happy to implement whichever, in this PR or a follow-up.

Confirmed but pre-existing — P2 "Choose a collision-free name for the injected config import"

Real: merge_dynamic_config_content() guards with the substring test content.contains("import oxlintConfig from"), which misses a local const, an aliased or type-only import, a namespace import, and a multiline default import — any of which yields a duplicate binding and invalid TypeScript.

Worth noting it is not introduced here: the same function already backs merge_tsdown_config (tsdownConfig → pack), so the bug predates this PR, though this PR does widen exposure to two more identifiers. The repo already has the right machinery in has_conflicting_lazy_plugins_binding() — AST walk plus the destructured, multi-declarator and aliased-import regexes — but it is hard-coded to the literal lazyPlugins. Generalizing it to an arbitrary identifier is a shared-code refactor that changes the tsdown path too, so I've kept it out of this PR rather than widen it unasked. Say the word if you'd prefer it folded in.

Unverified — P2 "Rewrite imported configs even when gitignored"

Half-confirmed only. detectConfigs() uses plain fs.existsSync with no ignore awareness, so it will indeed select a gitignored oxlint.config.ts. I could not exercise the rewriteAllImports() side in this environment, so I'm flagging it as unconfirmed rather than asserting the full failure mode.

Validation limits

This sandbox can't complete just init (the gitignored vite/ and rolldown/ checkouts that sync-remote creates aren't present), so the NAPI binding and the JS suite couldn't be built here. What I actually ran for this commit: cargo test -p vp_migration (320 passed), cargo clippy -p vp_migration --all-targets (clean), and cargo fmt. The TypeScript suite and the PTY snapshot fixture were not re-run — the change is Rust-only and adds no new CLI output, but CI is the real check on those.

@KTrain5169 — thanks, that's exactly the steer I was asking for.

On (2): agreed on the stronger wording and on applying it to oxfmt.config.ts too, and the editor-integration rationale for making it stronger than the tsdown line is convincing. I've deliberately not pushed it in this commit, because it adds a line to the success path and the migration_dynamic_oxc_configs PTY snapshot records that output — changing it without being able to regenerate the snapshot here would just turn CI red. It needs an environment that can run UPDATE_SNAPSHOTS=1 just snapshot-test, so I'd like a maintainer's nod on the wording before spending that round trip.

On (3): agreed, and I'll leave it out. Your reasoning matches what I found — projects in that state are already broken because oxlint hard-errors on it — and surveying how other migrators handle the case is genuinely outside this issue.

Generated by Claude Code

Both dispositions look right from where I sit — (1) matches the code now, and leaving (3) out is consistent with what I reproduced (oxlint hard-errors on that combination before migration ever sees it, so the warning would only narrate an already-broken state).

On (2): once a maintainer settles the wording, if the snapshot round trip is the blocker — I have a local checkout that runs the PTY suite fine, so I'm happy to run UPDATE_SNAPSHOTS=1 just snapshot-test migration_dynamic_oxc_configs against your branch and post the resulting .md diff here to save you the environment dance.

Thanks — and that offer is genuinely useful, since it removes the half of the blocker I can't solve in my environment.

One clarification on what's actually left, though: the snapshot regeneration was the second gate, not the only one. The wording itself is still a maintainer call. @KTrain5169 proposed:

We strongly recommend manually merging oxlint.config.ts immediately to ensure alignment with IDE integrations

applied to oxfmt.config.ts as well, with the stronger-than-tsdown phrasing justified by oxlint having an editor integration where tsdown doesn't. That reasoning convinces me and I'd be happy to use it close to verbatim — but it's a user-facing string on the success path, and two non-maintainers agreeing on it isn't the same as it being the wording the project wants to ship.

So the order stands as: once a maintainer settles the wording, I'll push the info line and take you up on the UPDATE_SNAPSHOTS=1 just snapshot-test migration_dynamic_oxc_configs run against the branch. Posting the resulting .md diff here would also mean the recorded output gets reviewed before it lands, rather than after CI turns red — which is strictly better than the round trip I was originally planning.

Generated by Claude Code

@fengmk2 ^

Since oxlint already hard errors on multi-config situations it should make intuitive sense for vp migrate to hard error there as well

Agreed, interrupting with an error is more reasonable, let user to manually address config conflicts first.

Thanks — that settles (3), so I've implemented it. Pushed 239cd38.

What it does

vp migrate now refuses to start when a directory holds both a JSON and a dynamic config for the same Oxc tool, naming every offending directory:

✘ Conflicting Oxc configs:
  - the project root has `.oxlintrc.json` and `oxlint.config.ts` — oxlint allows only one config per directory.
  - packages/app has `.oxfmtrc.json` and `oxfmt.config.mts` — oxfmt allows only one config per directory.

then exits 1 via cancelAndExit. It runs immediately after workspace detection — before any file is touched — and covers the workspace root plus every workspace package, on both the full-migration and the already-Vite+ paths. That placement matters more than the message: the failure mode is losing settings mid-rewrite, so the interrupt has to land before the first write, not at the summary.

Two scope calls, both easy to trim if you disagree

• Two JSON forms (.oxlintrc.json + .oxlintrc.jsonc) are deliberately not a conflict. The ambiguity being guarded is JSON-vs-dynamic, which is what oxlint itself rejects and what orphans a config across migration. Erroring on .json + .jsonc would be me guessing at behavior I could not verify.
• oxfmt is included alongside oxlint. Being straight about the asymmetry in the evidence: for oxlint the hard-error is quoted upstream behavior, and I could not run the bundled binaries here to confirm oxfmt does the same. The reason I still included it is the migrator-side hazard, which is symmetric regardless of what the binary does — first-match detection inlines and deletes the rc file and leaves the dynamic config unreferenced either way. If you'd rather this stayed strictly oxlint-only until someone confirms oxfmt's behavior, deleting the oxfmt entry from the tools array in detectOxcConfigConflicts is the whole change.

Verification

• 22 new unit tests in detector.spec.ts (27 in the file total), verified in both directions:
  • reverting detector.ts entirely → 22 failed / 5 passed (the 5 are the pre-existing detectConfigs tests, so the default path is untouched);
  • keeping the API but disabling only the conflict decision → 9 failed / 18 passed, i.e. the failures come from the behavior, not from a missing export.
• No existing snapshot output changes, and this is checked rather than assumed: no fixture under cli_snapshots/fixtures/ pairs a JSON and a dynamic config for the same tool. The dirs with two configs are all oxlint+oxfmt pairs, which this guard ignores.
• Ran the whole packages/cli/src/migration/__tests__ directory before and after and diffed the failure sets — identical, with +22 passing (397 → 419 tests). Absolute failures in that directory are my sandbox's NAPI stub, not results about this change.
• oxfmt --check and oxlint clean on the changed files, each run in the same invocation as untouched control files so a silent misconfiguration would show up; tsc --strict clean on detector.ts and no non-module-resolution errors in bin.ts.

Still open

• The PTY fixture for the new error path is not in this commit. I can't record a snapshot I can't run, and this sandbox has no built vp. @simulacre7 — if that offer still stands, this is a second, smaller thing to run alongside the (2) one: a fixture with .oxlintrc.json + oxlint.config.ts in the root asserting the interrupt.
• (2) is unchanged and still needs a maintainer call on the wording — the stronger "we strongly recommend manually merging …" line for oxlint.config.ts / oxfmt.config.ts. It's independent of this commit.

Generated by Claude Code

Ran the offer against 239cd38 — here is the PTY fixture for the new error path plus its recorded snapshot, ready to commit. Everything below was recorded and verified locally on macOS arm64 (UPDATE_SNAPSHOTS=1 just snapshot-test migration_oxc_config_conflict, then a clean re-run: 1 passed).

Also cross-checked your "no existing snapshot output changes" claim by running just snapshot-test migration_dynamic_oxc_configs on the branch: passes unchanged.

fixtures/migration_oxc_config_conflict/snapshots.toml

[[case]]
name = "migration_oxc_config_conflict"
vp = "global"
steps = [
  { argv = ["vp", "migrate", "--no-interactive"], comment = "migration should refuse to start on conflicting Oxc configs", continue-on-failure = true },
  { argv = ["vpt", "stat-file", ".oxlintrc.json", "--assert", "file"], comment = "both configs left untouched by the interrupt", continue-on-failure = true },
  { argv = ["vpt", "stat-file", "oxlint.config.ts", "--assert", "file"], continue-on-failure = true },
  { argv = ["vpt", "stat-file", "vite.config.ts", "--assert", "missing"], comment = "no file was written before the interrupt", continue-on-failure = true },
  { argv = ["vpt", "print-file", "package.json"], comment = "package.json unchanged", continue-on-failure = true },
]

fixtures/migration_oxc_config_conflict/package.json

{
  "name": "migration-oxc-config-conflict",
  "scripts": {
    "lint": "oxlint"
  },
  "devDependencies": {
    "oxlint": "^1.0.0",
    "vite": "^7.0.0"
  }
}

fixtures/migration_oxc_config_conflict/.oxlintrc.json

{
  "rules": {
    "no-console": "error"
  }
}

fixtures/migration_oxc_config_conflict/oxlint.config.ts

import { defineConfig } from 'oxlint';

export default defineConfig({
  rules: {
    eqeqeq: 'error',
  },
});

fixtures/migration_oxc_config_conflict/snapshots/migration_oxc_config_conflict.md (recorded)

# migration_oxc_config_conflict

## `vp migrate --no-interactive`

migration should refuse to start on conflicting Oxc configs

**Exit code:** 1

```
VITE+ - The Unified Toolchain for the Web

✘ Conflicting Oxc configs:
  - the project root has `.oxlintrc.json` and `oxlint.config.ts` — oxlint allows only one config per directory.
Keep a single config per directory, then run `vp migrate` again.
```

## `vpt stat-file .oxlintrc.json --assert file`

both configs left untouched by the interrupt

```
.oxlintrc.json: file
```

## `vpt stat-file oxlint.config.ts --assert file`

```
oxlint.config.ts: file
```

## `vpt stat-file vite.config.ts --assert missing`

no file was written before the interrupt

```
vite.config.ts: missing
```

## `vpt print-file package.json`

package.json unchanged

```
{
  "name": "migration-oxc-config-conflict",
  "scripts": {
    "lint": "oxlint"
  },
  "devDependencies": {
    "oxlint": "^1.0.0",
    "vite": "^7.0.0"
  }
}
```

The steps assert the "before any file is touched" placement, not just the message: both configs still present, no vite.config.ts created, package.json byte-identical. Still holding the (2) snapshot run for whenever the wording lands.

@simulacre7 thank you — landed as 13a11a9, with a Co-authored-by trailer since the recording is yours.

I didn't take the snapshot on trust, since I can't re-run the recorder here. What I did check before committing it:

• Ran the real detector against the real fixture directory in this sandbox — collectOxcConfigConflicts + formatOxcConfigConflict on fixtures/migration_oxc_config_conflict/ returns exactly one conflict (oxlint, dir .) and renders byte-identically to the block you recorded, em-dash and backticks included.
• The one structural difference from an existing recorded case is the second line inside log.error. prompts.log.error is a multi-line call here, and log.message defaults withGuide to false, so continuation lines get no bar prefix — the - indent is the one bin.ts supplies. Everything else in the frame (banner, blank line, ✘ line, then the bare cancelAndExit line) is pinned by migration_not_supported_npm8_2, which is an actual recorded run of the identical log.error + cancelAndExit pair.
• vpt stat-file prints {path}: {state} and print-file cats raw bytes, so those four steps' outputs are exact.
• The .md layout matches the writer in cli_snapshots/main.rs — ## heading, comment, **Exit code:** only when nonzero, fenced block.

Also worth saying explicitly, because it's the reason your step choice is the right one: the placement your steps assert is real, not incidental. assertNoOxcConfigConflicts runs immediately after detectWorkspace, before package-manager resolution and before the already-Vite+ early return. That's why the fixture needs no packageManager pin to reach the check, and why vite.config.ts: missing plus a byte-identical package.json are guaranteed rather than hopeful. A regression that moved the guard after the first write would fail those steps even if the error text were untouched — which is more than the message assertion alone would catch.

The one thing I could not do is run the PTY suite (no built vp in this sandbox), so the recorded .md is yours, unmodified. If CI's recorder disagrees with it on whitespace, the .md.new diff is the authority over my reading of the writer — please say so and I'll correct it rather than argue from source.

Still open and unchanged by this commit: (2)'s wording needs @fengmk2's call, and the oxfmt scope question from my last comment (whether the guard stays oxlint-only until someone can confirm oxfmt's own multi-config behaviour — deleting the oxfmt entry from the tools array is the entire change if so).

Generated by Claude Code

Closing the oxfmt scope question with the bundled binary (oxfmt 0.63.0 from this branch's packages/cli/node_modules):

$ ls
.oxfmtrc.json  oxfmt.config.mts  a.js
$ oxfmt --check a.js
Failed to load configuration file.
Both '.oxfmtrc.json' and 'oxfmt.config.mts' found in <dir>.
(exit 1)

Control with only .oxfmtrc.json present passes (exit 0). So oxfmt enforces the same one-config-per-directory rule as oxlint, and keeping oxfmt in the tools array is upstream-behavior-backed, not just migrator-hazard-backed — no trim needed.

Snapshot side of 13a11a9: re-ran just snapshot-test migration_oxc_config_conflict against the branch after your commit — passes against the landed .md as-is.

@simulacre7 that settles oxfmt — thank you. I reproduced it here against the same pinned pair (oxfmt@0.63.0, oxlint@1.78.0 from this branch's catalog) and got your exact result, so the oxfmt entry in the tools array stays: no trim.

But running the matrix rather than just your one case turned up a real gap in my own guard, so I've pushed 31163c5.

I had written, in the comment above, that two JSON forms are deliberately not a conflict, on the grounds that "erroring on .json + .jsonc would be me guessing at behavior I could not verify." I could verify it once I installed the binaries, and the guess was wrong. Both tools reject any two configs in a directory, not just the mixed pair:

So the rule is one config per directory, not one config form — and the two shapes the old predicate let through are exactly the ones migration would then walk into: first-match detection consumes one config and leaves the other on disk unreferenced. Same failure mode the guard exists to prevent, just reached by a different pair.

What changed

detectOxcConfigConflicts now flags any tool with more than one config present, and OxcConfigConflict carries a single configs list in the tool's own candidate order instead of the jsonConfigs/dynamicConfigs split. Net effect on the source is a simplification (−8 lines).

Verification

• Both directions, on the repo's own detector.spec.ts: 31 passed with the fix; 4 failed / 27 passed with only the decision reverted to the old JSON-and-dynamic predicate while keeping the new configs API — the four failures are precisely the four newly covered shapes, so they come from behaviour, not a renamed field. A full revert of detector.ts gives 14 failed / 17 passed.
• Whole packages/cli/src/migration/__tests__ directory, before and after: failure sets byte-identical (151 both sides, zero new, zero newly fixed), tests 419 → 423. The absolute failures there are my sandbox's NAPI stub, not results about this change.
• Your snapshot is untouched and still correct. I re-ran the real collectOxcConfigConflicts + formatOxcConfigConflict over every one of the 1,702 directories in the checkout under the new rule: exactly one conflict, fixtures/migration_oxc_config_conflict, and its rendered line compares === true against the recorded .md. No other fixture newly trips the widened guard.
• oxfmt --check and oxlint clean on the changed files, each run in the same invocation as untouched control files; tsc --strict --noEmit clean on both changed files, and no bin.ts error mentions the conflict API.

Message rendering is unchanged for the two-config case (so the snapshot stands as recorded) and gains comma separation once a directory holds three or more: has `a`, `b` and `c` .

Still not run here: the PTY suite, for the same reason as before — no built vp in this sandbox.

Unchanged and still needing @fengmk2: (2)'s wording, the stronger "manually merge" line. That's the only open item left on this PR from my side.

Generated by Claude Code

PTY suite result for 31163c5 (the gap you flagged): all 154 migration* cases pass on macOS arm64, both flavors.

Full detail for honesty: the parallel run finished 151/154, with 3 failures whose names changed between runs (migration_dynamic_oxc_configs among them, which had passed twice before) — all 3 pass deterministically when run in isolation, so those are load-induced PTY flakes on my machine, not the diff. An earlier run also showed local-flavor failures that were purely my stale packages/cli/dist tripping the freshness guard; rebuilt and they cleared. Nothing attributable to 31163c5, and migration_oxc_config_conflict passes against the landed .md unchanged.

@simulacre7 that closes the last gap on my side — thank you. The PTY suite was the one check I'd flagged as not runnable in my sandbox, so 31163c5 now has it covered.

Two things I could verify from this end that make the result a bit stronger than a bare pass:

The 154 is the complete migration set. Counting [[case]] entries across every migration_* fixture at 31163c5 gives exactly 154. So the run covered all of them, the new migration_oxc_config_conflict included — worth stating explicitly, because the count on its own rules out a filter that quietly skipped the new case.

The fixture that flaked isn't structurally unusual. migration_dynamic_oxc_configs is one of 21 fixtures that invoke vp migrate twice, and at 7 steps it sits mid-pack (19 migration fixtures have 7 steps or more). Nothing about it is heavier than its peers, which supports your reading — runner load, not something this PR's fixture does. Together with the failure names changing between runs and all three passing in isolation, I don't see a case for touching it.

If that non-deterministic set ever turns up on main outside this branch, it's probably worth its own issue. I can't chase it from here — no built vp in this environment.

Unchanged: (2)'s wording is the only item still open from my side, and it's a maintainer call rather than anything blocked on code or snapshots.

Generated by Claude Code

Followed up on the flake question: it reproduces on main too (cold first run after a fresh build; warm re-runs fully green; every failed case passes in isolation). Filed as #2565 with the five-run matrix, so nothing further is attributable to this branch.