Skip to content

Commit ebed67b

Browse files
committed
args, watch command review fix
1 parent 258741d commit ebed67b

4 files changed

Lines changed: 491 additions & 26 deletions

File tree

src/commands/deploy.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ export const deployCommand: Command = {
3939
const configLoadResult = await loadConfigFile(project.rootPath);
4040
const configFromFile = configLoadResult.config;
4141
const configFromEnv = loadDeployConfigFromEnv();
42+
const allowHttpProvided = hasBooleanOption(argv, 'allow-http');
43+
const updateProvided = hasBooleanOption(argv, 'update');
4244

4345
const cliConfig: DeployConfig = {
4446
url: parsed.values.url,
@@ -47,8 +49,8 @@ export const deployCommand: Command = {
4749
token: parsed.values.token,
4850
userId: parsed.values.userId,
4951
code: parsed.values.code,
50-
allowHttp: parsed.values['allow-http'],
51-
update: parsed.values.update,
52+
allowHttp: allowHttpProvided ? parsed.values['allow-http'] : undefined,
53+
update: updateProvided ? parsed.values.update : undefined,
5254
};
5355

5456
const deployConfig = mergeDeployConfig(mergeDeployConfig(configFromFile, configFromEnv), cliConfig);
@@ -100,3 +102,8 @@ export const deployCommand: Command = {
100102
success(`Deployment finished (${result.mode}).`);
101103
},
102104
};
105+
106+
function hasBooleanOption(args: string[], option: string): boolean {
107+
const optionPrefix = `--${option}`;
108+
return args.some((arg) => arg === optionPrefix || arg.startsWith(`${optionPrefix}=`));
109+
}

src/commands/watch.ts

Lines changed: 142 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { watch } from 'fs';
1+
import { FSWatcher, watch } from 'fs';
2+
import { readdir } from 'fs/promises';
23
import { parseArgs } from 'util';
34
import path from 'path';
45

@@ -27,9 +28,9 @@ export const watchCommand: Command = {
2728
token: { type: 'string', short: 't' },
2829
userId: { type: 'string', short: 'i' },
2930
code: { type: 'string', short: 'c' },
30-
'allow-http': { type: 'boolean', default: false },
31+
'allow-http': { type: 'boolean' },
3132
'legacy-compiler': { type: 'boolean', default: false },
32-
update: { type: 'boolean', default: false },
33+
update: { type: 'boolean' },
3334
force: { type: 'boolean', short: 'f', default: false },
3435
verbose: { type: 'boolean', short: 'v', default: false },
3536
debounce: { type: 'string' },
@@ -42,6 +43,8 @@ export const watchCommand: Command = {
4243
const configLoadResult = await loadConfigFile(project.rootPath);
4344
const configFromFile = configLoadResult.config;
4445
const configFromEnv = loadDeployConfigFromEnv();
46+
const allowHttpProvided = hasBooleanOption(argv, 'allow-http');
47+
const updateProvided = hasBooleanOption(argv, 'update');
4548

4649
const cliConfig: DeployConfig = {
4750
url: parsed.values.url,
@@ -50,8 +53,8 @@ export const watchCommand: Command = {
5053
token: parsed.values.token,
5154
userId: parsed.values.userId,
5255
code: parsed.values.code,
53-
allowHttp: parsed.values['allow-http'],
54-
update: parsed.values.update,
56+
allowHttp: allowHttpProvided ? parsed.values['allow-http'] : undefined,
57+
update: updateProvided ? parsed.values.update : undefined,
5558
};
5659

5760
const deployConfig = mergeDeployConfig(mergeDeployConfig(configFromFile, configFromEnv), cliConfig);
@@ -131,35 +134,153 @@ export const watchCommand: Command = {
131134
await runDeployment();
132135

133136
let timer: NodeJS.Timeout | undefined;
137+
const watchers = new Map<string, FSWatcher>();
138+
let watcherErrorHandler: ((error: Error) => void) | undefined;
139+
const recursiveWatchSupported = supportsRecursiveWatch();
134140

135-
const watcher = watch(project.rootPath, { recursive: true, encoding: 'utf8' }, (_eventType, fileName) => {
136-
if (!fileName) {
137-
return;
141+
const removeWatcher = (watchPath: string): void => {
142+
const watcher = watchers.get(watchPath);
143+
144+
if (watcher) {
145+
if (watcherErrorHandler) {
146+
watcher.off('error', watcherErrorHandler);
147+
}
148+
149+
watcher.close();
150+
watchers.delete(watchPath);
138151
}
152+
};
139153

140-
const relativePath = fileName.replace(/\\/g, '/');
154+
const addWatcher = (watchPath: string, recursive: boolean): void => {
155+
const watcher = watch(watchPath, { recursive, encoding: 'utf8' }, (eventType, fileName) => {
156+
if (!fileName) {
157+
return;
158+
}
141159

142-
if (isIgnored(relativePath)) {
143-
return;
160+
if (!recursiveWatchSupported && eventType === 'rename') {
161+
void syncWatchers();
162+
}
163+
const absolutePath = path.resolve(watchPath, fileName);
164+
const relativePath = toRelativeRootPath(project.rootPath, absolutePath);
165+
166+
if (!relativePath || isIgnored(relativePath)) {
167+
return;
168+
}
169+
170+
if (timer) {
171+
clearTimeout(timer);
172+
}
173+
174+
timer = setTimeout(() => {
175+
void runDeployment();
176+
}, debounceMs);
177+
});
178+
179+
if (watcherErrorHandler) {
180+
watcher.on('error', watcherErrorHandler);
144181
}
145182

146-
if (timer) {
147-
clearTimeout(timer);
183+
watchers.set(watchPath, watcher);
184+
};
185+
186+
const syncWatchers = async (): Promise<void> => {
187+
const discoveredDirectories = await collectDirectories(project.rootPath);
188+
189+
for (const directoryPath of discoveredDirectories) {
190+
if (!watchers.has(directoryPath)) {
191+
addWatcher(directoryPath, false);
192+
}
148193
}
194+
};
149195

150-
timer = setTimeout(() => {
151-
void runDeployment();
152-
}, debounceMs);
153-
});
196+
if (recursiveWatchSupported) {
197+
addWatcher(project.rootPath, true);
198+
} else {
199+
warn(
200+
'Recursive fs.watch is not supported on this platform. Falling back to multi-directory watch mode.',
201+
);
202+
await syncWatchers();
203+
}
154204

155205
step('Watching for changes. Press Ctrl+C to stop.');
156206

157207
await new Promise<void>((resolve, reject) => {
158-
watcher.on('error', reject);
159-
process.on('SIGINT', () => {
160-
watcher.close();
208+
let sigintHandler: (() => void) | undefined;
209+
210+
const cleanup = (): void => {
211+
if (timer) {
212+
clearTimeout(timer);
213+
timer = undefined;
214+
}
215+
216+
if (sigintHandler) {
217+
process.off('SIGINT', sigintHandler);
218+
sigintHandler = undefined;
219+
}
220+
221+
for (const watcherPath of Array.from(watchers.keys())) {
222+
removeWatcher(watcherPath);
223+
}
224+
225+
watcherErrorHandler = undefined;
226+
};
227+
228+
watcherErrorHandler = (error: Error) => {
229+
cleanup();
230+
reject(error);
231+
};
232+
233+
for (const watcher of watchers.values()) {
234+
watcher.on('error', watcherErrorHandler);
235+
}
236+
237+
sigintHandler = () => {
238+
cleanup();
161239
resolve();
162-
});
240+
};
241+
242+
process.once('SIGINT', sigintHandler);
163243
});
164244
},
165245
};
246+
247+
function hasBooleanOption(args: string[], option: string): boolean {
248+
const optionPrefix = `--${option}`;
249+
return args.some((arg) => arg === optionPrefix || arg.startsWith(`${optionPrefix}=`));
250+
}
251+
252+
function supportsRecursiveWatch(): boolean {
253+
return process.platform === 'darwin' || process.platform === 'win32';
254+
}
255+
256+
function toRelativeRootPath(rootPath: string, absolutePath: string): string | undefined {
257+
const relativePath = path.relative(rootPath, absolutePath);
258+
259+
if (!relativePath || relativePath.startsWith('..') || path.isAbsolute(relativePath)) {
260+
return undefined;
261+
}
262+
263+
return relativePath.replace(/\\/g, '/');
264+
}
265+
266+
async function collectDirectories(rootPath: string): Promise<string[]> {
267+
const directories: string[] = [rootPath];
268+
const queue: string[] = [rootPath];
269+
270+
while (queue.length > 0) {
271+
const directoryPath = queue.pop() as string;
272+
const entries = await readdir(directoryPath, { withFileTypes: true, encoding: 'utf8' }).catch(() => []);
273+
274+
for (const entry of entries) {
275+
if (!entry.isDirectory()) {
276+
continue;
277+
}
278+
279+
const nestedDirectoryPath = path.join(directoryPath, entry.name);
280+
directories.push(nestedDirectoryPath);
281+
queue.push(nestedDirectoryPath);
282+
}
283+
}
284+
285+
return directories;
286+
}

test/command-deploy.test.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ test('deploy command runs full deploy pipeline and shows warnings', async () =>
5757

5858
assert.equal(validatedConfig?.token, 'env-token');
5959
assert.equal(validatedConfig?.userId, 'env-user');
60+
assert.equal(validatedConfig?.allowHttp, true);
6061
assert.equal(uploadZipPath, '/project/dist/a_1.0.0.zip');
6162
assert.equal(calls.some(([type, message]) => type === 'warn' && message.includes('--experimental-native-compiler')), true);
6263
assert.equal(calls.some(([type, message]) => type === 'warn' && message.includes('Ignoring legacy .rcappsconfig field')), true);
@@ -66,6 +67,86 @@ test('deploy command runs full deploy pipeline and shows warnings', async () =>
6667
}
6768
});
6869

70+
test('deploy command preserves allow-http and update from config/env when flags are not provided', async () => {
71+
let validatedConfig: Record<string, unknown> | undefined;
72+
73+
const restore = patchMany([
74+
{
75+
obj: projectCore,
76+
key: 'loadProject',
77+
value: async () => ({ rootPath: '/project', appJsonPath: '/project/app.json', manifest: { id: 'app.id', nameSlug: 'a', version: '1.0.0' } }),
78+
},
79+
{ obj: projectCore, key: 'loadConfigFile', value: async () => ({ config: { allowHttp: true, update: true }, legacyFields: [] }) },
80+
{
81+
obj: envCore,
82+
key: 'loadDeployConfigFromEnv',
83+
value: () => ({ url: 'http://localhost:3000', username: 'user', password: 'pass' }),
84+
},
85+
{
86+
obj: deployCore,
87+
key: 'validateDeployCredentials',
88+
value: (config: unknown) => {
89+
validatedConfig = config as Record<string, unknown>;
90+
},
91+
},
92+
{ obj: deployCore, key: 'getServerInfo', value: async () => ({}) },
93+
{ obj: compiler, key: 'buildAndPackage', value: async () => 'dist/a_1.0.0.zip' },
94+
{ obj: deployCore, key: 'uploadApp', value: async () => ({ mode: 'create' }) },
95+
{ obj: output, key: 'warn', value: () => {} },
96+
{ obj: output, key: 'step', value: () => {} },
97+
{ obj: output, key: 'success', value: () => {} },
98+
{ obj: output, key: 'verbose', value: () => {} },
99+
]);
100+
101+
try {
102+
await deployCommand.run([], { cwd: '/project' });
103+
assert.equal(validatedConfig?.allowHttp, true);
104+
assert.equal(validatedConfig?.update, true);
105+
} finally {
106+
restore();
107+
}
108+
});
109+
110+
test('deploy command applies allow-http and update when flags are provided', async () => {
111+
let validatedConfig: Record<string, unknown> | undefined;
112+
113+
const restore = patchMany([
114+
{
115+
obj: projectCore,
116+
key: 'loadProject',
117+
value: async () => ({ rootPath: '/project', appJsonPath: '/project/app.json', manifest: { id: 'app.id', nameSlug: 'a', version: '1.0.0' } }),
118+
},
119+
{ obj: projectCore, key: 'loadConfigFile', value: async () => ({ config: { allowHttp: false, update: false }, legacyFields: [] }) },
120+
{
121+
obj: envCore,
122+
key: 'loadDeployConfigFromEnv',
123+
value: () => ({ url: 'http://localhost:3000', username: 'user', password: 'pass' }),
124+
},
125+
{
126+
obj: deployCore,
127+
key: 'validateDeployCredentials',
128+
value: (config: unknown) => {
129+
validatedConfig = config as Record<string, unknown>;
130+
},
131+
},
132+
{ obj: deployCore, key: 'getServerInfo', value: async () => ({}) },
133+
{ obj: compiler, key: 'buildAndPackage', value: async () => 'dist/a_1.0.0.zip' },
134+
{ obj: deployCore, key: 'uploadApp', value: async () => ({ mode: 'update' }) },
135+
{ obj: output, key: 'warn', value: () => {} },
136+
{ obj: output, key: 'step', value: () => {} },
137+
{ obj: output, key: 'success', value: () => {} },
138+
{ obj: output, key: 'verbose', value: () => {} },
139+
]);
140+
141+
try {
142+
await deployCommand.run(['--allow-http', '--update'], { cwd: '/project' });
143+
assert.equal(validatedConfig?.allowHttp, true);
144+
assert.equal(validatedConfig?.update, true);
145+
} finally {
146+
restore();
147+
}
148+
});
149+
69150
test('deploy command passes legacy compiler switch to build step', async () => {
70151
let compilerOptions: { force: boolean; verbose: boolean; useNativeCompiler: boolean } | undefined;
71152

0 commit comments

Comments
 (0)