Skip to content

Commit dcef017

Browse files
fengmk2fengmk2
committed
fix(migrate): isolate config compatibility checks
1 parent 1808d55 commit dcef017

10 files changed

Lines changed: 220 additions & 18 deletions

File tree

‎packages/cli/snap-tests/migration-config-process-crash-isolated/package.json‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"name": "migration-config-process-crash-isolated",
3+
"version": "0.0.0",
4+
"private": true,
5+
"devDependencies": {
6+
"vite": "^8.0.0"
7+
}
8+
}

‎packages/cli/snap-tests/migration-config-process-crash-isolated/snap.txt‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
> vp migrate --no-interactive --no-hooks 2>&1 # project config process handlers must not terminate migration
2+
◇ Migrated . to Vite+
3+
• Node <semver> pnpm <semver>
4+
• 1 file had imports rewritten
5+
6+
> cat vite.config.ts # migration still rewrites the config after its compatibility probe crashes
7+
import { defineConfig } from 'vite-plus';
8+
9+
// Models a project plugin that installs a process-level error backstop while
10+
// its config is loaded. Re-throwing from this handler makes Node exit with code
11+
// 7, which used to terminate `vp migrate` during its best-effort compatibility
12+
// check instead of allowing migration to continue.
13+
process.on('uncaughtException', (error) => {
14+
throw error;
15+
});
16+
queueMicrotask(() => {
17+
throw new Error('simulated project config crash');
18+
});
19+
20+
export default defineConfig({
21+
fmt: {},
22+
lint: {"jsPlugins":[{"name":"vite-plus","specifier":"vite-plus/oxlint-plugin"}],"rules":{"vite-plus/prefer-vite-plus-imports":"error"},"options":{"typeAware":true,"typeCheck":true}},
23+
});

‎packages/cli/snap-tests/migration-config-process-crash-isolated/steps.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"commands": [
3+
"vp migrate --no-interactive --no-hooks 2>&1 # project config process handlers must not terminate migration",
4+
"cat vite.config.ts # migration still rewrites the config after its compatibility probe crashes"
5+
]
6+
}

‎packages/cli/snap-tests/migration-config-process-crash-isolated/vite.config.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import { defineConfig } from 'vite';
2+
3+
// Models a project plugin that installs a process-level error backstop while
4+
// its config is loaded. Re-throwing from this handler makes Node exit with code
5+
// 7, which used to terminate `vp migrate` during its best-effort compatibility
6+
// check instead of allowing migration to continue.
7+
process.on('uncaughtException', (error) => {
8+
throw error;
9+
});
10+
queueMicrotask(() => {
11+
throw new Error('simulated project config crash');
12+
});
13+
14+
export default defineConfig({});

‎packages/cli/src/migration/__tests__/compat-runner.spec.ts‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import { beforeEach, describe, expect, it, vi } from 'vitest';
2+
3+
vi.mock('../../utils/command.ts', () => ({
4+
runCommandSilently: vi.fn(),
5+
}));
6+
7+
import { runCommandSilently } from '../../utils/command.ts';
8+
import { checkRolldownCompatibility, ROLLDOWN_COMPAT_RESULT_PREFIX } from '../compat-runner.ts';
9+
import { createMigrationReport } from '../report.ts';
10+
11+
const mockRunCommandSilently = vi.mocked(runCommandSilently);
12+
13+
describe('checkRolldownCompatibility', () => {
14+
beforeEach(() => {
15+
mockRunCommandSilently.mockReset();
16+
});
17+
18+
it('merges warnings returned by the isolated config worker', async () => {
19+
mockRunCommandSilently.mockResolvedValue({
20+
exitCode: 0,
21+
stdout: Buffer.from(
22+
`project config output\n${ROLLDOWN_COMPAT_RESULT_PREFIX}${JSON.stringify({ warnings: ['manualChunks warning'] })}\n`,
23+
),
24+
stderr: Buffer.alloc(0),
25+
});
26+
const report = createMigrationReport();
27+
28+
await checkRolldownCompatibility('/project', report);
29+
30+
expect(report.warnings).toEqual(['manualChunks warning']);
31+
expect(mockRunCommandSilently).toHaveBeenCalledWith({
32+
command: process.execPath,
33+
args: [expect.stringMatching(/compat-worker\.js$/), '/project'],
34+
cwd: '/project',
35+
envs: process.env,
36+
});
37+
});
38+
39+
it('skips compatibility checking when project config crashes the worker', async () => {
40+
mockRunCommandSilently.mockResolvedValue({
41+
exitCode: 7,
42+
stdout: Buffer.from(
43+
`${ROLLDOWN_COMPAT_RESULT_PREFIX}${JSON.stringify({ warnings: ['incomplete result'] })}\n`,
44+
),
45+
stderr: Buffer.from('project config crashed'),
46+
});
47+
const report = createMigrationReport();
48+
49+
await expect(checkRolldownCompatibility('/project', report)).resolves.toBeUndefined();
50+
expect(report.warnings).toEqual([]);
51+
});
52+
53+
it('skips compatibility checking when the worker cannot start', async () => {
54+
mockRunCommandSilently.mockRejectedValue(new Error('spawn failed'));
55+
const report = createMigrationReport();
56+
57+
await expect(checkRolldownCompatibility('/project', report)).resolves.toBeUndefined();
58+
expect(report.warnings).toEqual([]);
59+
});
60+
});

‎packages/cli/src/migration/bin.ts‎

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import {
4545
} from '../utils/tsconfig.ts';
4646
import type { PackageDependencies } from '../utils/types.ts';
4747
import { detectWorkspace } from '../utils/workspace.ts';
48+
import { checkRolldownCompatibility } from './compat-runner.ts';
4849
import {
4950
addFrameworkShim,
5051
checkVitestVersion,
@@ -885,24 +886,6 @@ function showMigrationSummary(options: {
885886
}
886887
}
887888

888-
async function checkRolldownCompatibility(rootDir: string, report: MigrationReport): Promise<void> {
889-
try {
890-
const { resolveConfig } = await import('../index.js');
891-
const { withConfigMetadataResolution } = await import('../define-config.js');
892-
const { checkManualChunksCompat } = await import('./compat.js');
893-
// Use 'runner' configLoader to avoid Rolldown bundling the config file,
894-
// which prints UNRESOLVED_IMPORT warnings that cannot be suppressed via logLevel.
895-
// Reads the config only for the manualChunks compat check, so skip the
896-
// user's plugin factory while it resolves.
897-
const config = await withConfigMetadataResolution(() =>
898-
resolveConfig({ root: rootDir, logLevel: 'silent', configLoader: 'runner' }, 'build'),
899-
);
900-
checkManualChunksCompat(config.build?.rollupOptions?.output, report);
901-
} catch {
902-
// Config resolution may fail — skip compatibility check silently
903-
}
904-
}
905-
906889
async function downloadSupportedPackageManager(options: {
907890
rootDir: string;
908891
packageManager: PackageManager;

‎packages/cli/src/migration/compat-protocol.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export const ROLLDOWN_COMPAT_RESULT_PREFIX = 'VITE_PLUS_ROLLDOWN_COMPAT_RESULT=';

‎packages/cli/src/migration/compat-runner.ts‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
import { fileURLToPath } from 'node:url';
2+
3+
import { runCommandSilently } from '../utils/command.ts';
4+
import { ROLLDOWN_COMPAT_RESULT_PREFIX } from './compat-protocol.ts';
5+
import { addMigrationWarning, type MigrationReport } from './report.ts';
6+
7+
export { ROLLDOWN_COMPAT_RESULT_PREFIX };
8+
9+
interface RolldownCompatibilityResult {
10+
warnings: string[];
11+
}
12+
13+
function parseRolldownCompatibilityResult(stdout: Buffer): RolldownCompatibilityResult | undefined {
14+
const output = stdout.toString();
15+
const markerIndex = output.lastIndexOf(ROLLDOWN_COMPAT_RESULT_PREFIX);
16+
if (markerIndex === -1) {
17+
return undefined;
18+
}
19+
20+
const resultStart = markerIndex + ROLLDOWN_COMPAT_RESULT_PREFIX.length;
21+
const resultEnd = output.indexOf('\n', resultStart);
22+
const serialized = output.slice(resultStart, resultEnd === -1 ? undefined : resultEnd).trim();
23+
24+
try {
25+
const result = JSON.parse(serialized) as Partial<RolldownCompatibilityResult>;
26+
if (
27+
!Array.isArray(result.warnings) ||
28+
!result.warnings.every((item) => typeof item === 'string')
29+
) {
30+
return undefined;
31+
}
32+
return { warnings: result.warnings };
33+
} catch {
34+
return undefined;
35+
}
36+
}
37+
38+
/**
39+
* Resolve a project's Vite config in a child process before checking it for
40+
* Rolldown-incompatible options. Config files execute arbitrary project code;
41+
* isolating them prevents process-level handlers, explicit exits, and
42+
* asynchronous crashes from terminating the migration itself.
43+
*/
44+
export async function checkRolldownCompatibility(
45+
rootDir: string,
46+
report: MigrationReport,
47+
): Promise<void> {
48+
try {
49+
const workerPath = fileURLToPath(new URL('./compat-worker.js', import.meta.url));
50+
const result = await runCommandSilently({
51+
command: process.execPath,
52+
args: [workerPath, rootDir],
53+
cwd: rootDir,
54+
envs: process.env,
55+
});
56+
57+
if (result.exitCode !== 0) {
58+
return;
59+
}
60+
61+
const compatibilityResult = parseRolldownCompatibilityResult(result.stdout);
62+
for (const warning of compatibilityResult?.warnings ?? []) {
63+
addMigrationWarning(report, warning);
64+
}
65+
} catch {
66+
// Config resolution is best-effort. Skip failures silently.
67+
}
68+
}

‎packages/cli/src/migration/compat-worker.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import { writeSync } from 'node:fs';
2+
3+
import { ROLLDOWN_COMPAT_RESULT_PREFIX } from './compat-protocol.ts';
4+
import { checkManualChunksCompat } from './compat.ts';
5+
import { createMigrationReport } from './report.ts';
6+
7+
async function main(): Promise<void> {
8+
const rootDir = process.argv[2];
9+
if (!rootDir) {
10+
return;
11+
}
12+
13+
try {
14+
const { resolveConfig } = await import('../index.js');
15+
// Use 'runner' configLoader to avoid Rolldown bundling the config file,
16+
// which prints UNRESOLVED_IMPORT warnings that cannot be suppressed via logLevel.
17+
const config = await resolveConfig(
18+
{ root: rootDir, logLevel: 'silent', configLoader: 'runner' },
19+
'build',
20+
);
21+
const report = createMigrationReport();
22+
checkManualChunksCompat(config.build?.rollupOptions?.output, report);
23+
writeSync(
24+
process.stdout.fd,
25+
`${ROLLDOWN_COMPAT_RESULT_PREFIX}${JSON.stringify({ warnings: report.warnings })}\n`,
26+
);
27+
} catch {
28+
// Config resolution may fail — skip compatibility checking silently.
29+
}
30+
}
31+
32+
// Config plugins may leave active handles behind. Once the result has been
33+
// written synchronously, terminate this disposable worker without waiting for
34+
// project-owned cleanup.
35+
main().then(
36+
() => process.exit(0),
37+
() => process.exit(0),
38+
);

‎packages/cli/tsdown.config.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ export default defineConfig([
5757
// Without these, tsdown inlines them into bin.js, breaking on-demand loading.
5858
'create/bin': './src/create/bin.ts',
5959
'migration/bin': './src/migration/bin.ts',
60+
'migration/compat-worker': './src/migration/compat-worker.ts',
6061
version: './src/version.ts',
6162
'config/bin': './src/config/bin.ts',
6263
'staged/bin': './src/staged/bin.ts',

0 commit comments

Comments
 (0)