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..2b67b610bd 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,93 @@ 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) +} + +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" => text.trim_matches(|c| c == '"' || c == '\'' || c == '`') == config_key, + _ => false, + } +} + +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_statement" => true, + // `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_statement" => is_inside_define_config_callback(&parent), + _ => false, + } +} + +fn is_define_config_call(call_node: &Node<'_, D>) -> bool { + call_node.kind() == "call_expression" + && call_node.field("function").is_some_and(|f| f.text() == "defineConfig") +} + +fn is_define_config_arrow_body(paren_node: &Node<'_, D>) -> bool { + 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)) +} + +fn is_inside_define_config_callback(node: &Node<'_, D>) -> bool { + let mut current = node.parent(); + while let Some(n) = current { + if 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 +446,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..6b9e3af932 100644 --- a/packages/cli/binding/src/migration.rs +++ b/packages/cli/binding/src/migration.rs @@ -120,6 +120,18 @@ 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)?; + Ok(vite_migration::has_config_key(&content, &config_key).map_err(anyhow::Error::from)?) +} + /// Error from batch import rewriting #[napi(object)] pub struct BatchRewriteError { 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..aea6e47aad --- /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/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 45b094caa7..c710d0b1d3 100644 --- a/packages/cli/src/migration/__tests__/migrator.spec.ts +++ b/packages/cli/src/migration/__tests__/migrator.spec.ts @@ -1441,3 +1441,50 @@ 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); + }); +}); diff --git a/packages/cli/src/migration/migrator.ts b/packages/cli/src/migration/migrator.ts index a771fbe7e3..bb891296fc 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, @@ -2065,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); @@ -2097,6 +2095,21 @@ 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. + // 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( + `${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);