Skip to content

Commit a9d939c

Browse files
committed
fix(migrate): broaden ESLint ecosystem cleanup and address review findings
Address findings from the extra-high-effort review of this PR: - Iterate `peerDependencies` and `optionalDependencies` in addition to `devDependencies` and `dependencies`, matching the sister code path that handles vite/vitest overrides. - Narrow the `@typescript-eslint/*` namespace removal to the ESLint-specific entry points (`eslint-plugin`, `parser`, `rule-tester`); preserve the reusable AST libraries (`utils`, `typescript-estree`, `scope-manager`, `types`) so codemods and doc generators that import them directly keep working. - Recognize `@eslint/*`, `@eslint-community/*`, and `@angular-eslint/*` scopes as ESLint-only (e.g. `@eslint/js`, `@eslint/eslintrc`, `@eslint-community/eslint-utils`, `@angular-eslint/template-parser`). - Recognize flat ESLint-only helpers: `eslint-formatter-*`, `eslintrc`, `eslint-utils`, `eslint-visitor-keys`, `eslint-scope`, `eslint-define-config`, `eslint-doc-generator`. - Recognize `@nuxt/eslint` (Nuxt's ESLint integration module that `require`s `eslint` at runtime — useless after removal). The unit test that previously asserted this was preserved is now inverted. - Strip `@types/` symmetrically: a `@types/<X>` is removable iff `<X>` is. Fixes the inconsistency where `@types/eslint-plugin-foo` was removed but `@types/eslint` survived. - Delete a dependency field entirely (`devDependencies`, `peerDependencies`, etc.) when our cleanup emptied it, avoiding `"devDependencies": {}` noise. Visible in the `migration-eslint-monorepo` snap diff. - Split `rewriteEslintPackageJson` into root vs workspace modes: workspace sub-packages get a conservative cleanup (only `eslint` itself), since they may intentionally publish plugins/configs as part of a shared lint-preset API consumed outside Vite+. - Drop the broken `"include": ["src/**/*"]` from the `migration-eslint-type-aware` fixture's `tsconfig.json` (no `src/` directory exists). - Expand the `migration-eslint-plugins-cleanup` fixture to exercise the wider matrix (scopes, formatters, `@types`, preserved `@typescript-eslint/utils`, peer `eslint`). Deferred (out of scope for this PR): - `detectEslintProject` gating on the literal `eslint` symbol — a project that has only `typescript-eslint` etc. never triggers the migration. Pre-existing. - Stale `--rule '@typescript-eslint/...'` arguments in scripts after plugin removal — requires Rust ast-grep rule updates. - Non-atomic `fs.writeFileSync` across N workspace `package.json` files; a SIGKILL mid-loop leaves the monorepo half-migrated. Pre-existing utility behavior.
1 parent e50e1ae commit a9d939c

7 files changed

Lines changed: 239 additions & 32 deletions

File tree

packages/cli/snap-tests-global/migration-eslint-monorepo/snap.txt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,7 @@
4444
"name": "@test/utils",
4545
"scripts": {
4646
"lint": "vp lint ."
47-
},
48-
"devDependencies": {}
47+
}
4948
}
5049

5150
> test ! -f eslint.config.mjs # check root eslint config is removed

packages/cli/snap-tests-global/migration-eslint-plugins-cleanup/package.json

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,27 @@
66
"lint": "eslint ."
77
},
88
"devDependencies": {
9+
"@angular-eslint/template-parser": "^18.0.0",
10+
"@eslint-community/eslint-utils": "^4.0.0",
11+
"@eslint/js": "^9.0.0",
12+
"@nuxt/eslint": "^0.5.0",
13+
"@nuxt/kit": "^3.13.0",
14+
"@types/eslint": "^9.0.0",
15+
"@types/node": "^22.0.0",
916
"@typescript-eslint/eslint-plugin": "^8.0.0",
1017
"@typescript-eslint/parser": "^8.0.0",
18+
"@typescript-eslint/utils": "^8.0.0",
1119
"@vitejs/plugin-vue": "^6.0.0",
1220
"@vue/eslint-config-typescript": "^14.0.0",
1321
"eslint": "^9.0.0",
1422
"eslint-config-airbnb": "^19.0.0",
23+
"eslint-formatter-pretty": "^6.0.0",
1524
"eslint-plugin-vue": "^10.0.0",
1625
"typescript-eslint": "^8.0.0",
1726
"vite": "^7.0.0",
1827
"vue": "^3.5.0"
28+
},
29+
"peerDependencies": {
30+
"eslint": ">=9"
1931
}
2032
}

packages/cli/snap-tests-global/migration-eslint-plugins-cleanup/snap.txt

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
> vp migrate --no-interactive # migration should remove ESLint and its plugins/configs
1+
> vp migrate --no-interactive # migration should remove ESLint, plugins, configs, scopes, formatters, and peer eslint
22
◇ Migrated . to Vite+
33
• Node <semver> pnpm <semver>
44
• 4 config updates applied
55
• ESLint rules migrated to Oxlint
66
• TypeScript shim added for framework component files
77

8-
> cat package.json # check eslint, eslint-plugin-*, eslint-config-*, typescript-eslint and @typescript-eslint/* are removed
8+
> cat package.json # verify the comprehensive ESLint ecosystem cleanup
99
{
1010
"name": "migration-eslint-plugins-cleanup",
1111
"scripts": {
@@ -15,6 +15,9 @@
1515
"prepare": "vp config"
1616
},
1717
"devDependencies": {
18+
"@nuxt/kit": "^3.13.0",
19+
"@types/node": "^22.0.0",
20+
"@typescript-eslint/utils": "^8.0.0",
1821
"@vitejs/plugin-vue": "^6.0.0",
1922
"vite": "catalog:",
2023
"vue": "^3.5.0",
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"commands": [
3-
"vp migrate --no-interactive # migration should remove ESLint and its plugins/configs",
4-
"cat package.json # check eslint, eslint-plugin-*, eslint-config-*, typescript-eslint and @typescript-eslint/* are removed",
3+
"vp migrate --no-interactive # migration should remove ESLint, plugins, configs, scopes, formatters, and peer eslint",
4+
"cat package.json # verify the comprehensive ESLint ecosystem cleanup",
55
"test ! -f eslint.config.mjs # check eslint config is removed"
66
]
77
}

packages/cli/snap-tests-global/migration-eslint-type-aware/tsconfig.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,5 @@
55
"moduleResolution": "Bundler",
66
"strict": true,
77
"skipLibCheck": true
8-
},
9-
"include": ["src/**/*"]
8+
}
109
}

packages/cli/src/migration/__tests__/migrator.spec.ts

Lines changed: 138 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -384,14 +384,128 @@ describe('rewriteEslintPackageJson', () => {
384384
expect(pkg.devDependencies).toEqual({ keepme: '^1.0.0' });
385385
});
386386

387-
it('preserves unrelated dependencies (e.g. @vitejs/plugin-vue, vue, vite)', () => {
387+
it('removes @eslint/*, @eslint-community/*, and @angular-eslint/* scope packages', () => {
388+
const pkgPath = writePkg({
389+
devDependencies: {
390+
eslint: '^9.0.0',
391+
'@eslint/js': '^9.0.0',
392+
'@eslint/eslintrc': '^3.0.0',
393+
'@eslint/compat': '^1.0.0',
394+
'@eslint-community/eslint-utils': '^4.0.0',
395+
'@eslint-community/regexpp': '^4.0.0',
396+
'@angular-eslint/template-parser': '^18.0.0',
397+
'@angular-eslint/builder': '^18.0.0',
398+
keepme: '^1.0.0',
399+
},
400+
});
401+
rewriteEslintPackageJson(pkgPath);
402+
const pkg = readJson(pkgPath);
403+
expect(pkg.devDependencies).toEqual({ keepme: '^1.0.0' });
404+
});
405+
406+
it('removes ESLint formatter, helper, and runtime-integration packages', () => {
407+
const pkgPath = writePkg({
408+
devDependencies: {
409+
eslint: '^9.0.0',
410+
'eslint-formatter-pretty': '^6.0.0',
411+
'eslint-formatter-gitlab': '^5.0.0',
412+
eslintrc: '^2.0.0',
413+
'eslint-utils': '^3.0.0',
414+
'eslint-visitor-keys': '^4.0.0',
415+
'eslint-scope': '^8.0.0',
416+
'eslint-define-config': '^2.0.0',
417+
'eslint-doc-generator': '^2.0.0',
418+
'@nuxt/eslint': '^0.5.0',
419+
keepme: '^1.0.0',
420+
},
421+
});
422+
rewriteEslintPackageJson(pkgPath);
423+
const pkg = readJson(pkgPath);
424+
expect(pkg.devDependencies).toEqual({ keepme: '^1.0.0' });
425+
});
426+
427+
it('preserves reusable @typescript-eslint/* AST libraries (utils, typescript-estree, etc.)', () => {
428+
const pkgPath = writePkg({
429+
devDependencies: {
430+
eslint: '^9.0.0',
431+
'@typescript-eslint/parser': '^8.0.0',
432+
'@typescript-eslint/eslint-plugin': '^8.0.0',
433+
'@typescript-eslint/rule-tester': '^8.0.0',
434+
'@typescript-eslint/utils': '^8.0.0',
435+
'@typescript-eslint/typescript-estree': '^8.0.0',
436+
'@typescript-eslint/scope-manager': '^8.0.0',
437+
'@typescript-eslint/types': '^8.0.0',
438+
vite: '^7.0.0',
439+
},
440+
});
441+
rewriteEslintPackageJson(pkgPath);
442+
const pkg = readJson(pkgPath);
443+
expect(pkg.devDependencies).toEqual({
444+
'@typescript-eslint/utils': '^8.0.0',
445+
'@typescript-eslint/typescript-estree': '^8.0.0',
446+
'@typescript-eslint/scope-manager': '^8.0.0',
447+
'@typescript-eslint/types': '^8.0.0',
448+
vite: '^7.0.0',
449+
});
450+
});
451+
452+
it('removes @types/<X> packages symmetrically with their runtime counterparts', () => {
453+
const pkgPath = writePkg({
454+
devDependencies: {
455+
eslint: '^9.0.0',
456+
'@types/eslint': '^9.0.0',
457+
'@types/eslint-plugin-foo': '^1.0.0',
458+
'@types/eslint-config-bar': '^1.0.0',
459+
// Type-only counterpart of an ESLint plugin should also go.
460+
'@types/eslint-scope': '^3.0.0',
461+
// Unrelated @types should stay.
462+
'@types/node': '^22.0.0',
463+
},
464+
});
465+
rewriteEslintPackageJson(pkgPath);
466+
const pkg = readJson(pkgPath);
467+
expect(pkg.devDependencies).toEqual({ '@types/node': '^22.0.0' });
468+
});
469+
470+
it('scrubs peerDependencies and optionalDependencies', () => {
471+
const pkgPath = writePkg({
472+
peerDependencies: {
473+
eslint: '>=9',
474+
'eslint-plugin-vue': '^10.0.0',
475+
},
476+
optionalDependencies: {
477+
'@typescript-eslint/parser': '^8.0.0',
478+
},
479+
devDependencies: { vite: '^7.0.0' },
480+
});
481+
rewriteEslintPackageJson(pkgPath);
482+
const pkg = readJson(pkgPath);
483+
expect(pkg.peerDependencies).toBeUndefined();
484+
expect(pkg.optionalDependencies).toBeUndefined();
485+
expect(pkg.devDependencies).toEqual({ vite: '^7.0.0' });
486+
});
487+
488+
it('deletes the dependency field entirely when our cleanup emptied it', () => {
489+
const pkgPath = writePkg({
490+
devDependencies: {
491+
eslint: '^9.0.0',
492+
'eslint-plugin-import': '^2.0.0',
493+
},
494+
dependencies: { 'eslint-config-airbnb': '^19.0.0' },
495+
});
496+
rewriteEslintPackageJson(pkgPath);
497+
const pkg = readJson(pkgPath);
498+
expect(pkg.devDependencies).toBeUndefined();
499+
expect(pkg.dependencies).toBeUndefined();
500+
});
501+
502+
it('preserves unrelated dependencies (e.g. @vitejs/plugin-vue, vue, vite, @nuxt/kit)', () => {
388503
const pkgPath = writePkg({
389504
devDependencies: {
390505
eslint: '^9.0.0',
391506
'@vitejs/plugin-vue': '^6.0.0',
392507
'@vue/runtime-core': '^3.5.0',
393508
'@nuxt/kit': '^3.13.0',
394-
'@nuxt/eslint': '^0.5.0',
395509
vite: '^7.0.0',
396510
},
397511
});
@@ -401,9 +515,28 @@ describe('rewriteEslintPackageJson', () => {
401515
'@vitejs/plugin-vue': '^6.0.0',
402516
'@vue/runtime-core': '^3.5.0',
403517
'@nuxt/kit': '^3.13.0',
404-
// `@nuxt/eslint` is the runtime package, not a config — preserved.
405-
// (Only `@nuxt/eslint-config` matches the scoped-config pattern.)
406-
'@nuxt/eslint': '^0.5.0',
518+
vite: '^7.0.0',
519+
});
520+
});
521+
522+
it('workspace mode removes only `eslint` itself, not plugins or configs', () => {
523+
const pkgPath = writePkg({
524+
devDependencies: {
525+
eslint: '^9.0.0',
526+
'eslint-plugin-import': '^2.0.0',
527+
'eslint-config-airbnb': '^19.0.0',
528+
'@typescript-eslint/parser': '^8.0.0',
529+
vite: '^7.0.0',
530+
},
531+
});
532+
rewriteEslintPackageJson(pkgPath, 'workspace');
533+
const pkg = readJson(pkgPath);
534+
// Plugins/configs that the workspace package may publish as a shared
535+
// lint-preset API are preserved; only `eslint` itself goes.
536+
expect(pkg.devDependencies).toEqual({
537+
'eslint-plugin-import': '^2.0.0',
538+
'eslint-config-airbnb': '^19.0.0',
539+
'@typescript-eslint/parser': '^8.0.0',
407540
vite: '^7.0.0',
408541
});
409542
});

packages/cli/src/migration/migrator.ts

Lines changed: 80 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -303,13 +303,16 @@ export async function migrateEslintToOxlint(
303303
// Step 3: Delete all eslint config files at root
304304
deleteEslintConfigFiles(projectPath, options?.report, options?.silent);
305305

306-
// Step 4: Remove eslint dependency and rewrite eslint scripts (root only)
307-
rewriteEslintPackageJson(path.join(projectPath, 'package.json'));
306+
// Step 4: Remove eslint and all ESLint-ecosystem dependencies at the root,
307+
// and rewrite eslint scripts.
308+
rewriteEslintPackageJson(path.join(projectPath, 'package.json'), 'root');
308309

309-
// Step 4b: Rewrite eslint scripts in workspace packages
310+
// Step 4b: Workspace packages get a conservative cleanup — only `eslint`
311+
// itself is removed; plugins and configs they declare may be part of a
312+
// shared lint-preset API consumed outside Vite+, so we leave them alone.
310313
if (packages) {
311314
for (const pkg of packages) {
312-
rewriteEslintPackageJson(path.join(projectPath, pkg.path, 'package.json'));
315+
rewriteEslintPackageJson(path.join(projectPath, pkg.path, 'package.json'), 'workspace');
313316
}
314317
}
315318

@@ -337,52 +340,110 @@ function deleteEslintConfigFiles(basePath: string, report?: MigrationReport, sil
337340
}
338341
}
339342

343+
// Bare names of packages whose sole purpose is to support ESLint. Removed
344+
// at root cleanup. Reusable AST libraries published under
345+
// `@typescript-eslint/*` (`utils`, `typescript-estree`, `scope-manager`,
346+
// `types`) are deliberately absent so codemods and doc generators that
347+
// import them directly keep working after migration.
348+
const ESLINT_ECOSYSTEM_NAMES = new Set<string>([
349+
'eslint',
350+
'typescript-eslint',
351+
'eslintrc',
352+
'eslint-utils',
353+
'eslint-visitor-keys',
354+
'eslint-scope',
355+
'eslint-define-config',
356+
'eslint-doc-generator',
357+
// ESLint-only typescript-eslint entry points:
358+
'@typescript-eslint/eslint-plugin',
359+
'@typescript-eslint/parser',
360+
'@typescript-eslint/rule-tester',
361+
// Framework runtime modules that wire ESLint and break without it:
362+
'@nuxt/eslint',
363+
]);
364+
365+
// Flat name prefixes that mark an ESLint-only package.
366+
const ESLINT_ECOSYSTEM_PREFIXES = ['eslint-plugin-', 'eslint-config-', 'eslint-formatter-'];
367+
368+
// Scopes whose every package is part of the ESLint ecosystem.
369+
// @eslint/* — official ESLint scope (e.g. @eslint/js, @eslint/eslintrc)
370+
// @eslint-community/* — community-maintained ESLint dependencies
371+
// @angular-eslint/* — Angular's ESLint integration family
372+
const ESLINT_ECOSYSTEM_SCOPES = ['@eslint/', '@eslint-community/', '@angular-eslint/'];
373+
340374
/**
341375
* Decide whether a dependency entry should be removed alongside `eslint`
342-
* itself. The set is conservative: only packages whose sole purpose is to
343-
* configure or extend ESLint. Anything else is preserved.
376+
* itself. The set is intentionally broad: anything whose only purpose is
377+
* to extend, configure, format, or wire ESLint becomes dead weight after
378+
* migration. `@types/<X>` packages are checked symmetrically with `<X>`
379+
* so type-only counterparts of removed runtime packages also go.
344380
*/
345381
function isEslintEcosystemDep(name: string): boolean {
346-
if (name === 'eslint') {
347-
return true;
348-
}
349-
if (name.startsWith('eslint-plugin-') || name.startsWith('eslint-config-')) {
382+
const stripped = name.startsWith('@types/') ? name.slice('@types/'.length) : name;
383+
if (ESLINT_ECOSYSTEM_NAMES.has(stripped)) {
350384
return true;
351385
}
352-
if (name === 'typescript-eslint') {
386+
if (ESLINT_ECOSYSTEM_PREFIXES.some((p) => stripped.startsWith(p))) {
353387
return true;
354388
}
355-
if (name.startsWith('@typescript-eslint/')) {
389+
if (ESLINT_ECOSYSTEM_SCOPES.some((s) => stripped.startsWith(s))) {
356390
return true;
357391
}
358-
// Scoped plugins/configs, e.g. `@vue/eslint-config-typescript`,
359-
// `@nuxt/eslint-config`, `@stylistic/eslint-plugin`. These are all
360-
// ESLint-only and have no other consumers.
361-
if (/^@[^/]+\/eslint-(plugin|config)(-.+)?$/.test(name)) {
392+
// Scoped plugins/configs/formatters, e.g.:
393+
// @vue/eslint-config-typescript
394+
// @stylistic/eslint-plugin-ts
395+
// @vitest/eslint-plugin
396+
if (/^@[^/]+\/eslint-(plugin|config|formatter)(-.+)?$/.test(stripped)) {
362397
return true;
363398
}
364399
return false;
365400
}
366401

367-
export function rewriteEslintPackageJson(packageJsonPath: string): void {
402+
/**
403+
* Rewrite a project's `package.json` after ESLint has been migrated to
404+
* Oxlint. Root cleanup is aggressive — every ESLint-ecosystem dependency
405+
* is removed (see `isEslintEcosystemDep`). Workspace cleanup is
406+
* conservative — only `eslint` itself is removed, because workspace
407+
* sub-packages may intentionally publish ESLint plugins / configs as
408+
* part of a shared lint-preset API consumed outside Vite+.
409+
*/
410+
export function rewriteEslintPackageJson(
411+
packageJsonPath: string,
412+
mode: 'root' | 'workspace' = 'root',
413+
): void {
368414
editJsonFile<{
369415
devDependencies?: Record<string, string>;
370416
dependencies?: Record<string, string>;
417+
peerDependencies?: Record<string, string>;
418+
optionalDependencies?: Record<string, string>;
371419
scripts?: Record<string, string>;
372420
'lint-staged'?: Record<string, string | string[]>;
373421
}>(packageJsonPath, (pkg) => {
374422
let changed = false;
375-
for (const field of ['devDependencies', 'dependencies'] as const) {
423+
const isRemovable = mode === 'root' ? isEslintEcosystemDep : (n: string) => n === 'eslint';
424+
for (const field of [
425+
'devDependencies',
426+
'dependencies',
427+
'peerDependencies',
428+
'optionalDependencies',
429+
] as const) {
376430
const deps = pkg[field];
377431
if (!deps) {
378432
continue;
379433
}
434+
let removedAny = false;
380435
for (const name of Object.keys(deps)) {
381-
if (isEslintEcosystemDep(name)) {
436+
if (isRemovable(name)) {
382437
delete deps[name];
383438
changed = true;
439+
removedAny = true;
384440
}
385441
}
442+
// Drop the field entirely if our cleanup emptied it — avoid
443+
// leaving `"devDependencies": {}` noise in the output.
444+
if (removedAny && Object.keys(deps).length === 0) {
445+
delete pkg[field];
446+
}
386447
}
387448
if (pkg.scripts) {
388449
const updated = rewriteEslint(JSON.stringify(pkg.scripts));

0 commit comments

Comments
 (0)