Open
Conversation
Signed-off-by: Emilien Escalle <emilien.escalle@escemi.com>
There was a problem hiding this comment.
Pull request overview
This PR migrates the repo (and the generated consumer-project setup) from ESLint/Prettier-based tooling to Biome, including adding a core migration to update existing installations and removing legacy ESLint config generation.
Changes:
- Add a new core migration (
20260311120000-migrate-to-biome) to convert managed ESLint/Prettier/lint-staged setup to Biome, update git hooks, and patch common Vite starter templates. - Remove ESLint configs/plugins and update package scripts/dev-deps across
core,react, and root workspace to use Biome. - Update Node/TS baselines (Node
>=18, TS target/lib toes2022) and reformat many files to the new formatting conventions.
Reviewed changes
Copilot reviewed 74 out of 78 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Raise TS target/lib to ES2022 to match modern runtime expectations. |
| packages/react/src/react.e2e.spec.ts | Update E2E assertions for Biome config and removal of Prettier config. |
| packages/react/src/snapshots/react.e2e.spec.ts.snap | Update E2E snapshots for Biome-driven install output and scripts. |
| packages/react/package.json | Switch lint scripts to Biome; add Node engines constraint; remove ESLint/Prettier config/deps. |
| packages/react/biome.json | Add Biome configuration for the React package. |
| packages/react/eslint.config.mjs | Remove legacy ESLint config file. |
| packages/react/src/eslint-plugin-ts-dev-tools/index.ts | Remove legacy ESLint plugin bundle for React. |
| packages/react/src/install/migrations/20240412185501-eslint-config.ts | Convert React ESLint-config migration into a no-op post-Biome migration. |
| packages/react/src/install/migrations/20240412185501-eslint-config.spec.ts | Update migration test expectations after ESLint config removal. |
| packages/react/src/install/migrations/snapshots/20240412185501-eslint-config.spec.ts.snap | Update snapshots for no-op migration behavior. |
| packages/react/src/install/migrations/20201111162698-init.ts | Formatting/type-only import adjustments in migration code. |
| packages/react/src/install/migrations/20201111162698-init.spec.ts | Formatting changes in migration tests. |
| packages/core/package.json | Add Biome dependency and Biome-based lint scripts; add Node engines constraint; remove ESLint/Prettier deps/config. |
| packages/core/biome.json | Add Biome configuration for the Core package. |
| packages/core/eslint.config.mjs | Remove legacy ESLint config file. |
| packages/core/src/eslint-plugin-ts-dev-tools/index.ts | Remove legacy Core ESLint plugin bundle. |
| packages/core/src/install/migrations/20260311120000-migrate-to-biome.ts | New migration to transition managed projects to Biome (scripts/config/hooks/template patches). |
| packages/core/src/install/migrations/20260311120000-migrate-to-biome.spec.ts | Add unit tests covering the Biome migration scenarios. |
| packages/core/src/install/migrations/snapshots/20260311120000-migrate-to-biome.spec.ts.snap | Add snapshots for Biome migration outputs (package.json, biome.json, hook content). |
| packages/core/src/install/migrations/20240412185500-eslint-config.ts | Stop generating eslint.config.mjs; keep only package.json eslintConfig cleanup. |
| packages/core/src/install/migrations/20240412185500-eslint-config.spec.ts | Update tests to stop asserting generated ESLint config file. |
| packages/core/src/install/migrations/snapshots/20240412185500-eslint-config.spec.ts.snap | Update snapshots after removing generated ESLint config file output. |
| packages/core/src/install/migrations/20250623095500-add-prettier-oxc.ts | Type-only import + formatting adjustments. |
| packages/core/src/install/migrations/20250623095500-add-prettier-oxc.spec.ts | Formatting changes in migration tests. |
| packages/core/src/install/migrations/20250623095600-remove-prettier-oxc.ts | Type-only import + formatting adjustments. |
| packages/core/src/install/migrations/20250623095600-remove-prettier-oxc.spec.ts | Formatting changes in migration tests. |
| packages/core/src/install/migrations/20240617094000-config-nx-scopes.ts | Formatting changes; keep package-install logic intact. |
| packages/core/src/install/migrations/20240617094000-config-nx-scopes.spec.ts | Formatting changes in migration tests. |
| packages/core/src/install/migrations/20240329200200-eslint-ignore.ts | Type-only import + formatting adjustments. |
| packages/core/src/install/migrations/20240329200200-eslint-ignore.spec.ts | Formatting changes in migration tests. |
| packages/core/src/install/migrations/20220617100200-prettier-cache.ts | Type-only import + formatting adjustments. |
| packages/core/src/install/migrations/20220617100200-prettier-cache.spec.ts | Formatting changes in migration tests. |
| packages/core/src/install/migrations/20201024173398-init.ts | Type-only import + formatting updates; keep hook setup logic. |
| packages/core/src/install/migrations/20201024173398-init.spec.ts | Formatting changes in migration tests. |
| packages/core/src/services/package-manager/AbstractPackageManagerAdapter.ts | Refactor command execution to avoid unnecessary shell usage and improve parsing. |
| packages/core/src/services/package-manager/YarnPackageManagerAdapter.ts | Formatting/refactor; retain monorepo detection logic and fallbacks. |
| packages/core/src/services/package-manager/NpmPackageManagerAdapter.ts | Formatting/refactor; retain monorepo detection and install checks. |
| packages/core/src/services/package-manager/PackageManagerAdapter.ts | Formatting-only interface changes. |
| packages/core/src/services/package-manager/PackageManagerType.ts | Formatting-only enum changes. |
| packages/core/src/services/PackageManagerService.ts | Formatting-only changes; keep adapter routing behavior. |
| packages/core/src/services/PackageManagerService.spec.ts | Formatting changes in tests. |
| packages/core/src/services/SymlinkDependenciesService.ts | Improve logging and formatting; adjust typing import style. |
| packages/core/src/services/SymlinkDependenciesService.spec.ts | Formatting changes in tests. |
| packages/core/src/services/snapshots/SymlinkDependenciesService.spec.ts.snap | Update snapshot output to reflect new symlinking behavior/logging. |
| packages/core/src/services/PeerDependenciesService.ts | Formatting updates and type-only import adjustments. |
| packages/core/src/services/PeerDependenciesService.spec.ts | Formatting updates in tests. |
| packages/core/src/services/PluginService.ts | Formatting updates and node: import changes. |
| packages/core/src/services/PluginService.spec.ts | Formatting updates in tests. |
| packages/core/src/services/DuplicateDependenciesService.ts | Formatting updates and type-only import adjustments. |
| packages/core/src/services/DuplicateDependenciesService.spec.ts | Formatting updates in tests. |
| packages/core/src/services/PackageJson.ts | Formatting + node:path import changes; minor string-template cleanups. |
| packages/core/src/services/PackageJson.spec.ts | Formatting updates in tests and minor string-template expectation updates. |
| packages/core/src/services/PackageJsonMerge.ts | Switch to Object.hasOwn and formatting refactor. |
| packages/core/src/services/PackageJsonMerge.spec.ts | Formatting updates in tests. |
| packages/core/src/services/MigrationsService.ts | Formatting updates and type-only import adjustments. |
| packages/core/src/services/MigrationsService.spec.ts | Formatting updates in tests. |
| packages/core/src/services/snapshots/MigrationsService.spec.ts.snap | Update migration execution snapshots to include the new Biome migration. |
| packages/core/src/services/GitService.ts | Add hook update capability and modernize fs/path imports. |
| packages/core/src/services/GitService.spec.ts | Add tests for updating managed hooks and adjust permissions assertions. |
| packages/core/src/services/FileService.ts | Modernize to node:fs import path. |
| packages/core/src/services/FileService.spec.ts | Formatting updates in tests and path import changes. |
| packages/core/src/services/CorePackageService.ts | Formatting updates and node:path import changes. |
| packages/core/src/services/CmdService.ts | Modernize child_process/fs imports and minor callback style updates. |
| packages/core/src/tests/test-project.ts | Modernize node: imports and refactor string concatenations/formatting. |
| packages/core/src/tests/test-project-monorepo.ts | Modernize node:path import and formatting/type-only import adjustment. |
| packages/core/src/tests/test-packages.ts | Modernize node: imports and refactor a few path operations for formatting. |
| packages/core/src/tests/test-cache.ts | Modernize node: imports and formatting updates. |
| packages/core/src/tests/file-system.ts | Modernize node:fs import path. |
| packages/core/src/tests/cli.ts | Modernize node:child_process import and small refactor/formatting changes. |
| packages/core/src/core.e2e.spec.ts | Update E2E checks to assert Biome config exists and Prettier config is absent. |
| packages/core/src/snapshots/core.e2e.spec.ts.snap | Update E2E snapshots for Biome migration output and scripts. |
| packages/core/src/bin.ts | Modernize node:path import and format version printing. |
| packages/core/src/bin.spec.ts | Modernize node:path import and formatting updates. |
| packages/core/src/install/command.ts | Modernize node: imports and formatting updates. |
| packages/core/src/install/command.spec.ts | Formatting updates in tests and assertion formatting changes. |
| package.json | Add Node engines, add Biome devDependency, and update lint-staged to run Biome. |
| .devcontainer/devcontainer.json | Update devcontainer name/image to match current environment choice. |
Comments suppressed due to low confidence (1)
packages/core/src/tests/cli.ts:19
- The
codefield is typed asnumber, buterror.codefrom Node'sExecExceptioncan be a string in some cases (e.g., ENOENT). This makes the return type inaccurate and can propagate unexpected non-numeric values. Consider normalizing to a numeric exit code (or changing the type tonumber | string).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+26
to
+29
| console.warn("Cache is disabled. Enable it one dev is done."); | ||
| } | ||
| if (!shouldCleanupAfterTest) { | ||
| console.warn("Cleanup is disabled. Enable it one dev is done."); |
There was a problem hiding this comment.
The warning text says "Enable it one dev is done." which reads like a typo/grammar issue. Consider changing "one" to "once" (and possibly rephrasing) for clarity.
Suggested change
| console.warn("Cache is disabled. Enable it one dev is done."); | |
| } | |
| if (!shouldCleanupAfterTest) { | |
| console.warn("Cleanup is disabled. Enable it one dev is done."); | |
| console.warn("Cache is disabled. Enable it once development is done."); | |
| } | |
| if (!shouldCleanupAfterTest) { | |
| console.warn("Cleanup is disabled. Enable it once development is done."); |
Comment on lines
+98
to
+114
| return new Promise((resolve, reject) => { | ||
| const child = spawn(cmd, cmdArgs, { | ||
| stdio: silent ? "pipe" : "inherit", | ||
| shell: useShell, | ||
| windowsVerbatimArguments: true, | ||
| cwd, | ||
| }); | ||
|
|
||
| const hasShellSyntax = args.some((arg) => { | ||
| const trimmedArg = arg.trim(); | ||
| return shellOperators.some( | ||
| (op) => | ||
| trimmedArg.startsWith(op) || | ||
| trimmedArg.endsWith(op) || | ||
| trimmedArg.includes(` ${op} `) || | ||
| trimmedArg.includes(` ${op}`) || | ||
| trimmedArg.includes(`${op} `) | ||
| ); | ||
| }); | ||
| let output = ""; | ||
| let error = ""; | ||
|
|
||
| if (hasShellSyntax) { | ||
| cmd = args.join(" ").trim(); | ||
| cmdArgs = []; | ||
| useShell = true; | ||
| } else { | ||
| cmd = args[0]; | ||
| cmdArgs = args.slice(1); | ||
| } | ||
| } else { | ||
| cmd = args; | ||
| cmdArgs = []; | ||
| useShell = true; | ||
| child.on("exit", (code) => { | ||
| if (code) { | ||
| return reject(output.length > 0 ? output : error); | ||
| } | ||
| resolve(output); | ||
| }); |
There was a problem hiding this comment.
execCommand only listens for the child process exit event; if spawn() fails (e.g., package manager binary not found), Node emits an error event and exit may never fire, leaving the returned Promise pending forever. Add a child.on("error", ...) handler (and consider close vs exit) to always resolve/reject deterministically.
Comment on lines
+13
to
+90
| const BIOME_INIT_COMMAND = "npx @biomejs/biome init"; | ||
|
|
||
| const OLD_PRE_COMMIT_COMMAND = | ||
| "npx --no-install lint-staged && npx --no-install pretty-quick --staged"; | ||
| const NEW_PRE_COMMIT_COMMAND = "npx --no-install biome check --staged --write"; | ||
|
|
||
| const MANAGED_ESLINT_MARKERS = [ | ||
| "tsDevToolsCore", | ||
| "tsDevToolsReact", | ||
| "export default tsDevTools", | ||
| ]; | ||
|
|
||
| const VITE_VANILLA_APP_ELEMENT_PATTERN = | ||
| "document.querySelector<HTMLDivElement>('#app')!.innerHTML = `"; | ||
| const VITE_VANILLA_APP_ELEMENT_REPLACEMENT = `const appElement = document.querySelector<HTMLDivElement>("#app"); | ||
|
|
||
| if (!appElement) { | ||
| throw new Error("Could not find #app element"); | ||
| } | ||
|
|
||
| appElement.innerHTML = \``; | ||
| const VITE_VANILLA_COUNTER_PATTERN = | ||
| "setupCounter(document.querySelector<HTMLButtonElement>('#counter')!)"; | ||
| const VITE_VANILLA_COUNTER_REPLACEMENT = `const counterElement = document.querySelector<HTMLButtonElement>("#counter"); | ||
|
|
||
| if (!counterElement) { | ||
| throw new Error("Could not find #counter element"); | ||
| } | ||
|
|
||
| setupCounter(counterElement)`; | ||
| const VITE_REACT_ROOT_PATTERN = | ||
| "createRoot(document.getElementById('root')!).render("; | ||
| const VITE_REACT_ROOT_REPLACEMENT = `const rootElement = document.getElementById("root"); | ||
|
|
||
| if (!rootElement) { | ||
| throw new Error("Could not find #root element"); | ||
| } | ||
|
|
||
| createRoot(rootElement).render(`; | ||
| const VITE_REACT_BUTTON_PATTERN = "<button onClick={"; | ||
| const VITE_REACT_BUTTON_REPLACEMENT = '<button type="button" onClick={'; | ||
|
|
||
| export const up: MigrationUpFunction = async ( | ||
| absoluteProjectDir: string, | ||
| ): Promise<void> => { | ||
| const packageJson = PackageJson.fromDirPath(absoluteProjectDir); | ||
| const packageJsonContent = packageJson.getContent(); | ||
| const hasManagedEslintConfigFile = | ||
| projectHasManagedEslintConfigFile(absoluteProjectDir); | ||
|
|
||
| const hasBiomeConfig = projectHasBiomeConfig(absoluteProjectDir); | ||
| if (!hasBiomeConfig) { | ||
| await runBiomeInit(absoluteProjectDir); | ||
| } | ||
|
|
||
| delete packageJsonContent.eslintConfig; | ||
| delete packageJsonContent.prettier; | ||
| delete packageJsonContent.importSort; | ||
| delete packageJsonContent["lint-staged"]; | ||
|
|
||
| packageJsonContent.scripts = { | ||
| ...packageJsonContent.scripts, | ||
| format: "biome format --write .", | ||
| lint: "biome lint .", | ||
| check: "biome check --write .", | ||
| }; | ||
|
|
||
| packageJson.setContent(packageJsonContent); | ||
|
|
||
| enableBiomeVcsIntegration(absoluteProjectDir); | ||
| migrateCommonViteStarterFiles(absoluteProjectDir); | ||
| deleteManagedEslintConfig(absoluteProjectDir, hasManagedEslintConfigFile); | ||
| updateManagedPreCommitHook(absoluteProjectDir); | ||
| }; | ||
|
|
||
| async function runBiomeInit(absoluteProjectDir: string): Promise<void> { | ||
| await CmdService.execCmd(BIOME_INIT_COMMAND, absoluteProjectDir, true); | ||
| } |
There was a problem hiding this comment.
runBiomeInit shells out to npx @biomejs/biome init, which makes the migration outcome depend on the Biome version available at runtime and can require network access in some environments. For a migration, prefer generating a known biome.json template (or invoking npx --no-install biome init with a locally provided Biome binary) to keep installs deterministic and offline-friendly.
Comment on lines
+99
to
+107
| const biomeConfigContent = JSON.parse( | ||
| FileService.getFileContent(biomeConfigFilePath), | ||
| ) as { | ||
| vcs?: { | ||
| clientKind?: string; | ||
| enabled?: boolean; | ||
| useIgnoreFile?: boolean; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
enableBiomeVcsIntegration uses JSON.parse on biome.json without any error handling. If the existing file is invalid JSON (partial write, manual edit, etc.), the migration will throw and can leave the project in a partially-migrated state. Consider wrapping the parse in try/catch and either skipping with a warning or preserving the original file content.
|
|
||
| function addRelNoopenerToBlankTargetLinks(content: string): string { | ||
| return content.replaceAll( | ||
| /target="_blank"(?!\s+rel=)/g, |
There was a problem hiding this comment.
addRelNoopenerToBlankTargetLinks only checks for rel= immediately following target="_blank". If a link already has a rel attribute later in the tag (e.g., target="_blank" class="..." rel="noopener"), this will add a duplicate rel and produce invalid HTML. Consider using a pattern that verifies no rel= exists anywhere before the closing > of the same tag, or a safer tag-level replacement approach.
Suggested change
| /target="_blank"(?!\s+rel=)/g, | |
| /target="_blank"(?![^>]*\srel=)/g, |
Comment on lines
+33
to
+37
| const gitHookDirPath = join(absoluteProjectDir, ".git/hooks"); | ||
| const gitHookFilePath = join(gitHookDirPath, gitHookName); | ||
| const gitHookFilePath = GitService.getGitHookFilePath( | ||
| absoluteProjectDir, | ||
| gitHookName, | ||
| ); |
There was a problem hiding this comment.
gitHookDirPath is declared but never used in addGitHook. It looks like leftover from when gitHookFilePath was built from the directory path; removing it would avoid dead code and potential future lint/noUnusedLocals issues.
Comment on lines
+29
to
+31
| throw new Error( | ||
| `An error occured while executing command "${cmd}": ${error}`, | ||
| ); |
There was a problem hiding this comment.
Typo in error message: "occured" should be "occurred".
Member
Author
|
@copilot rebase from main, resolve the conflicts and handle code review feedbacks |
- Merge origin/main into feat/migrate-to-biome - Update dependency versions to match main (newer versions) - Fix typos: 'one dev is done' -> 'once development is done' - Fix typo: 'occured' -> 'occurred' in cli.ts - Remove unused gitHookDirPath variable in GitService.ts - Add error event handler and use close event in AbstractPackageManagerAdapter - Add try/catch for JSON.parse in enableBiomeVcsIntegration - Fix addRelNoopenerToBlankTargetLinks regex to handle rel= anywhere in tag - Use --no-install flag in biome init command for deterministic offline use Co-authored-by: neilime <314088+neilime@users.noreply.github.com>
Contributor
Done in commit 92e4f0c. Merged from main (resolving conflicts in package.json files to keep newer dependency versions from main while preserving Biome changes) and addressed all code review feedback:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.