Skip to content

Commit 2db4893

Browse 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

2 files changed

Lines changed: 132 additions & 5 deletions

File tree

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

Lines 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+
consttype=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
constimportAttributes=resolveResult.importAttributes??request.attributes;
547553
letjob=this.loadCache.get(url,importAttributes.type);
@@ -570,11 +576,6 @@ class ModuleLoader {
570576
assert(moduleOrModulePromiseinstanceofModuleWrap,`Expected ModuleWrap for loading ${url}`);
571577
}
572578

573-
if(process.env.WATCH_REPORT_DEPENDENCIES&&process.send){
574-
consttype=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
constModuleJobCtor=(requestType===kImportInRequiredESM ? ModuleJobSync : ModuleJob);
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
import*ascommonfrom'../common/index.mjs';
2+
importtmpdirfrom'../common/tmpdir.js';
3+
importassertfrom'node:assert';
4+
importpathfrom'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+
lettmpFiles=0;
15+
functioncreateTmpFile(content='console.log("running");',ext='.js',basename=tmpdir.path){
16+
constfile=path.join(basename,`${tmpFiles++}${ext}`);
17+
writeFileSync(file,content);
18+
returnfile;
19+
}
20+
21+
functionrunInBackground({ args =[], options ={}, completed ='Completed running', shouldFail =false}){
22+
letfuture=Promise.withResolvers();
23+
letchild;
24+
letstderr='';
25+
letstdout=[];
26+
27+
construn=()=>{
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+
constrl=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+
}elseif(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+
asyncdone(){
60+
child?.kill();
61+
future.resolve();
62+
return{ stdout, stderr };
63+
},
64+
restart(timeout=1000){
65+
if(!child){
66+
run();
67+
}
68+
consttimer=setTimeout(()=>{
69+
if(!future.resolved){
70+
child.kill();
71+
future.reject(newError('Timed out waiting for restart'));
72+
}
73+
},timeout);
74+
returnfuture.promise.finally(()=>{
75+
clearTimeout(timer);
76+
});
77+
},
78+
};
79+
}
80+
81+
tmpdir.refresh();
82+
83+
// Create initial file with valid code
84+
constinitialContent=`console.log('hello, world');`;
85+
constfile=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 }=awaitrestart();
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+
constsyntaxErrorContent=`console.log('hello, wor`;
103+
writeFileSync(file,syntaxErrorContent);
104+
105+
// Wait for the failed restart
106+
const{stderr: stderr2,stdout: stdout2}=awaitrestart();
107+
assert.match(stderr2,/SyntaxError:Invalidorunexpectedtoken/);
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}=awaitrestart();
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+
awaitdone();
126+
}

0 commit comments

Comments
 (0)