Skip to content

Commit 5963e36

Browse files
committed
refactor: Add type annotations and address code review feedback
Address feedback from automated code review: - Add JSDoc type annotations throughout dev-watch.js for better type safety (ProcessInfo, ChildProcess, FSWatcher types) - Add clarifying comment explaining child_process.spawn is Bun-compatible (Bun supports Node.js APIs) - Convert test file from require() to ES6 imports (readFileSync) - Add function documentation with @param and @returns annotations Note: child_process.spawn is intentionally kept instead of Bun.spawn for compatibility with existing event handlers. Bun fully supports child_process APIs. fs.watch from node:fs is the correct API to use (native Bun.watch() doesn't exist yet). All tests passing (8/8).
1 parent 9bc72dc commit 5963e36

File tree

2 files changed

+39
-12
lines changed

2 files changed

+39
-12
lines changed

scripts/__tests__/dev-watch.test.ts

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
66
import { spawn, type ChildProcess } from 'child_process';
77
import { writeFile, readFile, mkdir } from 'node:fs/promises';
8-
import { existsSync } from 'node:fs';
8+
import { existsSync, readFileSync } from 'node:fs';
99
import path from 'path';
1010

1111
const PROJECT_ROOT = path.resolve(__dirname, '../..');
@@ -95,8 +95,7 @@ describe('Hot Reload Functionality', () => {
9595

9696
test('watched directories should include all CLI dependencies', () => {
9797
// Read the dev-watch.js file and verify it watches the right directories
98-
const fs = require('node:fs');
99-
const devWatchContent = fs.readFileSync(DEV_SCRIPT, 'utf-8');
98+
const devWatchContent = readFileSync(DEV_SCRIPT, 'utf-8');
10099

101100
// Check that critical packages are being watched
102101
const expectedPackages = [
@@ -116,8 +115,7 @@ describe('Hot Reload Functionality', () => {
116115

117116
test('debounce mechanism should prevent rapid rebuilds', async () => {
118117
// This test verifies that the debounce timer is set
119-
const fs = require('node:fs');
120-
const devWatchContent = fs.readFileSync(DEV_SCRIPT, 'utf-8');
118+
const devWatchContent = readFileSync(DEV_SCRIPT, 'utf-8');
121119

122120
// Check for debounce implementation
123121
expect(devWatchContent).toContain('rebuildDebounceTimer');
@@ -126,8 +124,7 @@ describe('Hot Reload Functionality', () => {
126124
});
127125

128126
test('rebuild function should stop server before rebuilding', () => {
129-
const fs = require('node:fs');
130-
const devWatchContent = fs.readFileSync(DEV_SCRIPT, 'utf-8');
127+
const devWatchContent = readFileSync(DEV_SCRIPT, 'utf-8');
131128

132129
// Check that rebuild logic includes server shutdown
133130
expect(devWatchContent).toContain('rebuildAndRestartServer');
@@ -136,17 +133,15 @@ describe('Hot Reload Functionality', () => {
136133
});
137134

138135
test('file watcher should only watch TypeScript files', () => {
139-
const fs = require('node:fs');
140-
const devWatchContent = fs.readFileSync(DEV_SCRIPT, 'utf-8');
136+
const devWatchContent = readFileSync(DEV_SCRIPT, 'utf-8');
141137

142138
// Check that file watcher filters for TypeScript files
143139
expect(devWatchContent).toContain('.ts');
144140
expect(devWatchContent).toContain('.tsx');
145141
});
146142

147143
test('cleanup should handle file watchers properly', () => {
148-
const fs = require('node:fs');
149-
const devWatchContent = fs.readFileSync(DEV_SCRIPT, 'utf-8');
144+
const devWatchContent = readFileSync(DEV_SCRIPT, 'utf-8');
150145

151146
// Check that cleanup includes watcher handling
152147
expect(devWatchContent).toContain("type === 'watcher'");

scripts/dev-watch.js

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,18 @@
11
#!/usr/bin/env bun
22

3+
// Note: Using child_process.spawn for compatibility with existing event handlers.
4+
// Bun fully supports child_process APIs. For new code, consider Bun.spawn or
5+
// the bun-exec utilities in packages/cli/src/utils/bun-exec.ts
36
import { spawn } from 'child_process';
47
import path from 'path';
58
import { fileURLToPath } from 'url';
69
import { watch } from 'node:fs';
710

11+
/** @typedef {import('node:fs').FSWatcher} FSWatcher */
12+
/** @typedef {import('child_process').ChildProcess} ChildProcess */
13+
/** @typedef {'build' | 'server' | 'client' | 'watcher'} ProcessType */
14+
/** @typedef {{ name: string; child: ChildProcess | FSWatcher; type: ProcessType }} ProcessInfo */
15+
816
const __filename = fileURLToPath(import.meta.url);
917
const __dirname = path.dirname(__filename);
1018

@@ -24,20 +32,36 @@ const watchDirs = [
2432
path.resolve(packagesDir, 'config/src'),
2533
];
2634

35+
/** @type {ProcessInfo[]} */
2736
let processes = [];
37+
/** @type {boolean} */
2838
let isShuttingDown = false;
39+
/** @type {boolean} */
2940
let serverReady = false;
41+
/** @type {boolean} */
3042
let isRebuilding = false;
43+
/** @type {NodeJS.Timeout | null} */
3144
let rebuildDebounceTimer = null;
3245

46+
/**
47+
* Log a message with a prefix and timestamp
48+
* @param {string} prefix - The log prefix
49+
* @param {string} message - The message to log
50+
*/
3351
function log(prefix, message) {
3452
console.log(`[${prefix}] ${new Date().toLocaleTimeString()} - ${message}`);
3553
}
3654

3755
// Simple color helpers (avoid external deps)
56+
/** @param {string} s */
3857
const cyan = (s) => `\x1b[36m${s}\x1b[0m`;
3958

40-
// Health check function to verify server is responding
59+
/**
60+
* Health check function to verify server is responding
61+
* @param {string} [url='http://localhost:3000/api/server/ping'] - URL to check
62+
* @param {number} [maxAttempts=30] - Maximum number of attempts
63+
* @returns {Promise<boolean>}
64+
*/
4165
async function waitForServer(url = 'http://localhost:3000/api/server/ping', maxAttempts = 30) {
4266
log('HEALTH', `Waiting for server to be ready at ${url}...`);
4367

@@ -70,6 +94,10 @@ async function waitForServer(url = 'http://localhost:3000/api/server/ping', maxA
7094
return false;
7195
}
7296

97+
/**
98+
* Start the Vite development server for the frontend
99+
* @returns {ChildProcess}
100+
*/
73101
function startViteDevServer() {
74102
log('CLIENT', 'Starting Vite dev server with HMR...');
75103

@@ -101,6 +129,10 @@ function startViteDevServer() {
101129
return child;
102130
}
103131

132+
/**
133+
* Start the CLI server by building and running it
134+
* @returns {ChildProcess}
135+
*/
104136
function startCliServer() {
105137
log('CLI', 'Starting CLI server build...');
106138

0 commit comments

Comments
 (0)