From 4b2a1db8c10bd7a40831c4b8786d57c275b9652c Mon Sep 17 00:00:00 2001 From: MK Date: Sat, 16 May 2026 16:14:59 +0800 Subject: [PATCH 1/4] fix(cli): skip merging standalone oxfmt/oxlint config when key already 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. --- .../.oxfmtrc.jsonc | 4 + .../.oxlintrc.json | 8 ++ .../package.json | 6 ++ .../snap.txt | 63 ++++++++++++++++ .../steps.json | 8 ++ .../vite.config.ts | 48 ++++++++++++ .../src/migration/__tests__/migrator.spec.ts | 75 +++++++++++++++++++ packages/cli/src/migration/migrator.ts | 14 ++++ 8 files changed, 226 insertions(+) create mode 100644 packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/.oxfmtrc.jsonc create mode 100644 packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/.oxlintrc.json create mode 100644 packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/package.json create mode 100644 packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/snap.txt create mode 100644 packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/steps.json create mode 100644 packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/vite.config.ts diff --git a/packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/.oxfmtrc.jsonc b/packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/.oxfmtrc.jsonc new file mode 100644 index 0000000000..7271d69042 --- /dev/null +++ b/packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/.oxfmtrc.jsonc @@ -0,0 +1,4 @@ +{ + "singleQuote": false, + "printWidth": 80 +} diff --git a/packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/.oxlintrc.json b/packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/.oxlintrc.json new file mode 100644 index 0000000000..232525913e --- /dev/null +++ b/packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/.oxlintrc.json @@ -0,0 +1,8 @@ +{ + "rules": { + "no-unused-vars": "error" + }, + "options": { + "typeAware": false + } +} diff --git a/packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/package.json b/packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/package.json new file mode 100644 index 0000000000..77272a6f34 --- /dev/null +++ b/packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/package.json @@ -0,0 +1,6 @@ +{ + "devDependencies": { + "oxfmt": "1", + "oxlint": "1" + } +} diff --git a/packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/snap.txt b/packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/snap.txt new file mode 100644 index 0000000000..33bf3d8916 --- /dev/null +++ b/packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/snap.txt @@ -0,0 +1,63 @@ +> vp migrate --no-interactive 2>&1 # should NOT duplicate fmt/lint blocks already in vite.config.ts (regression for vp create fate) +◇ Migrated . to Vite+ +• Node pnpm +• 1 config update applied + +> cat vite.config.ts # exactly one fmt: and one lint: block, preserving template values +import { defineConfig } from 'vite-plus'; + +export default defineConfig({ + staged: { + "*": "vp check --fix" + }, + fmt: { + experimentalSortImports: { + newlinesBetween: false, + }, + experimentalSortPackageJson: { + sortScripts: true, + }, + experimentalTailwindcss: { + stylesheet: 'client/src/App.css', + }, + ignorePatterns: [ + 'coverage/', + 'dist/', + '.fate/', + 'client/dist/', + 'client/src/translations/', + 'server/dist/', + 'pnpm-lock.yaml', + ], + singleQuote: true, + }, + lint: { + extends: ['@nkzw/oxlint-config'], + ignorePatterns: [ + 'coverage', + 'dist', + '.fate', + 'client/dist', + 'server/dist', + 'server/src/drizzle/migrations/**', + ], + options: { typeAware: true, typeCheck: true }, + overrides: [ + { + files: ['server/src/index.tsx', 'server/src/drizzle/seed.tsx', '**/__tests__/**'], + rules: { + 'no-console': 'off', + }, + }, + ], + rules: { + '@typescript-eslint/no-explicit-any': 'off', + }, + }, +}); + +> test ! -f .oxfmtrc.jsonc && echo '.oxfmtrc.jsonc removed' # redundant standalone file removed +.oxfmtrc.jsonc removed + +> test ! -f .oxlintrc.json && echo '.oxlintrc.json removed' # redundant standalone file removed +.oxlintrc.json removed diff --git a/packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/steps.json b/packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/steps.json new file mode 100644 index 0000000000..cd04d1c41b --- /dev/null +++ b/packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/steps.json @@ -0,0 +1,8 @@ +{ + "commands": [ + "vp migrate --no-interactive 2>&1 # should NOT duplicate fmt/lint blocks already in vite.config.ts (regression for vp create fate)", + "cat vite.config.ts # exactly one fmt: and one lint: block, preserving template values", + "test ! -f .oxfmtrc.jsonc && echo '.oxfmtrc.jsonc removed' # redundant standalone file removed", + "test ! -f .oxlintrc.json && echo '.oxlintrc.json removed' # redundant standalone file removed" + ] +} diff --git a/packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/vite.config.ts b/packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/vite.config.ts new file mode 100644 index 0000000000..c06fce4147 --- /dev/null +++ b/packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/vite.config.ts @@ -0,0 +1,48 @@ +import { defineConfig } from 'vite-plus'; + +export default defineConfig({ + fmt: { + experimentalSortImports: { + newlinesBetween: false, + }, + experimentalSortPackageJson: { + sortScripts: true, + }, + experimentalTailwindcss: { + stylesheet: 'client/src/App.css', + }, + ignorePatterns: [ + 'coverage/', + 'dist/', + '.fate/', + 'client/dist/', + 'client/src/translations/', + 'server/dist/', + 'pnpm-lock.yaml', + ], + singleQuote: true, + }, + lint: { + extends: ['@nkzw/oxlint-config'], + ignorePatterns: [ + 'coverage', + 'dist', + '.fate', + 'client/dist', + 'server/dist', + 'server/src/drizzle/migrations/**', + ], + options: { typeAware: true, typeCheck: true }, + overrides: [ + { + files: ['server/src/index.tsx', 'server/src/drizzle/seed.tsx', '**/__tests__/**'], + rules: { + 'no-console': 'off', + }, + }, + ], + rules: { + '@typescript-eslint/no-explicit-any': 'off', + }, + }, +}); diff --git a/packages/cli/src/migration/__tests__/migrator.spec.ts b/packages/cli/src/migration/__tests__/migrator.spec.ts index 45b094caa7..41cf6f552b 100644 --- a/packages/cli/src/migration/__tests__/migrator.spec.ts +++ b/packages/cli/src/migration/__tests__/migrator.spec.ts @@ -1441,3 +1441,78 @@ describe('rewriteStandaloneProject — tsconfig types rewriting', () => { ); }); }); + +// Regression: templates such as `create-fate` ship a populated vite.config.ts +// alongside a standalone `.oxfmtrc.jsonc` / `.oxlintrc.json`. The merge step +// must not insert a second `fmt:` / `lint:` block when one is already present. +describe('rewriteStandaloneProject — preserves existing fmt/lint blocks', () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'vp-test-merge-existing-')); + fs.writeFileSync( + path.join(tmpDir, 'package.json'), + JSON.stringify({ name: 'test', devDependencies: { vite: '^7.0.0' } }), + ); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('does not duplicate fmt block when vite.config.ts already has one and .oxfmtrc.jsonc exists', () => { + fs.writeFileSync( + path.join(tmpDir, 'vite.config.ts'), + `import { defineConfig } from 'vite-plus'; + +export default defineConfig({ + fmt: { + singleQuote: true, + }, +}); +`, + ); + fs.writeFileSync( + path.join(tmpDir, '.oxfmtrc.jsonc'), + JSON.stringify({ singleQuote: false }, null, 2), + ); + + rewriteStandaloneProject(tmpDir, makeWorkspaceInfo(tmpDir, PackageManager.pnpm), true, true); + + const viteConfig = fs.readFileSync(path.join(tmpDir, 'vite.config.ts'), 'utf8'); + expect(viteConfig.match(/\bfmt\s*:/g)?.length).toBe(1); + // Template-authored value wins (singleQuote: true) — standalone config dropped. + expect(viteConfig).toContain('singleQuote: true'); + expect(viteConfig).not.toContain('singleQuote: false'); + // Redundant standalone file removed. + expect(fs.existsSync(path.join(tmpDir, '.oxfmtrc.jsonc'))).toBe(false); + }); + + it('does not duplicate lint block when vite.config.ts already has one and .oxlintrc.json exists', () => { + fs.writeFileSync( + path.join(tmpDir, 'vite.config.ts'), + `import { defineConfig } from 'vite-plus'; + +export default defineConfig({ + lint: { + rules: { + 'no-console': 'warn', + }, + }, +}); +`, + ); + fs.writeFileSync( + path.join(tmpDir, '.oxlintrc.json'), + JSON.stringify({ rules: { 'no-unused-vars': 'error' } }, null, 2), + ); + + rewriteStandaloneProject(tmpDir, makeWorkspaceInfo(tmpDir, PackageManager.pnpm), true, true); + + const viteConfig = fs.readFileSync(path.join(tmpDir, 'vite.config.ts'), 'utf8'); + expect(viteConfig.match(/\blint\s*:/g)?.length).toBe(1); + expect(viteConfig).toContain("'no-console': 'warn'"); + expect(viteConfig).not.toContain("'no-unused-vars': 'error'"); + expect(fs.existsSync(path.join(tmpDir, '.oxlintrc.json'))).toBe(false); + }); +}); diff --git a/packages/cli/src/migration/migrator.ts b/packages/cli/src/migration/migrator.ts index a771fbe7e3..6d30508f13 100644 --- a/packages/cli/src/migration/migrator.ts +++ b/packages/cli/src/migration/migrator.ts @@ -2097,6 +2097,20 @@ function mergeAndRemoveJsonConfig( ): void { const fullViteConfigPath = path.join(projectPath, viteConfigPath); const fullJsonConfigPath = path.join(projectPath, jsonConfigPath); + // Skip merge when the key is already present in vite.config.ts — the Rust + // merge step always prepends, so without this guard a template that ships + // both an inline `${configKey}:` block and a standalone JSON file (e.g. + // create-fate's vite.config.ts + .oxfmtrc.jsonc) ends up with two of them. + const viteConfigContent = fs.readFileSync(fullViteConfigPath, 'utf8'); + if (new RegExp(`\\b${configKey}\\s*:`).test(viteConfigContent)) { + fs.unlinkSync(fullJsonConfigPath); + if (!silent) { + prompts.log.info( + `${configKey} config already present in ${displayRelative(fullViteConfigPath)} — removed redundant ${displayRelative(fullJsonConfigPath)}`, + ); + } + return; + } const result = mergeJsonConfig(fullViteConfigPath, fullJsonConfigPath, configKey); if (result.updated) { fs.writeFileSync(fullViteConfigPath, result.content); From 55d76e557ae04ea625a759772d32cf6c1a11ceb7 Mon Sep 17 00:00:00 2001 From: MK Date: Sat, 16 May 2026 16:37:57 +0800 Subject: [PATCH 2/4] refactor(cli): replace regex key-presence check with AST-based has_config_key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. --- crates/vite_migration/src/lib.rs | 2 +- crates/vite_migration/src/vite_config.rs | 301 +++++++++++++++++++++++ packages/cli/binding/index.cjs | 1 + packages/cli/binding/index.d.cts | 10 + packages/cli/binding/src/migration.rs | 14 ++ packages/cli/src/migration/migrator.ts | 6 +- 6 files changed, 331 insertions(+), 3 deletions(-) diff --git a/crates/vite_migration/src/lib.rs b/crates/vite_migration/src/lib.rs index c8ed9be841..95ab4c66f3 100644 --- a/crates/vite_migration/src/lib.rs +++ b/crates/vite_migration/src/lib.rs @@ -18,4 +18,4 @@ mod vite_config; pub use file_walker::{WalkResult, find_ts_files}; pub use import_rewriter::{BatchRewriteResult, rewrite_imports_in_directory}; pub use package::{rewrite_eslint, rewrite_prettier, rewrite_scripts}; -pub use vite_config::{MergeResult, merge_json_config, merge_tsdown_config}; +pub use vite_config::{MergeResult, has_config_key, merge_json_config, merge_tsdown_config}; diff --git a/crates/vite_migration/src/vite_config.rs b/crates/vite_migration/src/vite_config.rs index 011a5ca4bb..024edf0815 100644 --- a/crates/vite_migration/src/vite_config.rs +++ b/crates/vite_migration/src/vite_config.rs @@ -1,6 +1,7 @@ use std::{borrow::Cow, path::Path, sync::LazyLock}; use ast_grep_config::{GlobalRules, RuleConfig, from_yaml_string}; +use ast_grep_core::{Doc, Node}; use ast_grep_language::{LanguageExt, SupportLang}; use regex::Regex; use vite_error::Error; @@ -128,6 +129,115 @@ fn strip_schema_property(config: &str) -> Cow<'_, str> { RE_SCHEMA.replace_all(config, "") } +/// Check whether `config_key` is already declared as a top-level property in +/// the vite config's `defineConfig({...})` (or equivalent) object literal. +/// +/// Mirrors the six shapes the merger understands (see `generate_merge_rule`): +/// `defineConfig({...})`, `defineConfig((p) => ({...}))`, `return {...}` +/// inside a `defineConfig` callback, `export default {...}`, and the +/// `satisfies` export variant. The `return $VAR` variant cannot be inspected +/// statically — for that shape we conservatively report `false`, which is +/// safe because the merger uses object spread (`{ key: ..., ...$VAR }`) so +/// duplicate keys are resolved at runtime by JS spread semantics. +/// +/// Returns `true` only when the key appears as a **direct** member of one of +/// those recognized object literals. Comments, string occurrences, nested +/// keys (e.g. `plugins: [{ fmt: ... }]`), and unrelated objects are all +/// ignored correctly. +pub fn has_config_key(vite_config_content: &str, config_key: &str) -> Result { + let grep = SupportLang::TypeScript.ast_grep(vite_config_content); + let root = grep.root(); + + for node in root.dfs() { + if node.kind() != "pair" { + continue; + } + let Some(key_node) = node.field("key") else { continue }; + if !pair_key_matches(&key_node, config_key) { + continue; + } + let Some(parent_object) = node.parent() else { continue }; + if parent_object.kind() != "object" { + continue; + } + if is_recognized_config_object(&parent_object) { + return Ok(true); + } + } + + Ok(false) +} + +/// Whether a `pair`'s key node matches `config_key` (handles bare-identifier +/// keys like `fmt:` and string keys like `'fmt':` / `"fmt":`). +fn pair_key_matches(key_node: &Node<'_, D>, config_key: &str) -> bool { + let text = key_node.text(); + match key_node.kind().as_ref() { + "property_identifier" => text == config_key, + "string" => { + // string node text includes the surrounding quotes + let stripped = text.trim_matches(|c| c == '"' || c == '\'' || c == '`'); + stripped == config_key + } + _ => false, + } +} + +/// Whether an `object` literal sits in one of the merger-recognized positions. +fn is_recognized_config_object(object_node: &Node<'_, D>) -> bool { + let Some(parent) = object_node.parent() else { return false }; + match parent.kind().as_ref() { + // export default { ... } + "export_statement" => true, + // export default { ... } satisfies T + "satisfies_expression" => { + parent.parent().map(|p| p.kind() == "export_statement").unwrap_or(false) + } + // defineConfig({ ... }) — object is a direct argument + "arguments" => parent.parent().map(|c| is_define_config_call(&c)).unwrap_or(false), + // defineConfig((p) => ({ ... })) — body of an arrow function with + // parenthesized object expression + "parenthesized_expression" => is_define_config_arrow_body(&parent), + // return { ... } inside a defineConfig callback + "return_statement" => is_inside_define_config_callback(&parent), + _ => false, + } +} + +/// Is `call_node` a call to `defineConfig(...)`? +fn is_define_config_call(call_node: &Node<'_, D>) -> bool { + if call_node.kind() != "call_expression" { + return false; + } + call_node.field("function").map(|f| f.text() == "defineConfig").unwrap_or(false) +} + +/// Is `paren_node` the body of an arrow function passed directly to +/// `defineConfig(...)` (e.g. `defineConfig((p) => ({...}))`)? +fn is_define_config_arrow_body(paren_node: &Node<'_, D>) -> bool { + let Some(arrow) = paren_node.parent() else { return false }; + if arrow.kind() != "arrow_function" { + return false; + } + let Some(args) = arrow.parent() else { return false }; + if args.kind() != "arguments" { + return false; + } + args.parent().map(|c| is_define_config_call(&c)).unwrap_or(false) +} + +/// Is `node` nested inside the body of a `defineConfig(...)` callback? +fn is_inside_define_config_callback(node: &Node<'_, D>) -> bool { + let mut current = node.parent(); + while let Some(n) = current { + if n.kind() == "call_expression" && is_define_config_call(&n) { + return true; + } + current = n.parent(); + } + false +} + /// Check if the vite config uses a function callback pattern fn check_function_callback(vite_config_content: &str) -> Result { // Match both sync and async arrow functions @@ -358,6 +468,197 @@ mod tests { use super::*; + // ── has_config_key ──────────────────────────────────────────────────── + + #[test] + fn test_has_config_key_top_level_in_defineconfig() { + let cfg = r#"import { defineConfig } from 'vite-plus'; + +export default defineConfig({ + fmt: { singleQuote: true }, + lint: { rules: {} }, +}); +"#; + assert!(has_config_key(cfg, "fmt").unwrap()); + assert!(has_config_key(cfg, "lint").unwrap()); + assert!(!has_config_key(cfg, "pack").unwrap()); + assert!(!has_config_key(cfg, "staged").unwrap()); + } + + #[test] + fn test_has_config_key_quoted_key() { + let cfg = r#"import { defineConfig } from 'vite-plus'; + +export default defineConfig({ + 'fmt': { singleQuote: true }, + "lint": {}, +}); +"#; + assert!(has_config_key(cfg, "fmt").unwrap()); + assert!(has_config_key(cfg, "lint").unwrap()); + } + + #[test] + fn test_has_config_key_ignores_comment_mentions() { + // The regex check was a false positive on these — AST check ignores them. + let cfg = r#"import { defineConfig } from 'vite-plus'; + +// fmt: configure formatter here +/* lint: TODO wire this up */ +export default defineConfig({ + plugins: [], +}); +"#; + assert!(!has_config_key(cfg, "fmt").unwrap()); + assert!(!has_config_key(cfg, "lint").unwrap()); + } + + #[test] + fn test_has_config_key_ignores_string_literal_mentions() { + let cfg = r#"import { defineConfig } from 'vite-plus'; + +export default defineConfig({ + plugins: [], + description: 'has fmt: foo and lint: bar inside', +}); +"#; + assert!(!has_config_key(cfg, "fmt").unwrap()); + assert!(!has_config_key(cfg, "lint").unwrap()); + } + + #[test] + fn test_has_config_key_ignores_nested_keys() { + // `fmt:` is a nested property inside `plugins[0].options`, not top-level. + let cfg = r#"import { defineConfig } from 'vite-plus'; + +export default defineConfig({ + plugins: [ + somePlugin({ + fmt: 'auto', + lint: { enabled: true }, + }), + ], +}); +"#; + assert!(!has_config_key(cfg, "fmt").unwrap()); + assert!(!has_config_key(cfg, "lint").unwrap()); + } + + #[test] + fn test_has_config_key_arrow_callback() { + let cfg = r#"import { defineConfig } from 'vite-plus'; + +export default defineConfig((env) => ({ + fmt: { singleQuote: env.mode === 'production' }, +})); +"#; + assert!(has_config_key(cfg, "fmt").unwrap()); + assert!(!has_config_key(cfg, "lint").unwrap()); + } + + #[test] + fn test_has_config_key_return_block_callback() { + let cfg = r#"import { defineConfig } from 'vite-plus'; + +export default defineConfig(({ mode }) => { + return { + fmt: { singleQuote: true }, + }; +}); +"#; + assert!(has_config_key(cfg, "fmt").unwrap()); + assert!(!has_config_key(cfg, "lint").unwrap()); + } + + #[test] + fn test_has_config_key_async_return_block_callback() { + let cfg = r#" +export default defineConfig(async ({ command, mode }) => { + const data = await asyncFunction(); + return { + lint: { rules: {} }, + }; +}); +"#; + assert!(has_config_key(cfg, "lint").unwrap()); + assert!(!has_config_key(cfg, "fmt").unwrap()); + } + + #[test] + fn test_has_config_key_plain_export() { + let cfg = r#"export default { + fmt: { singleQuote: true }, +}; +"#; + assert!(has_config_key(cfg, "fmt").unwrap()); + assert!(!has_config_key(cfg, "lint").unwrap()); + } + + #[test] + fn test_has_config_key_satisfies_export() { + let cfg = r#"import type { UserConfig } from 'vite-plus'; + +export default { + lint: { rules: {} }, +} satisfies UserConfig; +"#; + assert!(has_config_key(cfg, "lint").unwrap()); + assert!(!has_config_key(cfg, "fmt").unwrap()); + } + + #[test] + fn test_has_config_key_return_variable_is_unknown() { + // The merger handles this via object spread, so duplication is benign. + // We conservatively report `false`. + let cfg = r#"import { defineConfig } from 'vite-plus'; + +export default defineConfig(({ mode }) => { + const configObject = { fmt: { singleQuote: true } }; + return configObject; +}); +"#; + assert!(!has_config_key(cfg, "fmt").unwrap()); + } + + #[test] + fn test_has_config_key_arrow_wrapper_around_defineconfig() { + // export default () => defineConfig({ ... }) — the wrapper is irrelevant; + // detection follows the defineConfig argument object. + let cfg = r#"import { defineConfig } from 'vite-plus'; + +export default () => + defineConfig({ + fmt: { singleQuote: true }, + }); +"#; + assert!(has_config_key(cfg, "fmt").unwrap()); + } + + #[test] + fn test_has_config_key_fate_template_shape() { + // Mirrors create-fate's drizzle template — the bug that motivated this fix. + let cfg = r#"import { defineConfig } from 'vite-plus'; + +export default defineConfig({ + fmt: { + experimentalSortImports: { newlinesBetween: false }, + ignorePatterns: ['coverage/', 'dist/'], + singleQuote: true, + }, + lint: { + extends: [nkzw], + options: { typeAware: true, typeCheck: true }, + rules: { '@typescript-eslint/no-explicit-any': 'off' }, + }, + staged: { '*': 'vp check --fix' }, +}); +"#; + assert!(has_config_key(cfg, "fmt").unwrap()); + assert!(has_config_key(cfg, "lint").unwrap()); + assert!(has_config_key(cfg, "staged").unwrap()); + assert!(!has_config_key(cfg, "pack").unwrap()); + } + #[test] fn test_check_function_callback() { let simple_config = r#" diff --git a/packages/cli/binding/index.cjs b/packages/cli/binding/index.cjs index be16457fe6..8abc4ebbef 100644 --- a/packages/cli/binding/index.cjs +++ b/packages/cli/binding/index.cjs @@ -767,6 +767,7 @@ if (!nativeBinding) { module.exports = nativeBinding; module.exports.detectWorkspace = nativeBinding.detectWorkspace; module.exports.downloadPackageManager = nativeBinding.downloadPackageManager; +module.exports.hasConfigKey = nativeBinding.hasConfigKey; module.exports.mergeJsonConfig = nativeBinding.mergeJsonConfig; module.exports.mergeTsdownConfig = nativeBinding.mergeTsdownConfig; module.exports.rewriteEslint = nativeBinding.rewriteEslint; diff --git a/packages/cli/binding/index.d.cts b/packages/cli/binding/index.d.cts index 334e9b12f1..2050d90502 100644 --- a/packages/cli/binding/index.d.cts +++ b/packages/cli/binding/index.d.cts @@ -116,6 +116,16 @@ export interface DownloadPackageManagerResult { version: string; } +/** + * Whether `config_key` is already declared as a top-level property in the + * vite config's `defineConfig({...})` (or equivalent) object literal. + * + * AST-based check covering the six shapes the merger understands; ignores + * comments, string literal occurrences, and nested keys. Returns `false` + * for unrecognized shapes (e.g. `return $VAR` from a callback). + */ +export declare function hasConfigKey(viteConfigPath: string, configKey: string): boolean; + /** Result returned by JavaScript resolver functions. */ export interface JsCommandResolvedResult { binPath: string; diff --git a/packages/cli/binding/src/migration.rs b/packages/cli/binding/src/migration.rs index 73d0553328..2fdc7ca03f 100644 --- a/packages/cli/binding/src/migration.rs +++ b/packages/cli/binding/src/migration.rs @@ -120,6 +120,20 @@ pub fn merge_json_config( }) } +/// Whether `config_key` is already declared as a top-level property in the +/// vite config's `defineConfig({...})` (or equivalent) object literal. +/// +/// AST-based check covering the six shapes the merger understands; ignores +/// comments, string literal occurrences, and nested keys. Returns `false` +/// for unrecognized shapes (e.g. `return $VAR` from a callback). +#[napi] +pub fn has_config_key(vite_config_path: String, config_key: String) -> Result { + let content = std::fs::read_to_string(&vite_config_path).map_err(anyhow::Error::from)?; + let present = + vite_migration::has_config_key(&content, &config_key).map_err(anyhow::Error::from)?; + Ok(present) +} + /// Error from batch import rewriting #[napi(object)] pub struct BatchRewriteError { diff --git a/packages/cli/src/migration/migrator.ts b/packages/cli/src/migration/migrator.ts index 6d30508f13..61174fc822 100644 --- a/packages/cli/src/migration/migrator.ts +++ b/packages/cli/src/migration/migrator.ts @@ -9,6 +9,7 @@ import semver from 'semver'; import { Scalar, YAMLMap, YAMLSeq } from 'yaml'; import { + hasConfigKey, mergeJsonConfig, mergeTsdownConfig, rewriteEslint, @@ -2101,8 +2102,9 @@ function mergeAndRemoveJsonConfig( // merge step always prepends, so without this guard a template that ships // both an inline `${configKey}:` block and a standalone JSON file (e.g. // create-fate's vite.config.ts + .oxfmtrc.jsonc) ends up with two of them. - const viteConfigContent = fs.readFileSync(fullViteConfigPath, 'utf8'); - if (new RegExp(`\\b${configKey}\\s*:`).test(viteConfigContent)) { + // AST-based check ignores comments, string-literal occurrences, and nested + // keys (e.g. `plugins: [{ fmt: ... }]`). + if (hasConfigKey(fullViteConfigPath, configKey)) { fs.unlinkSync(fullJsonConfigPath); if (!silent) { prompts.log.info( From bb5b78792519b4f3c15068aa42e23690692f1a60 Mon Sep 17 00:00:00 2001 From: MK Date: Sat, 16 May 2026 16:46:31 +0800 Subject: [PATCH 3/4] refactor(cli): consolidate hasConfigKey usage and clean up Rust helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- crates/vite_migration/src/vite_config.rs | 50 ++++++------------- packages/cli/binding/src/migration.rs | 4 +- packages/cli/src/init-config.ts | 7 +-- .../src/migration/__tests__/migrator.spec.ts | 28 ----------- packages/cli/src/migration/migrator.ts | 7 +-- 5 files changed, 18 insertions(+), 78 deletions(-) diff --git a/crates/vite_migration/src/vite_config.rs b/crates/vite_migration/src/vite_config.rs index 024edf0815..2b67b610bd 100644 --- a/crates/vite_migration/src/vite_config.rs +++ b/crates/vite_migration/src/vite_config.rs @@ -168,69 +168,47 @@ pub fn has_config_key(vite_config_content: &str, config_key: &str) -> Result(key_node: &Node<'_, D>, config_key: &str) -> bool { let text = key_node.text(); match key_node.kind().as_ref() { "property_identifier" => text == config_key, - "string" => { - // string node text includes the surrounding quotes - let stripped = text.trim_matches(|c| c == '"' || c == '\'' || c == '`'); - stripped == config_key - } + "string" => text.trim_matches(|c| c == '"' || c == '\'' || c == '`') == config_key, _ => false, } } -/// Whether an `object` literal sits in one of the merger-recognized positions. fn is_recognized_config_object(object_node: &Node<'_, D>) -> bool { let Some(parent) = object_node.parent() else { return false }; match parent.kind().as_ref() { - // export default { ... } "export_statement" => true, - // export default { ... } satisfies T - "satisfies_expression" => { - parent.parent().map(|p| p.kind() == "export_statement").unwrap_or(false) - } - // defineConfig({ ... }) — object is a direct argument - "arguments" => parent.parent().map(|c| is_define_config_call(&c)).unwrap_or(false), - // defineConfig((p) => ({ ... })) — body of an arrow function with - // parenthesized object expression + // `export default { ... } satisfies T` — hop past the satisfies wrapper. + "satisfies_expression" => parent.parent().is_some_and(|p| p.kind() == "export_statement"), + "arguments" => parent.parent().is_some_and(|c| is_define_config_call(&c)), "parenthesized_expression" => is_define_config_arrow_body(&parent), - // return { ... } inside a defineConfig callback "return_statement" => is_inside_define_config_callback(&parent), _ => false, } } -/// Is `call_node` a call to `defineConfig(...)`? fn is_define_config_call(call_node: &Node<'_, D>) -> bool { - if call_node.kind() != "call_expression" { - return false; - } - call_node.field("function").map(|f| f.text() == "defineConfig").unwrap_or(false) + call_node.kind() == "call_expression" + && call_node.field("function").is_some_and(|f| f.text() == "defineConfig") } -/// Is `paren_node` the body of an arrow function passed directly to -/// `defineConfig(...)` (e.g. `defineConfig((p) => ({...}))`)? fn is_define_config_arrow_body(paren_node: &Node<'_, D>) -> bool { - let Some(arrow) = paren_node.parent() else { return false }; - if arrow.kind() != "arrow_function" { - return false; - } - let Some(args) = arrow.parent() else { return false }; - if args.kind() != "arguments" { - return false; - } - args.parent().map(|c| is_define_config_call(&c)).unwrap_or(false) + paren_node + .parent() + .filter(|n| n.kind() == "arrow_function") + .and_then(|n| n.parent()) + .filter(|n| n.kind() == "arguments") + .and_then(|n| n.parent()) + .is_some_and(|c| is_define_config_call(&c)) } -/// Is `node` nested inside the body of a `defineConfig(...)` callback? fn is_inside_define_config_callback(node: &Node<'_, D>) -> bool { let mut current = node.parent(); while let Some(n) = current { - if n.kind() == "call_expression" && is_define_config_call(&n) { + if is_define_config_call(&n) { return true; } current = n.parent(); diff --git a/packages/cli/binding/src/migration.rs b/packages/cli/binding/src/migration.rs index 2fdc7ca03f..6b9e3af932 100644 --- a/packages/cli/binding/src/migration.rs +++ b/packages/cli/binding/src/migration.rs @@ -129,9 +129,7 @@ pub fn merge_json_config( #[napi] pub fn has_config_key(vite_config_path: String, config_key: String) -> Result { let content = std::fs::read_to_string(&vite_config_path).map_err(anyhow::Error::from)?; - let present = - vite_migration::has_config_key(&content, &config_key).map_err(anyhow::Error::from)?; - Ok(present) + Ok(vite_migration::has_config_key(&content, &config_key).map_err(anyhow::Error::from)?) } /// Error from batch import rewriting diff --git a/packages/cli/src/init-config.ts b/packages/cli/src/init-config.ts index 15c4ad7e87..7991a0fa62 100644 --- a/packages/cli/src/init-config.ts +++ b/packages/cli/src/init-config.ts @@ -1,7 +1,7 @@ import fs from 'node:fs'; import path from 'node:path'; -import { mergeJsonConfig } from '../binding/index.js'; +import { hasConfigKey, mergeJsonConfig } from '../binding/index.js'; import { createDefaultVitePlusLintConfig } from './oxlint-plugin-config.ts'; import { fmt as resolveFmt } from './resolve-fmt.ts'; import { runCommandSilently } from './utils/command.ts'; @@ -134,11 +134,6 @@ export default defineConfig({}); return viteConfigPath; } -function hasConfigKey(viteConfigPath: string, configKey: string): boolean { - const viteConfig = fs.readFileSync(viteConfigPath, 'utf8'); - return new RegExp(`\\b${configKey}\\s*:`).test(viteConfig); -} - async function vpFmt(cwd: string, filePath: string): Promise { const { binPath, envs } = await resolveFmt(); const result = await runCommandSilently({ diff --git a/packages/cli/src/migration/__tests__/migrator.spec.ts b/packages/cli/src/migration/__tests__/migrator.spec.ts index 41cf6f552b..c710d0b1d3 100644 --- a/packages/cli/src/migration/__tests__/migrator.spec.ts +++ b/packages/cli/src/migration/__tests__/migrator.spec.ts @@ -1487,32 +1487,4 @@ export default defineConfig({ // Redundant standalone file removed. expect(fs.existsSync(path.join(tmpDir, '.oxfmtrc.jsonc'))).toBe(false); }); - - it('does not duplicate lint block when vite.config.ts already has one and .oxlintrc.json exists', () => { - fs.writeFileSync( - path.join(tmpDir, 'vite.config.ts'), - `import { defineConfig } from 'vite-plus'; - -export default defineConfig({ - lint: { - rules: { - 'no-console': 'warn', - }, - }, -}); -`, - ); - fs.writeFileSync( - path.join(tmpDir, '.oxlintrc.json'), - JSON.stringify({ rules: { 'no-unused-vars': 'error' } }, null, 2), - ); - - rewriteStandaloneProject(tmpDir, makeWorkspaceInfo(tmpDir, PackageManager.pnpm), true, true); - - const viteConfig = fs.readFileSync(path.join(tmpDir, 'vite.config.ts'), 'utf8'); - expect(viteConfig.match(/\blint\s*:/g)?.length).toBe(1); - expect(viteConfig).toContain("'no-console': 'warn'"); - expect(viteConfig).not.toContain("'no-unused-vars': 'error'"); - expect(fs.existsSync(path.join(tmpDir, '.oxlintrc.json'))).toBe(false); - }); }); diff --git a/packages/cli/src/migration/migrator.ts b/packages/cli/src/migration/migrator.ts index 61174fc822..bb891296fc 100644 --- a/packages/cli/src/migration/migrator.ts +++ b/packages/cli/src/migration/migrator.ts @@ -2066,11 +2066,8 @@ function injectConfigDefaults( report?: MigrationReport, ): void { const configs = detectConfigs(projectPath); - if (configs.viteConfig) { - const content = fs.readFileSync(path.join(projectPath, configs.viteConfig), 'utf8'); - if (new RegExp(`\\b${configKey}\\s*:`).test(content)) { - return; - } + if (configs.viteConfig && hasConfigKey(path.join(projectPath, configs.viteConfig), configKey)) { + return; } const viteConfig = ensureViteConfig(projectPath, configs, silent, report); From e0c99be9fc01c6af4d493731e4408de1b1d883ca Mon Sep 17 00:00:00 2001 From: MK Date: Sat, 16 May 2026 16:48:12 +0800 Subject: [PATCH 4/4] test(cli): add trailing comma to snap fixture so vp check passes --- .../migration-preserves-existing-fmt-and-lint/.oxfmtrc.jsonc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/.oxfmtrc.jsonc b/packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/.oxfmtrc.jsonc index 7271d69042..aea6e47aad 100644 --- a/packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/.oxfmtrc.jsonc +++ b/packages/cli/snap-tests-global/migration-preserves-existing-fmt-and-lint/.oxfmtrc.jsonc @@ -1,4 +1,4 @@ { "singleQuote": false, - "printWidth": 80 + "printWidth": 80, }