fix(cli): skip merging standalone oxfmt/oxlint config when key already in vite.config.ts#1601
Merged
Conversation
…y in vite.config.ts `mergeAndRemoveJsonConfig` was the only `mergeJsonConfig` call site missing the "skip if key already present" guard the other call sites use. Since the Rust merge step always prepends, templates that ship a populated `vite.config.ts` AND a standalone `.oxfmtrc.jsonc` (e.g. create-fate) ended up with two `fmt:` blocks after `vp create` / `vp migrate`. Now read `vite.config.ts` first; if the key is already declared, unlink the redundant standalone file (matching the existing skip-existing behavior in `applyToolInitConfigToViteConfig`) and log info instead of merging. Adds reproducing unit tests (fmt + lint) and a global snap test mirroring fate's real-world fmt/lint configuration.
✅ Deploy Preview for viteplus-preview canceled.
|
…nfig_key
`mergeAndRemoveJsonConfig`'s regex gate (`\b${configKey}\s*:`) had known
false positives (comments, string literals, nested keys like
`plugins: [{ fmt: ... }]`) and false negatives (e.g. spread). Move the
check into Rust as a precise AST walker that mirrors the six object-literal
shapes the merger already understands (`defineConfig({...})`, arrow
callback, `return {...}` inside callback, plain `export default`,
`satisfies` export, arrow-wrapped defineConfig). Bare identifier keys and
quoted string keys both match; computed keys, comments, strings, and
nested-object keys are ignored.
The `return $VAR` variant cannot be inspected statically — that path
conservatively reports `false`, which is safe because the merger uses
object spread (`{ key: ..., ...$VAR }`) so duplicate keys resolve to the
later spread at runtime.
- crates/vite_migration: new `has_config_key`, 13 unit tests covering the
shapes plus comment/string/nesting traps and the fate template shape.
- packages/cli/binding: NAPI export `hasConfigKey(viteConfigPath, configKey)`.
- packages/cli: `mergeAndRemoveJsonConfig` calls `hasConfigKey` instead of
the regex.
Snap output is byte-identical to the regex version on the regression case;
all 75 migrator unit tests and the migration snap suite still pass.
Address review feedback from /simplify: - Drop the regex-based hasConfigKey twin in init-config.ts; route inspectInitCommand / applyToolInitConfigToViteConfig through the new AST-based NAPI. Same swap for the inline regex in injectConfigDefaults. Removes the bug-prone duplicate that motivated this PR. - Simplify Rust helpers: .map().unwrap_or(false) → .is_some_and(), is_define_config_arrow_body to a filter-chain, drop narrating comments that restate kind names already in the match arms. - Drop the redundant lint unit test in migrator.spec.ts; the Rust suite exhaustively covers AST shapes (13 cases), and the fmt test alone proves the TS wiring through NAPI + mergeAndRemoveJsonConfig. All 180 Rust + 81 TS tests pass; init-config and migration snap tests unchanged.
Member
Author
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit e0c99be. Configure here.
fengmk2
added
test: e2e
cpojer
approved these changes
May 18, 2026
Member
cpojer
left a comment
cpojer
left a comment
There was a problem hiding this comment.
Haha, I didn't realize the fate templates ship with duplicate configs. Woops. Thank you for the fix in Vite+!
Member
Author
|
@cpojer fate templates help me realize how fragile the previous temporary solutions were 😢 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
vp create fateproduced avite.config.tswith twofmt:blocks becausemergeAndRemoveJsonConfig(packages/cli/src/migration/migrator.ts) was the onlymergeJsonConfigcall site missing the "skip if key already present" guard the other call sites use. The Rust merge step always prepends, so when fate's template ships both an inlinefmt:block and a standalone.oxfmtrc.jsonc, both ended up in the output.vite.config.tsfirst; if the key is already declared,fs.unlinkSyncs the redundant standalone file and emits an info log instead of merging. Mirrors the existing skip-existing behavior inapplyToolInitConfigToViteConfig. Also covers the analogouslint:+.oxlintrc.jsoncase.Test plan
packages/cli/src/migration/__tests__/migrator.spec.ts(fmt + lint), confirmed red before the fix and green after —vp test --run packages/cli/src/migration/__tests__/migrator.spec.ts(75/75 passing).packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/modeled on fate's real-world fmt/lint config (experimental options, ignore patterns, overrides); standalone files carry conflicting values so any regression would be obvious in the snapshot.pnpm -F vite-plus snap-test-global migration— all existing migration snaps still pass with no diffs.pnpm -F vite-plus snap-test-global new-create-vite— all create snaps still pass with no diffs.Closes the duplicate-fmt regression reported when running
vp create fate.Note
Medium Risk
Migration logic now conditionally deletes standalone
.oxfmtrc/.oxlintrcfiles based on a new AST-based vite config scan exposed via a native binding; mistakes in the detector could cause config merges to be skipped or applied unexpectedly.Overview
Fixes a migration regression where projects with an existing inline
fmt/lintblock invite.config.*could end up with duplicated blocks after merging standalone.oxfmtrc/.oxlintrcfiles.This adds a Rust AST-based
has_config_keycheck (exported through the NAPI binding ashasConfigKey) and uses it in the CLI migration paths to skip merging when the key already exists, instead deleting the redundant standalone config and optionally logging an info message. New unit and snapshot tests cover the create-fate-style template shape and ensure only a singlefmt/lintblock remains.Reviewed by Cursor Bugbot for commit e0c99be. Configure here.