Skip to content

Commit 1a47e25

Browse files
fengmk2fengmk2
committed
fix(migrate): resolve installed peers before plugin cleanup
1 parent 17649e0 commit 1a47e25

5 files changed

Lines changed: 249 additions & 34 deletions

File tree

‎crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/migration_oxlint_existing_vite_plus/test-fixture/package.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"name": "uninstalled-test-fixture",
3+
"private": true,
4+
"devDependencies": {
5+
"uninstalled-plugin": "1.0.0"
6+
}
7+
}

‎packages/cli/src/migration/__tests__/oxlint-plugin-dependency.spec.ts‎

Lines changed: 162 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,15 @@ import path from 'node:path';
44

55
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
66

7-
import { PackageManager } from '../../types/index.ts';
7+
import { PackageManager, type WorkspaceInfo } from '../../types/index.ts';
88
import {
99
collectOxlintOwnerDirs,
1010
dropDeadOxlintPluginsDependency,
1111
finalizeCoreMigrationForExistingVitePlus,
1212
packageOwnsOxlintApi,
1313
rewritePackageJson,
14+
rewriteMonorepo,
15+
rewriteStandaloneProject,
1416
sourceTreeReferencesOxlintPluginsPackage,
1517
usesVitestBrowserMode,
1618
} from '../migrator.ts';
@@ -64,6 +66,70 @@ describe('Oxlint plugin dependency cleanup', () => {
6466
},
6567
);
6668

69+
it.each([false, true])(
70+
'cleans up before newly injected browser packages are installed (monorepo: %s)',
71+
(isMonorepo) => {
72+
const packageJsonPath = path.join(projectPath, 'package.json');
73+
fs.writeFileSync(
74+
packageJsonPath,
75+
JSON.stringify({
76+
name: 'project',
77+
devDependencies: { 'vite-plus': 'latest', '@oxlint/plugins': '^1.79.0' },
78+
}),
79+
);
80+
const browserProjectPath = isMonorepo
81+
? path.join(projectPath, 'packages', 'app')
82+
: projectPath;
83+
if (isMonorepo) {
84+
fs.mkdirSync(browserProjectPath, { recursive: true });
85+
fs.writeFileSync(path.join(browserProjectPath, 'package.json'), '{"name":"app"}');
86+
fs.writeFileSync(
87+
path.join(projectPath, 'pnpm-workspace.yaml'),
88+
'packages:\n - packages/*\n',
89+
);
90+
}
91+
fs.writeFileSync(
92+
path.join(browserProjectPath, 'browser.ts'),
93+
"import { playwright } from '@vitest/browser-playwright';",
94+
);
95+
fs.writeFileSync(
96+
path.join(projectPath, 'plugin.ts'),
97+
"import { defineRule } from '@oxlint/plugins';",
98+
);
99+
const workspace: WorkspaceInfo = {
100+
rootDir: projectPath,
101+
isMonorepo,
102+
monorepoScope: '',
103+
workspacePatterns: isMonorepo ? ['packages/*'] : [],
104+
parentDirs: [],
105+
packages: isMonorepo ? [{ name: 'app', path: 'packages/app' }] : [],
106+
packageManager: PackageManager.pnpm,
107+
packageManagerVersion: '10.33.0',
108+
downloadPackageManager: {
109+
name: PackageManager.pnpm,
110+
packageName: 'pnpm',
111+
version: '10.33.0',
112+
installDir: projectPath,
113+
binPrefix: projectPath,
114+
},
115+
};
116+
117+
if (isMonorepo) {
118+
rewriteMonorepo(workspace, true, true);
119+
} else {
120+
rewriteStandaloneProject(projectPath, workspace, true, true);
121+
}
122+
123+
expect(
124+
JSON.parse(fs.readFileSync(packageJsonPath, 'utf8')).devDependencies,
125+
).not.toHaveProperty('@oxlint/plugins');
126+
expect(
127+
JSON.parse(fs.readFileSync(path.join(browserProjectPath, 'package.json'), 'utf8'))
128+
.devDependencies,
129+
).toHaveProperty('@vitest/browser-playwright');
130+
},
131+
);
132+
67133
it.each([
68134
{ scripts: { 'check-plugin': `node -e "require('@oxlint/plugins')"` } },
69135
{ imports: { '#plugin-api': '@oxlint/plugins' } },
@@ -186,34 +252,103 @@ describe('Oxlint plugin dependency cleanup', () => {
186252
},
187253
);
188254

189-
it('retains a root peer provider for a plugin used by a nested package', () => {
190-
const pkg = { devDependencies: { 'vite-plus': 'latest', '@oxlint/plugins': '^1.79.0' } };
191-
const packageJsonPath = path.join(projectPath, 'package.json');
192-
fs.writeFileSync(packageJsonPath, JSON.stringify(pkg));
193-
const appPath = path.join(projectPath, 'packages', 'app');
194-
fs.mkdirSync(appPath, { recursive: true });
195-
fs.writeFileSync(
196-
path.join(appPath, 'package.json'),
197-
JSON.stringify({ dependencies: { 'review-oxlint-plugin': '1.0.0' } }),
198-
);
199-
const pluginPath = path.join(appPath, 'node_modules', 'review-oxlint-plugin');
200-
fs.mkdirSync(pluginPath, { recursive: true });
201-
fs.writeFileSync(
202-
path.join(pluginPath, 'package.json'),
203-
JSON.stringify({
204-
name: 'review-oxlint-plugin',
205-
peerDependencies: { '@oxlint/plugins': '^1.79.0' },
206-
}),
207-
);
255+
it.each([false, true])(
256+
'retains a root peer provider for a nested plugin (workspace: %s)',
257+
(isWorkspacePackage) => {
258+
const pkg = { devDependencies: { 'vite-plus': 'latest', '@oxlint/plugins': '^1.79.0' } };
259+
const packageJsonPath = path.join(projectPath, 'package.json');
260+
fs.writeFileSync(packageJsonPath, JSON.stringify(pkg));
261+
const appPath = path.join(projectPath, 'packages', 'app');
262+
fs.mkdirSync(appPath, { recursive: true });
263+
fs.writeFileSync(
264+
path.join(appPath, 'package.json'),
265+
JSON.stringify({ dependencies: { 'review-oxlint-plugin': '1.0.0' } }),
266+
);
267+
const pluginPath = path.join(appPath, 'node_modules', 'review-oxlint-plugin');
268+
fs.mkdirSync(pluginPath, { recursive: true });
269+
fs.writeFileSync(
270+
path.join(pluginPath, 'package.json'),
271+
JSON.stringify({
272+
name: 'review-oxlint-plugin',
273+
peerDependencies: { '@oxlint/plugins': '^1.79.0' },
274+
}),
275+
);
208276

209-
const result = finalizeCoreMigrationForExistingVitePlus(
210-
{ rootDir: projectPath, packages: [{ name: 'app', path: 'packages/app' }] },
211-
true,
212-
);
277+
const result = finalizeCoreMigrationForExistingVitePlus(
278+
{
279+
rootDir: projectPath,
280+
packages: isWorkspacePackage ? [{ name: 'app', path: 'packages/app' }] : undefined,
281+
},
282+
true,
283+
);
213284

214-
expect(result.dependencies).toBe(false);
215-
expect(JSON.parse(fs.readFileSync(packageJsonPath, 'utf8'))).toEqual(pkg);
216-
});
285+
expect(result.dependencies).toBe(false);
286+
expect(JSON.parse(fs.readFileSync(packageJsonPath, 'utf8'))).toEqual(pkg);
287+
},
288+
);
289+
290+
it.each([{ '.': { import: './index.js' } }, { '.': './dist/index.js' }])(
291+
'reads installed peer metadata despite inaccessible exports %j',
292+
(exports) => {
293+
const packageJsonPath = path.join(projectPath, 'package.json');
294+
fs.writeFileSync(
295+
packageJsonPath,
296+
JSON.stringify({
297+
devDependencies: {
298+
'vite-plus': 'latest',
299+
'@oxlint/plugins': '^1.79.0',
300+
'review-oxlint-plugin': '1.0.0',
301+
},
302+
}),
303+
);
304+
const pluginPath = path.join(projectPath, 'node_modules', 'review-oxlint-plugin');
305+
fs.mkdirSync(pluginPath, { recursive: true });
306+
fs.writeFileSync(
307+
path.join(pluginPath, 'package.json'),
308+
JSON.stringify({ name: 'review-oxlint-plugin', version: '1.0.0', exports }),
309+
);
310+
fs.writeFileSync(path.join(pluginPath, 'index.js'), 'export default {};');
311+
312+
const result = finalizeCoreMigrationForExistingVitePlus({ rootDir: projectPath }, true);
313+
314+
expect(result.dependencies).toBe(true);
315+
expect(
316+
JSON.parse(fs.readFileSync(packageJsonPath, 'utf8')).devDependencies['@oxlint/plugins'],
317+
).toBeUndefined();
318+
},
319+
);
320+
321+
it.each([false, true])(
322+
'only retains unknown nested peer contracts for workspace packages (workspace: %s)',
323+
(isWorkspacePackage) => {
324+
const packageJsonPath = path.join(projectPath, 'package.json');
325+
fs.writeFileSync(
326+
packageJsonPath,
327+
JSON.stringify({
328+
devDependencies: { 'vite-plus': 'latest', '@oxlint/plugins': '^1.79.0' },
329+
}),
330+
);
331+
const nestedPath = path.join(projectPath, 'nested');
332+
fs.mkdirSync(nestedPath);
333+
fs.writeFileSync(
334+
path.join(nestedPath, 'package.json'),
335+
JSON.stringify({ name: 'nested', devDependencies: { 'uninstalled-plugin': '1.0.0' } }),
336+
);
337+
338+
const result = finalizeCoreMigrationForExistingVitePlus(
339+
{
340+
rootDir: projectPath,
341+
packages: isWorkspacePackage ? [{ name: 'nested', path: 'nested' }] : undefined,
342+
},
343+
true,
344+
);
345+
346+
expect(result.dependencies).toBe(!isWorkspacePackage);
347+
expect(
348+
JSON.parse(fs.readFileSync(packageJsonPath, 'utf8')).devDependencies['@oxlint/plugins'],
349+
).toBe(isWorkspacePackage ? '^1.79.0' : undefined);
350+
},
351+
);
217352

218353
it.each(['#!/usr/bin/env node\n', ''])(
219354
'retains an extensionless Node script with prefix %j',

‎packages/cli/src/migration/migrator/orchestrators.ts‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
cleanupDeprecatedTsconfigOptions,
1111
collectInjectedProviderNames,
1212
collectOxlintOwnerDirs,
13+
collectOxlintPeerDependencyNames,
1314
dropDeadOxlintPluginsDependency,
1415
collectProviderSourceModes,
1516
collectVitestEcosystemInstallDependencyNames,
@@ -81,6 +82,10 @@ export function rewriteStandaloneProject(
8182
// Captured before `rewritePackageJson` strips `oxlint`; the import rewriter
8283
// reads the manifests afterwards and would no longer see the signal.
8384
const oxlintOwnerDirs = collectOxlintOwnerDirs(projectPath, workspaceInfo.packages);
85+
const oxlintPeerDependencyNames = collectOxlintPeerDependencyNames(
86+
projectPath,
87+
workspaceInfo.packages,
88+
);
8489
// Source-tree scan signals are computed once here and reused below (and inside
8590
// projectUsesVitestDirectly / collectInjectedProviderNames) so the source tree
8691
// is traversed once each instead of repeatedly. They do not depend on
@@ -339,7 +344,7 @@ export function rewriteStandaloneProject(
339344
mergeTsdownConfigFile(projectPath, silent, report);
340345
// rewrite imports in all TypeScript/JavaScript files before lazy plugin import merging
341346
rewriteAllImports(projectPath, silent, report, true, oxlintOwnerDirs);
342-
dropDeadOxlintPluginsDependency(projectPath, workspaceInfo.packages);
347+
dropDeadOxlintPluginsDependency(projectPath, workspaceInfo.packages, oxlintPeerDependencyNames);
343348
wrapLazyPluginsInViteConfig(projectPath, silent, report);
344349
// set package manager
345350
setPackageManager(projectPath, workspaceInfo.downloadPackageManager);
@@ -362,6 +367,10 @@ export function rewriteMonorepo(
362367
// Captured before `rewritePackageJson` strips `oxlint`; the import rewriter
363368
// reads the manifests afterwards and would no longer see the signal.
364369
const oxlintOwnerDirs = collectOxlintOwnerDirs(workspaceInfo.rootDir, workspaceInfo.packages);
370+
const oxlintPeerDependencyNames = collectOxlintPeerDependencyNames(
371+
workspaceInfo.rootDir,
372+
workspaceInfo.packages,
373+
);
365374
const pnpmMajorVersion = pnpmMajor(workspaceInfo.downloadPackageManager.version);
366375
const usePnpmWorkspaceSettings = pnpmSupportsWorkspaceSettings(
367376
workspaceInfo.downloadPackageManager.version,
@@ -473,7 +482,11 @@ export function rewriteMonorepo(
473482
mergeTsdownConfigFile(workspaceInfo.rootDir, silent, report);
474483
// rewrite imports in all TypeScript/JavaScript files before lazy plugin import merging
475484
rewriteAllImports(workspaceInfo.rootDir, silent, report, true, oxlintOwnerDirs);
476-
dropDeadOxlintPluginsDependency(workspaceInfo.rootDir, workspaceInfo.packages);
485+
dropDeadOxlintPluginsDependency(
486+
workspaceInfo.rootDir,
487+
workspaceInfo.packages,
488+
oxlintPeerDependencyNames,
489+
);
477490
wrapLazyPluginsInViteConfig(workspaceInfo.rootDir, silent, report);
478491
for (const pkg of workspaceInfo.packages) {
479492
wrapLazyPluginsInViteConfig(path.join(workspaceInfo.rootDir, pkg.path), silent, report);

‎packages/cli/src/migration/migrator/source-scan.ts‎

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -383,13 +383,26 @@ export function collectProviderSourceModes(projectPath: string): Record<string,
383383
* A substring scan conservatively retains references the rewriter leaves alone,
384384
* including require calls, type references, and strings.
385385
*/
386-
export function sourceTreeReferencesOxlintPluginsPackage(projectPath: string): boolean {
386+
export function sourceTreeReferencesOxlintPluginsPackage(
387+
projectPath: string,
388+
peerDependencyNames: ReadonlyMap<string, ReadonlySet<string>> = collectOxlintPeerDependencyNames(
389+
projectPath,
390+
),
391+
): boolean {
387392
return sourceTreeMatches(projectPath, (content) => content.includes(OXLINT_PLUGINS_PACKAGE), {
388393
crossPackageBoundaries: true,
389394
includePackageReferences: true,
390395
// Node can execute these scripts without a shebang or executable bit.
391396
includeExtensionless: true,
392-
matchesPackage: projectListsRequiredOxlintPluginsPeer,
397+
// Nested fixtures and templates can declare dependencies that this workspace
398+
// never installs. Their source still counts, but unknown peers do not.
399+
matchesPackage: (dir, pkg) =>
400+
projectListsRequiredOxlintPluginsPeer(
401+
dir,
402+
pkg,
403+
peerDependencyNames.has(dir),
404+
peerDependencyNames.get(dir),
405+
),
393406
skipDirs: OXLINT_RETENTION_SKIP_DIRS,
394407
});
395408
}
@@ -399,6 +412,8 @@ export function sourceTreeReferencesOxlintPluginsPackage(projectPath: string): b
399412
export function projectListsRequiredOxlintPluginsPeer(
400413
projectPath: string,
401414
pkg: DependencyBag,
415+
retainUnknownPeers = true,
416+
originalDependencyNames?: ReadonlySet<string>,
402417
): boolean {
403418
const dependencyNames = new Set([
404419
...Object.keys(pkg.dependencies ?? {}),
@@ -410,9 +425,15 @@ export function projectListsRequiredOxlintPluginsPeer(
410425
dependencyNames.delete(VITE_PLUS_NAME);
411426
dependencyNames.delete('vite');
412427
for (const name of dependencyNames) {
428+
if (originalDependencyNames && !originalDependencyNames.has(name)) {
429+
continue;
430+
}
413431
const metadata = detectPackageMetadata(projectPath, name);
414432
if (!metadata) {
415-
return true;
433+
if (retainUnknownPeers) {
434+
return true;
435+
}
436+
continue;
416437
}
417438
try {
418439
const installedPkg = readJsonFile(path.join(metadata.path, 'package.json')) as {
@@ -427,12 +448,36 @@ export function projectListsRequiredOxlintPluginsPeer(
427448
}
428449
} catch {
429450
// An unknown peer contract is not evidence that the provider is unused.
430-
return true;
451+
if (retainUnknownPeers) {
452+
return true;
453+
}
431454
}
432455
}
433456
return false;
434457
}
435458

459+
// Capture the original dependency names before migration injects new toolchain
460+
// packages. Check peers only for original dependencies that survive migration;
461+
// newly injected packages are not installed until after source cleanup.
462+
export function collectOxlintPeerDependencyNames(
463+
rootDir: string,
464+
packages?: readonly { path: string }[],
465+
): Map<string, ReadonlySet<string>> {
466+
const names = new Map<string, ReadonlySet<string>>();
467+
for (const dir of [rootDir, ...(packages ?? []).map((pkg) => path.join(rootDir, pkg.path))]) {
468+
const pkg = readPackageJsonIfExists(path.join(dir, 'package.json'));
469+
names.set(
470+
dir,
471+
new Set([
472+
...Object.keys(pkg?.dependencies ?? {}),
473+
...Object.keys(pkg?.devDependencies ?? {}),
474+
...Object.keys(pkg?.optionalDependencies ?? {}),
475+
]),
476+
);
477+
}
478+
return names;
479+
}
480+
436481
/**
437482
* Drop `@oxlint/plugins` from devDependencies once nothing names it any more.
438483
*
@@ -443,6 +488,10 @@ export function projectListsRequiredOxlintPluginsPeer(
443488
export function dropDeadOxlintPluginsDependency(
444489
rootDir: string,
445490
packages?: readonly { path: string }[],
491+
peerDependencyNames: ReadonlyMap<string, ReadonlySet<string>> = collectOxlintPeerDependencyNames(
492+
rootDir,
493+
packages,
494+
),
446495
): boolean {
447496
let changed = false;
448497
const dirs = [rootDir, ...(packages ?? []).map((pkg) => path.join(rootDir, pkg.path))];
@@ -452,7 +501,10 @@ export function dropDeadOxlintPluginsDependency(
452501
if (pkg?.devDependencies?.[OXLINT_PLUGINS_PACKAGE] === undefined) {
453502
continue;
454503
}
455-
if (packageOwnsOxlintApi(pkg) || sourceTreeReferencesOxlintPluginsPackage(dir)) {
504+
if (
505+
packageOwnsOxlintApi(pkg) ||
506+
sourceTreeReferencesOxlintPluginsPackage(dir, peerDependencyNames)
507+
) {
456508
continue;
457509
}
458510
editJsonFile<{

0 commit comments

Comments
 (0)