Skip to content

Commit 9e44db1

Browse files
authored
fix(cli): skip merging standalone oxfmt/oxlint config when key already in vite.config.ts (#1601)
## Summary - `vp create fate` produced a `vite.config.ts` with two `fmt:` blocks because `mergeAndRemoveJsonConfig` (`packages/cli/src/migration/migrator.ts`) was the only `mergeJsonConfig` call 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 inline `fmt:` block and a standalone `.oxfmtrc.jsonc`, both ended up in the output. - Now reads `vite.config.ts` first; if the key is already declared, `fs.unlinkSync`s the redundant standalone file and emits an info log instead of merging. Mirrors the existing skip-existing behavior in `applyToolInitConfigToViteConfig`. Also covers the analogous `lint:` + `.oxlintrc.json` case. ## Test plan - [x] New unit tests in `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). - [x] New global snap test `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. - [x] `pnpm -F vite-plus snap-test-global migration` — all existing migration snaps still pass with no diffs. - [x] `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`. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Migration logic now conditionally deletes standalone `.oxfmtrc`/`.oxlintrc` files 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`/`lint` block in `vite.config.*` could end up with duplicated blocks after merging standalone `.oxfmtrc`/`.oxlintrc` files. > > This adds a Rust AST-based `has_config_key` check (exported through the NAPI binding as `hasConfigKey`) 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 single `fmt`/`lint` block remains. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit e0c99be. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 646ea16 commit 9e44db1

14 files changed

Lines changed: 506 additions & 12 deletions

File tree

crates/vite_migration/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,4 @@ mod vite_config;
1818
pub use file_walker::{WalkResult, find_ts_files};
1919
pub use import_rewriter::{BatchRewriteResult, rewrite_imports_in_directory};
2020
pub use package::{rewrite_eslint, rewrite_prettier, rewrite_scripts};
21-
pub use vite_config::{MergeResult, merge_json_config, merge_tsdown_config};
21+
pub use vite_config::{MergeResult, has_config_key, merge_json_config, merge_tsdown_config};

crates/vite_migration/src/vite_config.rs

Lines changed: 279 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use std::{borrow::Cow, path::Path, sync::LazyLock};
22

33
use ast_grep_config::{GlobalRules, RuleConfig, from_yaml_string};
4+
use ast_grep_core::{Doc, Node};
45
use ast_grep_language::{LanguageExt, SupportLang};
56
use regex::Regex;
67
use vite_error::Error;
@@ -128,6 +129,93 @@ fn strip_schema_property(config: &str) -> Cow<'_, str> {
128129
RE_SCHEMA.replace_all(config, "")
129130
}
130131

132+
/// Check whether `config_key` is already declared as a top-level property in
133+
/// the vite config's `defineConfig({...})` (or equivalent) object literal.
134+
///
135+
/// Mirrors the six shapes the merger understands (see `generate_merge_rule`):
136+
/// `defineConfig({...})`, `defineConfig((p) => ({...}))`, `return {...}`
137+
/// inside a `defineConfig` callback, `export default {...}`, and the
138+
/// `satisfies` export variant. The `return $VAR` variant cannot be inspected
139+
/// statically — for that shape we conservatively report `false`, which is
140+
/// safe because the merger uses object spread (`{ key: ..., ...$VAR }`) so
141+
/// duplicate keys are resolved at runtime by JS spread semantics.
142+
///
143+
/// Returns `true` only when the key appears as a **direct** member of one of
144+
/// those recognized object literals. Comments, string occurrences, nested
145+
/// keys (e.g. `plugins: [{ fmt: ... }]`), and unrelated objects are all
146+
/// ignored correctly.
147+
pub fn has_config_key(vite_config_content: &str, config_key: &str) -> Result<bool, Error> {
148+
let grep = SupportLang::TypeScript.ast_grep(vite_config_content);
149+
let root = grep.root();
150+
151+
for node in root.dfs() {
152+
if node.kind() != "pair" {
153+
continue;
154+
}
155+
let Some(key_node) = node.field("key") else { continue };
156+
if !pair_key_matches(&key_node, config_key) {
157+
continue;
158+
}
159+
let Some(parent_object) = node.parent() else { continue };
160+
if parent_object.kind() != "object" {
161+
continue;
162+
}
163+
if is_recognized_config_object(&parent_object) {
164+
return Ok(true);
165+
}
166+
}
167+
168+
Ok(false)
169+
}
170+
171+
fn pair_key_matches<D: Doc>(key_node: &Node<'_, D>, config_key: &str) -> bool {
172+
let text = key_node.text();
173+
match key_node.kind().as_ref() {
174+
"property_identifier" => text == config_key,
175+
"string" => text.trim_matches(|c| c == '"' || c == '\'' || c == '`') == config_key,
176+
_ => false,
177+
}
178+
}
179+
180+
fn is_recognized_config_object<D: Doc>(object_node: &Node<'_, D>) -> bool {
181+
let Some(parent) = object_node.parent() else { return false };
182+
match parent.kind().as_ref() {
183+
"export_statement" => true,
184+
// `export default { ... } satisfies T` — hop past the satisfies wrapper.
185+
"satisfies_expression" => parent.parent().is_some_and(|p| p.kind() == "export_statement"),
186+
"arguments" => parent.parent().is_some_and(|c| is_define_config_call(&c)),
187+
"parenthesized_expression" => is_define_config_arrow_body(&parent),
188+
"return_statement" => is_inside_define_config_callback(&parent),
189+
_ => false,
190+
}
191+
}
192+
193+
fn is_define_config_call<D: Doc>(call_node: &Node<'_, D>) -> bool {
194+
call_node.kind() == "call_expression"
195+
&& call_node.field("function").is_some_and(|f| f.text() == "defineConfig")
196+
}
197+
198+
fn is_define_config_arrow_body<D: Doc>(paren_node: &Node<'_, D>) -> bool {
199+
paren_node
200+
.parent()
201+
.filter(|n| n.kind() == "arrow_function")
202+
.and_then(|n| n.parent())
203+
.filter(|n| n.kind() == "arguments")
204+
.and_then(|n| n.parent())
205+
.is_some_and(|c| is_define_config_call(&c))
206+
}
207+
208+
fn is_inside_define_config_callback<D: Doc>(node: &Node<'_, D>) -> bool {
209+
let mut current = node.parent();
210+
while let Some(n) = current {
211+
if is_define_config_call(&n) {
212+
return true;
213+
}
214+
current = n.parent();
215+
}
216+
false
217+
}
218+
131219
/// Check if the vite config uses a function callback pattern
132220
fn check_function_callback(vite_config_content: &str) -> Result<bool, Error> {
133221
// Match both sync and async arrow functions
@@ -358,6 +446,197 @@ mod tests {
358446

359447
use super::*;
360448

449+
// ── has_config_key ────────────────────────────────────────────────────
450+
451+
#[test]
452+
fn test_has_config_key_top_level_in_defineconfig() {
453+
let cfg = r#"import { defineConfig } from 'vite-plus';
454+
455+
export default defineConfig({
456+
fmt: { singleQuote: true },
457+
lint: { rules: {} },
458+
});
459+
"#;
460+
assert!(has_config_key(cfg, "fmt").unwrap());
461+
assert!(has_config_key(cfg, "lint").unwrap());
462+
assert!(!has_config_key(cfg, "pack").unwrap());
463+
assert!(!has_config_key(cfg, "staged").unwrap());
464+
}
465+
466+
#[test]
467+
fn test_has_config_key_quoted_key() {
468+
let cfg = r#"import { defineConfig } from 'vite-plus';
469+
470+
export default defineConfig({
471+
'fmt': { singleQuote: true },
472+
"lint": {},
473+
});
474+
"#;
475+
assert!(has_config_key(cfg, "fmt").unwrap());
476+
assert!(has_config_key(cfg, "lint").unwrap());
477+
}
478+
479+
#[test]
480+
fn test_has_config_key_ignores_comment_mentions() {
481+
// The regex check was a false positive on these — AST check ignores them.
482+
let cfg = r#"import { defineConfig } from 'vite-plus';
483+
484+
// fmt: configure formatter here
485+
/* lint: TODO wire this up */
486+
export default defineConfig({
487+
plugins: [],
488+
});
489+
"#;
490+
assert!(!has_config_key(cfg, "fmt").unwrap());
491+
assert!(!has_config_key(cfg, "lint").unwrap());
492+
}
493+
494+
#[test]
495+
fn test_has_config_key_ignores_string_literal_mentions() {
496+
let cfg = r#"import { defineConfig } from 'vite-plus';
497+
498+
export default defineConfig({
499+
plugins: [],
500+
description: 'has fmt: foo and lint: bar inside',
501+
});
502+
"#;
503+
assert!(!has_config_key(cfg, "fmt").unwrap());
504+
assert!(!has_config_key(cfg, "lint").unwrap());
505+
}
506+
507+
#[test]
508+
fn test_has_config_key_ignores_nested_keys() {
509+
// `fmt:` is a nested property inside `plugins[0].options`, not top-level.
510+
let cfg = r#"import { defineConfig } from 'vite-plus';
511+
512+
export default defineConfig({
513+
plugins: [
514+
somePlugin({
515+
fmt: 'auto',
516+
lint: { enabled: true },
517+
}),
518+
],
519+
});
520+
"#;
521+
assert!(!has_config_key(cfg, "fmt").unwrap());
522+
assert!(!has_config_key(cfg, "lint").unwrap());
523+
}
524+
525+
#[test]
526+
fn test_has_config_key_arrow_callback() {
527+
let cfg = r#"import { defineConfig } from 'vite-plus';
528+
529+
export default defineConfig((env) => ({
530+
fmt: { singleQuote: env.mode === 'production' },
531+
}));
532+
"#;
533+
assert!(has_config_key(cfg, "fmt").unwrap());
534+
assert!(!has_config_key(cfg, "lint").unwrap());
535+
}
536+
537+
#[test]
538+
fn test_has_config_key_return_block_callback() {
539+
let cfg = r#"import { defineConfig } from 'vite-plus';
540+
541+
export default defineConfig(({ mode }) => {
542+
return {
543+
fmt: { singleQuote: true },
544+
};
545+
});
546+
"#;
547+
assert!(has_config_key(cfg, "fmt").unwrap());
548+
assert!(!has_config_key(cfg, "lint").unwrap());
549+
}
550+
551+
#[test]
552+
fn test_has_config_key_async_return_block_callback() {
553+
let cfg = r#"
554+
export default defineConfig(async ({ command, mode }) => {
555+
const data = await asyncFunction();
556+
return {
557+
lint: { rules: {} },
558+
};
559+
});
560+
"#;
561+
assert!(has_config_key(cfg, "lint").unwrap());
562+
assert!(!has_config_key(cfg, "fmt").unwrap());
563+
}
564+
565+
#[test]
566+
fn test_has_config_key_plain_export() {
567+
let cfg = r#"export default {
568+
fmt: { singleQuote: true },
569+
};
570+
"#;
571+
assert!(has_config_key(cfg, "fmt").unwrap());
572+
assert!(!has_config_key(cfg, "lint").unwrap());
573+
}
574+
575+
#[test]
576+
fn test_has_config_key_satisfies_export() {
577+
let cfg = r#"import type { UserConfig } from 'vite-plus';
578+
579+
export default {
580+
lint: { rules: {} },
581+
} satisfies UserConfig;
582+
"#;
583+
assert!(has_config_key(cfg, "lint").unwrap());
584+
assert!(!has_config_key(cfg, "fmt").unwrap());
585+
}
586+
587+
#[test]
588+
fn test_has_config_key_return_variable_is_unknown() {
589+
// The merger handles this via object spread, so duplication is benign.
590+
// We conservatively report `false`.
591+
let cfg = r#"import { defineConfig } from 'vite-plus';
592+
593+
export default defineConfig(({ mode }) => {
594+
const configObject = { fmt: { singleQuote: true } };
595+
return configObject;
596+
});
597+
"#;
598+
assert!(!has_config_key(cfg, "fmt").unwrap());
599+
}
600+
601+
#[test]
602+
fn test_has_config_key_arrow_wrapper_around_defineconfig() {
603+
// export default () => defineConfig({ ... }) — the wrapper is irrelevant;
604+
// detection follows the defineConfig argument object.
605+
let cfg = r#"import { defineConfig } from 'vite-plus';
606+
607+
export default () =>
608+
defineConfig({
609+
fmt: { singleQuote: true },
610+
});
611+
"#;
612+
assert!(has_config_key(cfg, "fmt").unwrap());
613+
}
614+
615+
#[test]
616+
fn test_has_config_key_fate_template_shape() {
617+
// Mirrors create-fate's drizzle template — the bug that motivated this fix.
618+
let cfg = r#"import { defineConfig } from 'vite-plus';
619+
620+
export default defineConfig({
621+
fmt: {
622+
experimentalSortImports: { newlinesBetween: false },
623+
ignorePatterns: ['coverage/', 'dist/'],
624+
singleQuote: true,
625+
},
626+
lint: {
627+
extends: [nkzw],
628+
options: { typeAware: true, typeCheck: true },
629+
rules: { '@typescript-eslint/no-explicit-any': 'off' },
630+
},
631+
staged: { '*': 'vp check --fix' },
632+
});
633+
"#;
634+
assert!(has_config_key(cfg, "fmt").unwrap());
635+
assert!(has_config_key(cfg, "lint").unwrap());
636+
assert!(has_config_key(cfg, "staged").unwrap());
637+
assert!(!has_config_key(cfg, "pack").unwrap());
638+
}
639+
361640
#[test]
362641
fn test_check_function_callback() {
363642
let simple_config = r#"

packages/cli/binding/index.cjs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -767,6 +767,7 @@ if (!nativeBinding) {
767767
module.exports = nativeBinding;
768768
module.exports.detectWorkspace = nativeBinding.detectWorkspace;
769769
module.exports.downloadPackageManager = nativeBinding.downloadPackageManager;
770+
module.exports.hasConfigKey = nativeBinding.hasConfigKey;
770771
module.exports.mergeJsonConfig = nativeBinding.mergeJsonConfig;
771772
module.exports.mergeTsdownConfig = nativeBinding.mergeTsdownConfig;
772773
module.exports.rewriteEslint = nativeBinding.rewriteEslint;

packages/cli/binding/index.d.cts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,16 @@ export interface DownloadPackageManagerResult {
116116
version: string;
117117
}
118118

119+
/**
120+
* Whether `config_key` is already declared as a top-level property in the
121+
* vite config's `defineConfig({...})` (or equivalent) object literal.
122+
*
123+
* AST-based check covering the six shapes the merger understands; ignores
124+
* comments, string literal occurrences, and nested keys. Returns `false`
125+
* for unrecognized shapes (e.g. `return $VAR` from a callback).
126+
*/
127+
export declare function hasConfigKey(viteConfigPath: string, configKey: string): boolean;
128+
119129
/** Result returned by JavaScript resolver functions. */
120130
export interface JsCommandResolvedResult {
121131
binPath: string;

packages/cli/binding/src/migration.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,18 @@ pub fn merge_json_config(
120120
})
121121
}
122122

123+
/// Whether `config_key` is already declared as a top-level property in the
124+
/// vite config's `defineConfig({...})` (or equivalent) object literal.
125+
///
126+
/// AST-based check covering the six shapes the merger understands; ignores
127+
/// comments, string literal occurrences, and nested keys. Returns `false`
128+
/// for unrecognized shapes (e.g. `return $VAR` from a callback).
129+
#[napi]
130+
pub fn has_config_key(vite_config_path: String, config_key: String) -> Result<bool> {
131+
let content = std::fs::read_to_string(&vite_config_path).map_err(anyhow::Error::from)?;
132+
Ok(vite_migration::has_config_key(&content, &config_key).map_err(anyhow::Error::from)?)
133+
}
134+
123135
/// Error from batch import rewriting
124136
#[napi(object)]
125137
pub struct BatchRewriteError {
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"singleQuote": false,
3+
"printWidth": 80,
4+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"rules": {
3+
"no-unused-vars": "error"
4+
},
5+
"options": {
6+
"typeAware": false
7+
}
8+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"devDependencies": {
3+
"oxfmt": "1",
4+
"oxlint": "1"
5+
}
6+
}

0 commit comments

Comments
 (0)