Skip to content

Commit 0746a5f

Browse files
committed
chore(studio): address cleanup review follow-ups
Three targeted fixes from the self-review pass on 7ceead9: - Collapse the renderer dev port to a single source of truth. scripts/renderer-dev-config.mjs now owns host/port/URL and is imported by rsbuild.config.ts, wait-for-electron-build.mjs and a new launch-electron-dev.mjs launcher that injects the env var for electron. The dev script in package.json no longer hardcodes the URL, so the port lives in exactly one place. - Wrap the non-ENOENT throw in readMtimeMs with a contextual Error (file path + original message via cause), so unexpected IO failures during the dev gate point directly at the offending file. - Add a regression test that createFreshBuildChecker propagates a non-ENOENT reader error instead of treating it as a missing file, locking in the narrowed catch behavior. Validated with pnpm --filter studio build, pnpm --filter studio test, and pnpm run lint.
1 parent 7ceead9 commit 0746a5f

File tree

6 files changed

+75
-8
lines changed

6 files changed

+75
-8
lines changed

apps/studio/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
"type": "module",
66
"scripts": {
77
"build": "rsbuild build && node scripts/sync-static-assets.mjs",
8-
"dev": "concurrently -k -n build,app \"rsbuild dev\" \"node scripts/wait-for-electron-build.mjs && MIDSCENE_STUDIO_RENDERER_URL=http://127.0.0.1:3210 electron dist/main/main.cjs\"",
8+
"dev": "concurrently -k -n build,app \"rsbuild dev\" \"node scripts/wait-for-electron-build.mjs && node scripts/launch-electron-dev.mjs\"",
99
"preview": "rsbuild preview --environment renderer",
1010
"start": "node scripts/sync-static-assets.mjs && electron dist/main/main.cjs",
1111
"test": "vitest run"

apps/studio/rsbuild.config.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,14 @@ import { pluginLess } from '@rsbuild/plugin-less';
33
import { pluginReact } from '@rsbuild/plugin-react';
44
import { pluginTypeCheck } from '@rsbuild/plugin-type-check';
55
import { version as appVersion } from './package.json';
6-
7-
// Must match the port used by MIDSCENE_STUDIO_RENDERER_URL in the `dev`
8-
// script of package.json.
9-
const rendererDevPort = 3210;
6+
import {
7+
rendererDevHost,
8+
rendererDevPort,
9+
} from './scripts/renderer-dev-config.mjs';
1010

1111
export default defineConfig({
1212
server: {
13-
host: '127.0.0.1',
13+
host: rendererDevHost,
1414
port: rendererDevPort,
1515
},
1616
dev: {
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { spawn } from 'node:child_process';
2+
import { createRequire } from 'node:module';
3+
import path from 'node:path';
4+
import { fileURLToPath } from 'node:url';
5+
import { rendererDevUrl } from './renderer-dev-config.mjs';
6+
7+
// Spawns Electron with MIDSCENE_STUDIO_RENDERER_URL sourced from the shared
8+
// dev config, so the port is not duplicated in package.json scripts.
9+
const require = createRequire(import.meta.url);
10+
const electronBinary = require('electron');
11+
const rootDir = path.resolve(
12+
path.dirname(fileURLToPath(import.meta.url)),
13+
'..',
14+
);
15+
16+
const child = spawn(
17+
electronBinary,
18+
[path.join(rootDir, 'dist/main/main.cjs')],
19+
{
20+
env: { ...process.env, MIDSCENE_STUDIO_RENDERER_URL: rendererDevUrl },
21+
stdio: 'inherit',
22+
},
23+
);
24+
25+
const forwardSignal = (signal) => {
26+
if (!child.killed) child.kill(signal);
27+
};
28+
process.on('SIGINT', forwardSignal);
29+
process.on('SIGTERM', forwardSignal);
30+
31+
child.on('exit', (code, signal) => {
32+
if (signal) {
33+
process.kill(process.pid, signal);
34+
return;
35+
}
36+
process.exit(code ?? 0);
37+
});
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// Single source of truth for the renderer dev server host/port.
2+
// Imported by rsbuild.config.ts (to bind the dev server), by
3+
// scripts/wait-for-electron-build.mjs (to probe readiness) and by
4+
// scripts/launch-electron-dev.mjs (to tell the main process where
5+
// to load from via MIDSCENE_STUDIO_RENDERER_URL).
6+
export const rendererDevHost = '127.0.0.1';
7+
export const rendererDevPort = 3210;
8+
export const rendererDevUrl = `http://${rendererDevHost}:${rendererDevPort}`;

apps/studio/scripts/wait-for-electron-build.mjs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import fs from 'node:fs';
22
import http from 'node:http';
33
import path from 'node:path';
44
import { fileURLToPath, pathToFileURL } from 'node:url';
5+
import { rendererDevUrl } from './renderer-dev-config.mjs';
56

67
const __filename = fileURLToPath(import.meta.url);
78
const __dirname = path.dirname(__filename);
@@ -12,7 +13,7 @@ export const defaultRequiredFiles = [
1213
path.join(rootDir, 'dist/preload/preload.cjs'),
1314
];
1415

15-
export const defaultRendererUrl = 'http://127.0.0.1:3210';
16+
export const defaultRendererUrl = rendererDevUrl;
1617
export const defaultMaxWaitMs = 180000;
1718
export const defaultPollIntervalMs = 500;
1819

@@ -26,7 +27,12 @@ export const readMtimeMs = (file) => {
2627
// (permission denied, IO error, ...) should surface instead of being
2728
// silently swallowed into a stale-build state.
2829
if (error && error.code === 'ENOENT') return null;
29-
throw error;
30+
throw new Error(
31+
`wait-for-electron-build: failed to stat ${file}: ${
32+
error instanceof Error ? error.message : String(error)
33+
}`,
34+
{ cause: error },
35+
);
3036
}
3137
};
3238

apps/studio/tests/wait-for-electron-build.test.mjs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,22 @@ describe('createFreshBuildChecker', () => {
8585
// fallback, so equal-mtime must stay false.
8686
expect(isFresh()).toBe(false);
8787
});
88+
89+
it('propagates non-ENOENT read errors instead of treating them as missing files', () => {
90+
// Regression guard: readMtimeMs narrows its catch to ENOENT so real IO
91+
// errors (permission denied, disk failure, ...) surface instead of being
92+
// silently reinterpreted as "file not built yet". If someone widens the
93+
// catch back to `catch {}`, this test must break.
94+
const throwingReader = () => {
95+
const error = new Error('permission denied');
96+
error.code = 'EACCES';
97+
throw error;
98+
};
99+
100+
expect(() => createFreshBuildChecker([FILE_A], throwingReader)).toThrow(
101+
/permission denied/,
102+
);
103+
});
88104
});
89105

90106
describe('waitForBuild', () => {

0 commit comments

Comments
 (0)