Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 2db4893

Browse filesBrowse files
Xstoudiaduh95
authored andcommitted
esm: ensure watch mode restarts after syntax errors
Move watch dependency reporting earlier in module resolution to ensure file dependencies are tracked even when parsing fails. Fixes: #61153 PR-URL: #61232 Reviewed-By: Aviv Keller <me@aviv.sh> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
1 parent 7e211e6 commit 2db4893
Copy full SHA for 2db4893

2 files changed

+132-5Lines changed: 132 additions & 5 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎lib/internal/modules/esm/loader.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/loader.js
+6-5Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,12 @@ class ModuleLoader {
542542
*/
543543
#getOrCreateModuleJobAfterResolve(parentURL, resolveResult, request, requestType) {
544544
const { url, format } = resolveResult;
545+
546+
if (process.env.WATCH_REPORT_DEPENDENCIES && process.send) {
547+
const type = requestType === kRequireInImportedCJS ? 'require' : 'import';
548+
process.send({ [`watch:${type}`]: [url] });
549+
}
550+
545551
// TODO(joyeecheung): update the module requests to use importAttributes as property names.
546552
const importAttributes = resolveResult.importAttributes ?? request.attributes;
547553
let job = this.loadCache.get(url, importAttributes.type);
@@ -570,11 +576,6 @@ class ModuleLoader {
570576
assert(moduleOrModulePromise instanceof ModuleWrap, `Expected ModuleWrap for loading ${url}`);
571577
}
572578

573-
if (process.env.WATCH_REPORT_DEPENDENCIES && process.send) {
574-
const type = requestType === kRequireInImportedCJS ? 'require' : 'import';
575-
process.send({ [`watch:${type}`]: [url] });
576-
}
577-
578579
const { ModuleJob, ModuleJobSync } = require('internal/modules/esm/module_job');
579580
// TODO(joyeecheung): use ModuleJobSync for kRequireInImportedCJS too.
580581
const ModuleJobCtor = (requestType === kImportInRequiredESM ? ModuleJobSync : ModuleJob);
Collapse file
+126Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
import * as common from '../common/index.mjs';
2+
import tmpdir from '../common/tmpdir.js';
3+
import assert from 'node:assert';
4+
import path from 'node:path';
5+
import { execPath } from 'node:process';
6+
import { spawn } from 'node:child_process';
7+
import { writeFileSync } from 'node:fs';
8+
import { inspect } from 'node:util';
9+
import { createInterface } from 'node:readline';
10+
11+
if (common.isIBMi)
12+
common.skip('IBMi does not support `fs.watch()`');
13+
14+
let tmpFiles = 0;
15+
function createTmpFile(content = 'console.log("running");', ext = '.js', basename = tmpdir.path) {
16+
const file = path.join(basename, `${tmpFiles++}${ext}`);
17+
writeFileSync(file, content);
18+
return file;
19+
}
20+
21+
function runInBackground({ args = [], options = {}, completed = 'Completed running', shouldFail = false }) {
22+
let future = Promise.withResolvers();
23+
let child;
24+
let stderr = '';
25+
let stdout = [];
26+
27+
const run = () => {
28+
args.unshift('--no-warnings');
29+
child = spawn(execPath, args, { encoding: 'utf8', stdio: 'pipe', ...options });
30+
31+
child.stderr.on('data', (data) => {
32+
stderr += data;
33+
});
34+
35+
const rl = createInterface({ input: child.stdout });
36+
rl.on('line', (data) => {
37+
if (!data.startsWith('Waiting for graceful termination') && !data.startsWith('Gracefully restarted')) {
38+
stdout.push(data);
39+
if (data.startsWith(completed)) {
40+
future.resolve({ stderr, stdout });
41+
future = Promise.withResolvers();
42+
stdout = [];
43+
stderr = '';
44+
} else if (data.startsWith('Failed running')) {
45+
if (shouldFail) {
46+
future.resolve({ stderr, stdout });
47+
} else {
48+
future.reject({ stderr, stdout });
49+
}
50+
future = Promise.withResolvers();
51+
stdout = [];
52+
stderr = '';
53+
}
54+
}
55+
});
56+
};
57+
58+
return {
59+
async done() {
60+
child?.kill();
61+
future.resolve();
62+
return { stdout, stderr };
63+
},
64+
restart(timeout = 1000) {
65+
if (!child) {
66+
run();
67+
}
68+
const timer = setTimeout(() => {
69+
if (!future.resolved) {
70+
child.kill();
71+
future.reject(new Error('Timed out waiting for restart'));
72+
}
73+
}, timeout);
74+
return future.promise.finally(() => {
75+
clearTimeout(timer);
76+
});
77+
},
78+
};
79+
}
80+
81+
tmpdir.refresh();
82+
83+
// Create initial file with valid code
84+
const initialContent = `console.log('hello, world');`;
85+
const file = createTmpFile(initialContent, '.mjs');
86+
87+
const { done, restart } = runInBackground({
88+
args: ['--watch', file],
89+
completed: 'Completed running',
90+
shouldFail: true,
91+
});
92+
93+
try {
94+
const { stdout, stderr } = await restart();
95+
assert.strictEqual(stderr, '');
96+
assert.deepStrictEqual(stdout, [
97+
'hello, world',
98+
`Completed running ${inspect(file)}. Waiting for file changes before restarting...`,
99+
]);
100+
101+
// Update file with syntax error
102+
const syntaxErrorContent = `console.log('hello, wor`;
103+
writeFileSync(file, syntaxErrorContent);
104+
105+
// Wait for the failed restart
106+
const { stderr: stderr2, stdout: stdout2 } = await restart();
107+
assert.match(stderr2, /SyntaxError: Invalid or unexpected token/);
108+
assert.deepStrictEqual(stdout2, [
109+
`Restarting ${inspect(file)}`,
110+
`Failed running ${inspect(file)}. Waiting for file changes before restarting...`,
111+
]);
112+
113+
writeFileSync(file, `console.log('hello again, world');`);
114+
115+
const { stderr: stderr3, stdout: stdout3 } = await restart();
116+
117+
// Verify it recovered and ran successfully
118+
assert.strictEqual(stderr3, '');
119+
assert.deepStrictEqual(stdout3, [
120+
`Restarting ${inspect(file)}`,
121+
'hello again, world',
122+
`Completed running ${inspect(file)}. Waiting for file changes before restarting...`,
123+
]);
124+
} finally {
125+
await done();
126+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.