Conversation
Includes detection logic, cleanup helpers, and Biome configuration inference based on existing ESLint/Prettier settings.
Adds the migrate command to the CLI, orchestrating the migration process including detection, installation, config migration, and cleanup.
Updates README and CHANGELOG with details about the new migrate command features and usage.
Add robust error handling for missing package.json files and improve async file operations throughout the migration utilities. Handle cancelled prompts gracefully and expand CI testing to Node.js 18, 20, and 22. Key improvements: - Safe package.json reading with fallback handling - Async file existence checks in project detection - Proper cancellation handling in user prompts - Merge globals instead of overwriting existing ones - Move semver to production dependencies - Add proper test cleanup in unit tests
WalkthroughThis PR adds a full ESLint/Prettier → Biome migration flow: a new Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as CLI (src/cli.ts)
participant Migrate as Migrate (src/commands/migrate.ts)
participant Detect as Detect (src/utils/detect-project.ts)
participant Config as BiomeConfig (src/utils/biome-config.ts)
participant Generators as Generators (src/generators/*)
participant Cleanup as Cleanup (src/generators/cleanup.ts)
participant Validate as Validate (src/utils/validate-migration.ts)
participant Git as GitUtils (src/utils/git-safety.ts)
User->>CLI: run `biome migrate`
CLI->>Migrate: runMigrate(options)
Migrate->>Detect: detectProject()
Detect-->>Migrate: ProjectInfo
Migrate->>User: show findings / dry-run summary
alt user confirms
Migrate->>Git: isGitRepo(), hasUncommittedChanges()
Git-->>Migrate: status
Migrate->>Git: createSafetyCommit() (if enabled)
Migrate->>Config: readPrettierConfig(), prompt relaxed rules
Config-->>Migrate: formatter settings
Migrate->>Generators: migrateESLint(), migratePrettier() (respect dry-run)
Generators-->>Migrate: migration results
Migrate->>Cleanup: cleanupOldDeps() (if enabled)
Cleanup-->>Migrate: deps/files removed
Migrate->>Validate: validateBiomeVersion(), validateMigration()
Validate-->>Migrate: validation results
Migrate->>User: final summary & next steps
else user cancels
Migrate->>User: abort message
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (17)
src/generators/migrate-eslint.ts (1)
5-36: Consider enhancing error information.The error handling currently only captures the error message, but the
execaerror object contains additional useful information likestderr,exitCode, andcommandthat could help with debugging migration failures.Apply this diff to capture more error details:
} catch (error) { - const errorMessage = error instanceof Error ? error.message : "Unknown error"; + const errorMessage = + error instanceof Error + ? error.message + : "Unknown error"; + const stderr = (error as any)?.stderr; s.stop(pc.red("✗ Failed to migrate ESLint")); return { success: false, - message: `ESLint migration failed: ${errorMessage}`, + message: `ESLint migration failed: ${errorMessage}${stderr ? `\n${stderr}` : ""}`, }; }src/utils/validate-migration.ts (2)
67-86: Consider caching package.json read.The function reads
package.jsontwice (lines 68-76 and 79-86). Consider reading it once and reusing the parsed object to improve efficiency.Apply this diff:
// 2. Verify that @biomejs/biome is installed const pkgPath = path.join(cwd, "package.json"); + let pkg: any = null; + if (await fs.pathExists(pkgPath)) { - const pkg = await fs.readJson(pkgPath); + pkg = await fs.readJson(pkgPath); const hasBiomeDep = pkg.devDependencies?.["@biomejs/biome"] || pkg.dependencies?.["@biomejs/biome"]; if (!hasBiomeDep) { issues.push("@biomejs/biome not found in dependencies"); } } // 3. Check if scripts are updated - if (await fs.pathExists(pkgPath)) { - const pkg = await fs.readJson(pkgPath); + if (pkg) { const lintScript = pkg.scripts?.lint; if (!lintScript?.includes("biome")) { issues.push('package.json "lint" script does not use Biome'); } }
89-94: Clarify the biome check behavior.The comment says "We only care that it runs" but the try-catch silently swallows all errors. Consider logging or adding a more specific comment about what types of failures are acceptable (lint errors) vs. what might indicate a real problem (command not found, syntax errors).
Consider adding more context:
// 4. Run biome check to verify it works try { await execa("npx", ["@biomejs/biome", "check", ".", "--max-diagnostics=0"], { cwd }); } catch { - // Biome check might have lint errors, but that's OK - // We only care that it runs + // Biome check might report lint/format errors, which is expected and OK + // We only verify that the command executes (config is valid, Biome is installed) + // Command-not-found or syntax errors in biome.json would be caught by earlier checks }src/utils/biome-config.ts (2)
218-337: Consider refactoring the large applyPrettierCompatibility function.This function is 120 lines long and handles multiple responsibilities: reading configs, detecting folders, transforming patterns, prompting users, and writing config. Consider breaking it into smaller, focused functions for better maintainability and testability.
For example, extract:
buildExclusionPatterns(cwd, eslintIgnorePatterns, generatedFolders)applyFormatterSettings(config, prettierConfig, trailingCommasOverride)applyRelaxedRules(config)addTestGlobals(config)This would make the main function more readable and each piece easier to test in isolation.
311-327: LGTM with a suggestion.The logic to add Jest/Vitest globals is sound. The use of
Setensures no duplicates. Consider adding a comment explaining that these globals are always added regardless of whether the project uses Jest/Vitest, which might be intentional for compatibility but could cause confusion.Consider adding a clarifying comment:
// Add Jest/Vitest globals for test files (merge with existing) + // Note: These are added proactively even if tests aren't detected, + // to avoid "undefined variable" errors when tests are added later config.javascript = config.javascript || {};tests/unit/cleanup.test.ts (1)
86-97: Consider adding edge case for empty/undefined package object.The test for no matching deps is good, but consider adding a test case for when
pkgis empty{}or whendependencies/devDependenciesare explicitlyundefinedto ensure robustness.it("handles empty package object", () => { const pkg = {}; const result = findDepsToRemove(pkg); expect(result).toHaveLength(0); });src/utils/git-safety.ts (1)
33-36: HandleisCancelfrom the confirm prompt.If the user cancels the prompt (e.g., Ctrl+C),
shouldCommitwill be a symbol, not a boolean. The current checkshouldCommit === truehandles this safely by returningfalse, but explicitly handling cancellation would be more intentional.+import { confirm, isCancel } from "@clack/prompts"; const shouldCommit = await confirm({ message: "You have uncommitted changes. Create safety commit before migrating?", initialValue: true, }); + if (isCancel(shouldCommit)) { + return false; + } + if (shouldCommit === true) {src/generators/cleanup.ts (4)
38-47: Missing error handling for uninstall command failure.If the package manager command fails (e.g., network issues, permission errors), the error will propagate unhandled. Consider catching and re-throwing with a more descriptive message, or letting the caller handle it explicitly.
export async function uninstallDeps( deps: string[], pm: PackageManager, cwd: string, ): Promise<void> { if (deps.length === 0) return; const uninstallCmd = pm === "npm" ? "uninstall" : "remove"; - await execa(pm, [uninstallCmd, ...deps], { cwd }); + try { + await execa(pm, [uninstallCmd, ...deps], { cwd }); + } catch (error) { + const errorMessage = error instanceof Error ? error.message : "Unknown error"; + throw new Error(`Failed to uninstall packages: ${errorMessage}`); + } }
49-84: Unused parameter_infoshould be removed or utilized.The
_infoparameter is prefixed with underscore indicating it's unused. IfProjectInfocontext isn't needed here, remove the parameter entirely to avoid confusion.-export async function removeConfigFiles(_info: ProjectInfo, cwd: string): Promise<string[]> { +export async function removeConfigFiles(cwd: string): Promise<string[]> {Note: This would require updating the call site in
cleanupOldDepsat line 162.
105-114: Handle user cancellation fromconfirmprompt.The
confirmfunction from@clack/promptscan return a symbol when the user cancels (Ctrl+C). Currently only checking=== true, but not handling the cancel case gracefully.+import { confirm, spinner, isCancel } from "@clack/prompts"; ... const shouldUpdate = await confirm({ message: "Update package.json scripts to use Biome?", initialValue: true, }); + if (isCancel(shouldUpdate)) { + return { updated: false, scripts: newScripts }; + } + if (shouldUpdate === true) {
146-159: Handle user cancellation fromconfirmprompt.Similar to
updatePackageScripts, theconfirmcall here doesn't handle the cancel case explicitly. When user presses Ctrl+C,shouldRemovewill be a symbol, nottrueorfalse.const shouldRemove = await confirm({ message: `Remove ${depsToRemove.length} ESLint/Prettier packages?`, initialValue: true, }); + if (isCancel(shouldRemove)) { + return { depsRemoved: [], filesRemoved: [] }; + } + if (shouldRemove === true) {src/utils/detect-project.ts (3)
18-29: Missing.prettierrc.tomlconfiguration file.Prettier supports TOML format for configuration files, which is not included in the list.
const PRETTIER_CONFIGS = [ ".prettierrc", ".prettierrc.json", ".prettierrc.yaml", ".prettierrc.yml", ".prettierrc.js", ".prettierrc.cjs", ".prettierrc.mjs", + ".prettierrc.toml", "prettier.config.js", "prettier.config.cjs", "prettier.config.mjs", ];
64-65: Consider detectingbiome.jsoncin addition tobiome.json.Biome supports JSONC format (JSON with comments), and some projects may use
biome.jsoncinstead.// Detect existing Biome config (async) - const hasBiome = await fs.pathExists(path.join(cwd, "biome.json")); + const hasBiome = + (await fs.pathExists(path.join(cwd, "biome.json"))) || + (await fs.pathExists(path.join(cwd, "biome.jsonc")));
6-16: Addeslint.config.ts,eslint.config.mts, andeslint.config.ctsto the ESLint config detection list.ESLint v9.18.0+ officially supports TypeScript configuration files. Including these extensions ensures the utility detects ESLint configs in projects using TypeScript configuration files (requires jiti v2.0+ for Node.js).
src/commands/migrate.ts (3)
128-129: Hardcodednpxmay not work correctly with all package managers.The code uses the detected
packageManagerfor installation but switches tonpxfor runningbiome init. This could cause issues with pnpm or yarn workspaces.+ const runCmd = projectInfo.packageManager === "npm" ? "npx" : + projectInfo.packageManager === "pnpm" ? "pnpm" : + projectInfo.packageManager === "yarn" ? "yarn" : "bunx"; + if (!projectInfo.hasBiome) { const s3 = spinner(); s3.start("Initializing Biome configuration..."); try { - await execa("npx", ["@biomejs/biome", "init"], { cwd }); + await execa(runCmd, ["@biomejs/biome", "init"], { cwd }); s3.stop(pc.green("✓ Created biome.json"));Apply the same pattern at line 204 for
biome check --write.
192-210: Missing cancellation handling for the formatting confirmation.If the user cancels with Ctrl+C during this prompt,
applyFormattingwill be a symbol, and the code will silently skip formatting. Consider handling cancellation explicitly.const applyFormatting = await confirm({ message: "Biome may format code differently than Prettier.\n" + "Apply Biome formatting now? (recommended, creates one reformatting commit)", initialValue: true, }); + if (isCancel(applyFormatting)) { + cancel("Migration cancelled"); + return; + } + if (applyFormatting === true) {
100-114: Check package.json for existing @biomejs/biome dependency before installation.The
hasBiomecheck only verifies the existence ofbiome.json, but ignores whether@biomejs/biomeis already listed inpackage.jsondependencies. If the package is already declared butbiome.jsondoesn't exist, the installation will still run unnecessarily. Consider checking bothpackage.jsondevDependencies/dependencies (similar to the logic invalidate-migration.tslines 70-71) alongside thebiome.jsoncheck to avoid redundant installation attempts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
CHANGELOG.md(1 hunks)README.md(2 hunks)package.json(1 hunks)src/cli.ts(2 hunks)src/commands/migrate.ts(1 hunks)src/generators/cleanup.ts(1 hunks)src/generators/migrate-eslint.ts(1 hunks)src/generators/migrate-prettier.ts(1 hunks)src/types/index.ts(1 hunks)src/utils/biome-config.ts(1 hunks)src/utils/detect-project.ts(1 hunks)src/utils/git-safety.ts(1 hunks)src/utils/validate-migration.ts(1 hunks)tests/fixtures/eslint-project/.eslintrc.json(1 hunks)tests/fixtures/eslint-project/package.json(1 hunks)tests/fixtures/prettier-project/.prettierrc(1 hunks)tests/fixtures/prettier-project/package.json(1 hunks)tests/unit/cleanup.test.ts(1 hunks)tests/unit/detect-project.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
tests/unit/detect-project.test.ts (1)
src/utils/detect-project.ts (3)
detectProject(31-76)getESLintConfigs(78-80)getPrettierConfigs(82-84)
tests/unit/cleanup.test.ts (1)
src/generators/cleanup.ts (1)
findDepsToRemove(25-36)
src/generators/cleanup.ts (2)
src/types/index.ts (2)
PackageManager(1-1)ProjectInfo(27-35)src/utils/detect-project.ts (2)
getESLintConfigs(78-80)getPrettierConfigs(82-84)
src/cli.ts (1)
src/commands/migrate.ts (1)
runMigrate(17-233)
src/utils/detect-project.ts (2)
src/types/index.ts (1)
ProjectInfo(27-35)src/utils/detect-pm.ts (1)
detectPackageManager(17-36)
🔇 Additional comments (26)
tests/fixtures/prettier-project/.prettierrc (1)
1-7: LGTM!This test fixture provides a representative Prettier configuration for testing the migration workflow. The settings are standard and appropriate.
README.md (1)
7-73: LGTM!The documentation clearly explains both the create and migrate workflows with comprehensive command options and expected outcomes. The structure is user-friendly and informative.
CHANGELOG.md (1)
10-26: LGTM!The changelog comprehensively documents the new migration features. The format follows Keep a Changelog conventions and provides clear, detailed information about the new functionality.
src/types/index.ts (1)
19-43: LGTM!The new type definitions are well-structured and provide clear contracts for the migration workflow. The field names are descriptive and types are appropriate.
src/utils/validate-migration.ts (2)
10-19: LGTM!The version extraction logic correctly handles different output formats from the Biome CLI. The regex pattern is appropriate and the error handling gracefully returns null if extraction fails.
21-49: LGTM!The version validation logic correctly uses
semver.gte()to ensure the installed Biome version meets the minimum requirement. The error messages are clear and actionable.src/utils/biome-config.ts (4)
6-28: LGTM!The type definitions accurately represent Prettier and Biome configuration options. The interfaces are well-structured and cover the necessary properties for format migration.
43-85: LGTM!The Prettier config reading logic appropriately handles multiple config file locations and formats, with proper fallback to package.json. The error handling gracefully continues searching if a config file is invalid.
91-138: LGTM!The ESLint ignore patterns reader correctly handles multiple sources (.eslintrc, .eslintrc.json, package.json, .eslintignore) with appropriate error handling. The .eslintignore file parsing correctly filters comments and empty lines.
260-284: All formatter options being set are supported by Biome 1.7.0. Biome supports indentStyle, indentWidth, and lineWidth as standard formatter options, and bracketSpacing, lineEnding, and trailingCommas are documented configuration options. The JavaScript formatter options quoteStyle, arrowParentheses, trailingCommas, bracketSpacing, and semicolons are all supported. No compatibility concerns exist with the target version.package.json (1)
30-31: Package versions are secure and appropriate for the migration.The specified versions
semver@^7.7.3andpicocolors@^1.1.1are secure. The semver version is well above the fix threshold for known vulnerabilities (CVE-2022-25883 was fixed in 7.5.2), and no current security advisories apply to these versions. The caret operator allows compatible updates within the major version, ensuring minor and patch improvements are accessible without breaking changes.tests/fixtures/eslint-project/package.json (1)
1-13: LGTM!Well-structured test fixture for ESLint detection. The devDependencies cover the main ESLint ecosystem packages that the migration tool needs to detect and clean up.
tests/fixtures/prettier-project/package.json (1)
1-11: LGTM!Good test fixture for Prettier detection. Including
prettier-plugin-tailwindcssensures the cleanup pattern matching forprettier-*packages is also tested.tests/fixtures/eslint-project/.eslintrc.json (1)
1-25: LGTM!Realistic ESLint configuration that exercises various detection and migration scenarios: extends, plugins, custom rules, TypeScript parser, and React support. Good coverage for testing the migration workflow.
tests/unit/cleanup.test.ts (1)
1-114: LGTM!Comprehensive test coverage for the
findDepsToRemovefunction. Tests cover all major ESLint/Prettier package patterns and verify that unrelated packages liketypescriptare correctly excluded.src/cli.ts (1)
38-52: LGTM!Clean integration of the migrate command with well-named options that are consistent with the existing
createcommand patterns. The options mapping torunMigrateis straightforward and correct.tests/unit/detect-project.test.ts (1)
1-132: Good test coverage for project detection utilities.The tests cover all major configuration file formats for ESLint and Prettier, plus edge cases like package.json embedded configs and lockfile-based package manager detection. The temp directory isolation pattern is well-implemented.
src/generators/migrate-prettier.ts (1)
5-36: LGTM!Clean implementation that follows the same pattern as
migrateESLint. The spinner feedback, error handling, and return structure are consistent with the codebase conventions. ThedryRunparameter is appropriately handled since the migrate command exits early during dry-run mode before calling this function.src/utils/git-safety.ts (1)
4-20: LGTM!The
isGitRepoandhasUncommittedChangesutilities are well-implemented with appropriate error handling. Returningfalseon errors is a safe default for these checks.src/generators/cleanup.ts (2)
10-19: LGTM! Regex patterns are well-designed for package detection.The patterns correctly match ESLint and Prettier related packages using appropriate anchors and prefixes.
21-36: LGTM! Clean implementation.The
matchesPatternshelper andfindDepsToRemovelogic are clear and correctly aggregate dependencies from bothdependenciesanddevDependencies.src/utils/detect-project.ts (2)
31-76: LGTM! Well-structured detection logic.The function correctly:
- Handles missing/invalid package.json gracefully
- Detects both file-based and package.json-embedded configs
- Returns a comprehensive ProjectInfo object
- Uses async operations appropriately
78-84: LGTM! Simple accessor functions.These helpers expose the config lists for use in other modules like cleanup.ts.
src/commands/migrate.ts (3)
17-32: LGTM! Good initial validation.The function correctly detects project setup and exits early with a clear message if there's nothing to migrate.
53-70: LGTM! Dry-run mode provides useful preview.The dry-run implementation clearly shows what changes would be made without modifying files.
227-232: LGTM! Clear and helpful completion message.The outro provides actionable next steps with the lint command.
| // 9. Read Prettier config BEFORE cleanup (so we can preserve settings) | ||
| const prettierConfig = await readPrettierConfig(cwd); | ||
| if (prettierConfig) { | ||
| console.log(pc.dim(" Read Prettier settings to preserve formatting preferences")); | ||
| } |
There was a problem hiding this comment.
Prettier config is read after migration, but migration may have already removed the file.
If migratePrettier removes or modifies the Prettier config file as part of migration, readPrettierConfig at line 158 might not find the original settings. The comment says "BEFORE cleanup" but Prettier migration happens at line 150.
Consider reading the Prettier config earlier, before any migration steps:
+ // Read Prettier config BEFORE migration (so we can preserve settings)
+ const prettierConfig = projectInfo.hasPrettier ? await readPrettierConfig(cwd) : null;
+ if (prettierConfig) {
+ console.log(pc.dim(" Read Prettier settings to preserve formatting preferences"));
+ }
+
// 7. Migrate ESLint
if (projectInfo.hasESLint) {
...
}
// 8. Migrate Prettier
if (projectInfo.hasPrettier) {
...
}
- // 9. Read Prettier config BEFORE cleanup (so we can preserve settings)
- const prettierConfig = await readPrettierConfig(cwd);
- if (prettierConfig) {
- console.log(pc.dim(" Read Prettier settings to preserve formatting preferences"));
- }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/commands/migrate.ts around lines 157 to 161, readPrettierConfig is
executed after migration/cleanup steps (contrary to the comment) which may
remove or change the Prettier config; move the call to readPrettierConfig so it
runs before any migratePrettier or cleanup operations (i.e., before line ~150),
store the result for later use, and ensure any errors are handled or ignored
gracefully so logging about preserved settings remains valid.
| export function mapPrettierToBiome( | ||
| prettier: PrettierConfig, | ||
| trailingCommasOverride?: "none" | "all", | ||
| ): BiomeFormatterSettings { | ||
| // If es5, require override; otherwise map directly | ||
| let trailingCommas: "none" | "all" = "all"; | ||
| if (trailingCommasOverride) { | ||
| trailingCommas = trailingCommasOverride; | ||
| } else if (prettier.trailingComma === "none") { | ||
| trailingCommas = "none"; | ||
| } else if (prettier.trailingComma === "all") { | ||
| trailingCommas = "all"; | ||
| } | ||
|
|
||
| return { | ||
| quoteStyle: prettier.singleQuote ? "single" : "double", | ||
| trailingCommas, | ||
| lineWidth: prettier.printWidth || 80, | ||
| indentWidth: prettier.tabWidth || 2, | ||
| indentStyle: prettier.useTabs ? "tab" : "space", | ||
| semicolons: prettier.semi === false ? "asNeeded" : "always", | ||
| bracketSpacing: prettier.bracketSpacing !== false, | ||
| arrowParentheses: prettier.arrowParens === "avoid" ? "asNeeded" : "always", | ||
| lineEnding: prettier.endOfLine === "crlf" ? "crlf" : prettier.endOfLine === "cr" ? "cr" : "lf", | ||
| }; | ||
| } |
There was a problem hiding this comment.
Verify default trailingCommas behavior.
When prettier.trailingComma is undefined and no override is provided, the function defaults to "all" (line 149). However, Prettier's default is "es5". This could lead to unexpected behavior when migrating projects without an explicit trailingComma setting.
Consider this adjustment:
): BiomeFormatterSettings {
// If es5, require override; otherwise map directly
- let trailingCommas: "none" | "all" = "all";
+ let trailingCommas: "none" | "all" = "all"; // Default to "all" to match common preference
if (trailingCommasOverride) {
trailingCommas = trailingCommasOverride;
} else if (prettier.trailingComma === "none") {
trailingCommas = "none";
} else if (prettier.trailingComma === "all") {
trailingCommas = "all";
+ } else if (prettier.trailingComma === undefined) {
+ // Prettier defaults to "es5", which Biome doesn't support
+ // Default to "all" as it's closer to "es5" behavior
+ trailingCommas = "all";
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function mapPrettierToBiome( | |
| prettier: PrettierConfig, | |
| trailingCommasOverride?: "none" | "all", | |
| ): BiomeFormatterSettings { | |
| // If es5, require override; otherwise map directly | |
| let trailingCommas: "none" | "all" = "all"; | |
| if (trailingCommasOverride) { | |
| trailingCommas = trailingCommasOverride; | |
| } else if (prettier.trailingComma === "none") { | |
| trailingCommas = "none"; | |
| } else if (prettier.trailingComma === "all") { | |
| trailingCommas = "all"; | |
| } | |
| return { | |
| quoteStyle: prettier.singleQuote ? "single" : "double", | |
| trailingCommas, | |
| lineWidth: prettier.printWidth || 80, | |
| indentWidth: prettier.tabWidth || 2, | |
| indentStyle: prettier.useTabs ? "tab" : "space", | |
| semicolons: prettier.semi === false ? "asNeeded" : "always", | |
| bracketSpacing: prettier.bracketSpacing !== false, | |
| arrowParentheses: prettier.arrowParens === "avoid" ? "asNeeded" : "always", | |
| lineEnding: prettier.endOfLine === "crlf" ? "crlf" : prettier.endOfLine === "cr" ? "cr" : "lf", | |
| }; | |
| } | |
| export function mapPrettierToBiome( | |
| prettier: PrettierConfig, | |
| trailingCommasOverride?: "none" | "all", | |
| ): BiomeFormatterSettings { | |
| // If es5, require override; otherwise map directly | |
| let trailingCommas: "none" | "all" = "all"; // Default to "all" to match common preference | |
| if (trailingCommasOverride) { | |
| trailingCommas = trailingCommasOverride; | |
| } else if (prettier.trailingComma === "none") { | |
| trailingCommas = "none"; | |
| } else if (prettier.trailingComma === "all") { | |
| trailingCommas = "all"; | |
| } else if (prettier.trailingComma === undefined) { | |
| // Prettier defaults to "es5", which Biome doesn't support | |
| // Default to "all" as it's closer to "es5" behavior | |
| trailingCommas = "all"; | |
| } | |
| return { | |
| quoteStyle: prettier.singleQuote ? "single" : "double", | |
| trailingCommas, | |
| lineWidth: prettier.printWidth || 80, | |
| indentWidth: prettier.tabWidth || 2, | |
| indentStyle: prettier.useTabs ? "tab" : "space", | |
| semicolons: prettier.semi === false ? "asNeeded" : "always", | |
| bracketSpacing: prettier.bracketSpacing !== false, | |
| arrowParentheses: prettier.arrowParens === "avoid" ? "asNeeded" : "always", | |
| lineEnding: prettier.endOfLine === "crlf" ? "crlf" : prettier.endOfLine === "cr" ? "cr" : "lf", | |
| }; | |
| } |
…ng, refine Biome exclusion patterns, and adjust git staging.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/commands/migrate.ts (2)
157-161: Prettier config read after migration may fail.The comment at line 157 states "BEFORE cleanup," but
readPrettierConfigis called aftermigratePrettier(line 150), which may have already removed or modified the Prettier configuration file. This could result in reading stale or missing config data.Move the
readPrettierConfigcall before the migration steps to ensure the original Prettier settings are captured:+ // Read Prettier config BEFORE migration (so we can preserve settings) + const prettierConfig = projectInfo.hasPrettier ? await readPrettierConfig(cwd) : null; + if (prettierConfig) { + console.log(pc.dim(" Read Prettier settings to preserve formatting preferences")); + } + // 7. Migrate ESLint if (projectInfo.hasESLint) { const eslintResult = await migrateESLint(cwd); result.eslintMigrated = eslintResult.success; if (!eslintResult.success) { note(eslintResult.message, "ESLint migration warning"); } } // 8. Migrate Prettier if (projectInfo.hasPrettier) { const prettierResult = await migratePrettier(cwd); result.prettierMigrated = prettierResult.success; if (!prettierResult.success) { note(prettierResult.message, "Prettier migration warning"); } } - // 9. Read Prettier config BEFORE cleanup (so we can preserve settings) - const prettierConfig = await readPrettierConfig(cwd); - if (prettierConfig) { - console.log(pc.dim(" Read Prettier settings to preserve formatting preferences")); - }
169-171: Validation result not used.The validation result from
validateMigrationis not captured or used. This means cleanup proceeds regardless of validation outcome, which could remove dependencies even if the migration didn't complete successfully.Consider either:
- Capture the result and skip cleanup if validation fails:
// 12. Validate migration before cleanup - await validateMigration(cwd); + const validation = await validateMigration(cwd); // 13. Cleanup old dependencies (only if migration was successful) - if (!options.skipCleanup) { + if (!options.skipCleanup && validation.success) { const cleanup = await cleanupOldDeps(projectInfo, cwd); result.depsRemoved = cleanup.depsRemoved; result.filesRemoved = cleanup.filesRemoved; }
- Or remove the variable assignment entirely if validation is purely informational:
// 12. Validate migration (informational only) await validateMigration(cwd);
🧹 Nitpick comments (1)
src/utils/biome-config.ts (1)
144-169: Consider documenting the default trailingCommas behavior.The function defaults to
"all"for trailing commas whenprettier.trailingCommais undefined (line 149). While functionally correct, this differs from Prettier's default of"es5"(which Biome doesn't support). Adding a comment would clarify this intentional choice.): BiomeFormatterSettings { // If es5, require override; otherwise map directly - let trailingCommas: "none" | "all" = "all"; + // Default to "all" since Biome doesn't support Prettier's default "es5" + // "all" is closer to "es5" behavior than "none" + let trailingCommas: "none" | "all" = "all"; if (trailingCommasOverride) { trailingCommas = trailingCommasOverride; } else if (prettier.trailingComma === "none") { trailingCommas = "none"; } else if (prettier.trailingComma === "all") { trailingCommas = "all"; + } else if (prettier.trailingComma === undefined) { + // Explicitly handle undefined: keep default "all" + trailingCommas = "all"; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/commands/migrate.ts(1 hunks)src/generators/cleanup.ts(1 hunks)src/utils/biome-config.ts(1 hunks)src/utils/git-safety.ts(1 hunks)tests/unit/detect-project.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/generators/cleanup.ts
- tests/unit/detect-project.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/commands/migrate.ts (8)
src/types/index.ts (2)
MigrateOptions(20-25)MigrationResult(37-43)src/utils/detect-project.ts (1)
detectProject(31-76)src/utils/git-safety.ts (1)
createSafetyCommit(22-51)src/utils/validate-migration.ts (2)
validateBiomeVersion(21-49)validateMigration(51-107)src/generators/migrate-eslint.ts (1)
migrateESLint(5-36)src/generators/migrate-prettier.ts (1)
migratePrettier(5-36)src/utils/biome-config.ts (3)
readPrettierConfig(43-85)promptRelaxedRules(342-349)applyPrettierCompatibility(218-337)src/generators/cleanup.ts (2)
cleanupOldDeps(119-174)updatePackageScripts(86-117)
🔇 Additional comments (13)
src/utils/git-safety.ts (3)
4-11: LGTM!The function correctly detects Git repositories and handles errors gracefully by returning
false.
13-20: LGTM!The function correctly detects uncommitted changes and handles errors safely.
22-51: Previous concern addressed!The function now correctly uses
git add -uat line 40, which only stages modified and deleted tracked files. This addresses the previous review concern about staging untracked files unintentionally. The logic properly checks for a Git repository, verifies uncommitted changes, prompts the user, and creates a safety commit.src/commands/migrate.ts (5)
17-52: LGTM!The project detection and display logic correctly identifies ESLint/Prettier configurations and provides clear feedback to the user about what was detected.
54-89: LGTM!The dry-run mode and safety commit flow are well implemented. The user is properly informed of changes before they're applied, and the git safety commit provides a rollback point.
91-137: LGTM!The Biome installation and initialization flow correctly handles conditional installation, version validation, and configuration initialization with proper error handling.
173-190: LGTM!The cleanup and script update logic correctly handles optional cleanup and provides helpful feedback when scripts aren't automatically updated.
192-233: LGTM!The optional formatting step and migration summary provide excellent user experience. The clear warning about formatting differences and the comprehensive summary help users understand what changed.
src/utils/biome-config.ts (5)
43-85: LGTM!The function correctly reads Prettier configuration from various file formats and safely handles errors. The decision to skip JavaScript config files is reasonable since they cannot be statically parsed.
91-138: LGTM!The function correctly reads ESLint ignore patterns from multiple sources (config files, package.json, .eslintignore file) and properly filters out comments and empty lines.
174-191: LGTM!The function provides a clear user prompt with helpful hints to resolve the "es5" trailing comma incompatibility, and safely handles cancellation.
196-213: LGTM!The function correctly detects common generated folders and formats them as Biome exclusion patterns.
218-337: LGTM!The function comprehensively applies Prettier compatibility settings to the Biome configuration. The ESLint ignore pattern normalization (lines 240-246) correctly filters existing negations and creates new exclusion patterns. The function properly:
- Merges exclusion patterns from multiple sources
- Maps Prettier settings to Biome equivalents
- Applies optional relaxed lint rules
- Configures test globals
- Disables organize imports during migration
1 participant
migratecommand — Migrate existing projects from ESLint/Prettier to Biomebiome migrate eslint)biome migrate prettier)gen/,dist/,build/)--dry-runmode for previewing changesSummary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.