Skip to content

Commit 3263ceb

Browse files
MoLowruyadorno
authored andcommitted
watch: watch for missing dependencies
PR-URL: #45348 Reviewed-By: Ruy Adorno <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Nitzan Uziely <[email protected]>
1 parent 8a34ef4 commit 3263ceb

File tree

6 files changed

+84
-22
lines changed

6 files changed

+84
-22
lines changed

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ const{
2828
ArrayPrototypeIncludes,
2929
ArrayPrototypeIndexOf,
3030
ArrayPrototypeJoin,
31+
ArrayPrototypeMap,
3132
ArrayPrototypePush,
3233
ArrayPrototypeSlice,
3334
ArrayPrototypeSplice,
@@ -191,7 +192,13 @@ function updateChildren(parent, child, scan){
191192

192193
functionreportModuleToWatchMode(filename){
193194
if(shouldReportRequiredModules&&process.send){
194-
process.send({'watch:require': filename});
195+
process.send({'watch:require': [filename]});
196+
}
197+
}
198+
199+
functionreportModuleNotFoundToWatchMode(basePath,extensions){
200+
if(shouldReportRequiredModules&&process.send){
201+
process.send({'watch:require': ArrayPrototypeMap(extensions,(ext)=>path.resolve(`${basePath}${ext}`))});
195202
}
196203
}
197204

@@ -648,6 +655,7 @@ Module._findPath = function(request, paths, isMain){
648655
Module._pathCache[cacheKey]=filename;
649656
returnfilename;
650657
}
658+
reportModuleNotFoundToWatchMode(basePath,ArrayPrototypeConcat([''],exts));
651659
}
652660

653661
returnfalse;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,7 @@ class ESMLoader{
463463
);
464464

465465
if(process.env.WATCH_REPORT_DEPENDENCIES&&process.send){
466-
process.send({'watch:import': url});
466+
process.send({'watch:import': [url]});
467467
}
468468

469469
constjob=newModuleJob(

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,9 @@ function finalizeResolution(resolved, base, preserveSymlinks){
254254
err.url=String(resolved);
255255
throwerr;
256256
}elseif(!stats.isFile()){
257+
if(process.env.WATCH_REPORT_DEPENDENCIES&&process.send){
258+
process.send({'watch:require': [path||resolved.pathname]});
259+
}
257260
thrownewERR_MODULE_NOT_FOUND(
258261
path||resolved.pathname,base&&fileURLToPath(base),'module');
259262
}

‎lib/internal/watch_mode/files_watcher.js‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
'use strict';
22

33
const{
4+
ArrayIsArray,
5+
ArrayPrototypeForEach,
46
SafeMap,
57
SafeSet,
68
StringPrototypeStartsWith,
@@ -94,6 +96,7 @@ class FilesWatcher extends EventEmitter{
9496
}
9597

9698
filterFile(file){
99+
if(!file)return;
97100
if(supportsRecursiveWatching){
98101
this.watchPath(dirname(file));
99102
}else{
@@ -109,11 +112,11 @@ class FilesWatcher extends EventEmitter{
109112
}
110113
child.on('message',(message)=>{
111114
try{
112-
if(message['watch:require']){
113-
this.filterFile(message['watch:require']);
115+
if(ArrayIsArray(message['watch:require'])){
116+
ArrayPrototypeForEach(message['watch:require'],(file)=>this.filterFile(file));
114117
}
115-
if(message['watch:import']){
116-
this.filterFile(fileURLToPath(message['watch:import']));
118+
if(ArrayIsArray(message['watch:import'])){
119+
ArrayPrototypeForEach(message['watch:import'],(file)=>this.filterFile(fileURLToPath(file)));
117120
}
118121
}catch{
119122
// Failed watching file. ignore

‎test/fixtures/watch-mode/ipc.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ const tmpdir = require('../../common/tmpdir');
66
consttmpfile=path.join(tmpdir.path,'file');
77
fs.writeFileSync(tmpfile,'');
88

9-
process.send({'watch:require': path.resolve(__filename)});
10-
process.send({'watch:import': url.pathToFileURL(path.resolve(__filename)).toString()});
11-
process.send({'watch:import': url.pathToFileURL(tmpfile).toString()});
12-
process.send({'watch:import': newURL('http://invalid.com').toString()});
9+
process.send({'watch:require': [path.resolve(__filename)]});
10+
process.send({'watch:import': [url.pathToFileURL(path.resolve(__filename)).toString()]});
11+
process.send({'watch:import': [url.pathToFileURL(tmpfile).toString()]});
12+
process.send({'watch:import': [newURL('http://invalid.com').toString()]});

‎test/sequential/test-watch-mode.mjs‎

Lines changed: 60 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,13 @@ import{spawn } from 'node:child_process'
99
import{writeFileSync,readFileSync}from'node:fs';
1010
import{inspect}from'node:util';
1111
import{once}from'node:events';
12+
import{createInterface}from'node:readline/promises';
1213

1314
if(common.isIBMi)
1415
common.skip('IBMi does not support `fs.watch()`');
1516

17+
constsupportsRecursive=common.isOSX||common.isWindows;
18+
1619
functionrestart(file){
1720
// To avoid flakiness, we save the file repeatedly until test is done
1821
writeFileSync(file,readFileSync(file));
@@ -59,8 +62,8 @@ async function spawnWithRestarts({
5962
}
6063

6164
lettmpFiles=0;
62-
functioncreateTmpFile(content='console.log("running");'){
63-
constfile=path.join(tmpdir.path,`${tmpFiles++}.js`);
65+
functioncreateTmpFile(content='console.log("running");',ext='.js'){
66+
constfile=path.join(tmpdir.path,`${tmpFiles++}${ext}`);
6467
writeFileSync(file,content);
6568
returnfile;
6669
}
@@ -74,11 +77,29 @@ function assertRestartedCorrectly({stdout, messages:{inner, completed, restar
7477
assert.deepStrictEqual(lines.slice(-end.length),end);
7578
}
7679

80+
asyncfunctionfailWriteSucceed({ file, watchedFile }){
81+
constchild=spawn(execPath,['--watch','--no-warnings',file],{encoding: 'utf8'});
82+
83+
try{
84+
// Break the chunks into lines
85+
forawait(constdataofcreateInterface({input: child.stdout})){
86+
if(data.startsWith('Completed running')){
87+
break;
88+
}
89+
if(data.startsWith('Failed running')){
90+
writeFileSync(watchedFile,'console.log("test has ran");');
91+
}
92+
}
93+
}finally{
94+
child.kill();
95+
}
96+
}
97+
7798
tmpdir.refresh();
7899

79100
// Warning: this suite can run safely with concurrency: true
80101
// only if tests do not watch/depend on the same files
81-
describe('watch mode',{concurrency: true,timeout: 60_0000},()=>{
102+
describe('watch mode',{concurrency: true,timeout: 60_000},()=>{
82103
it('should watch changes to a file - event loop ended',async()=>{
83104
constfile=createTmpFile();
84105
const{ stderr, stdout }=awaitspawnWithRestarts({ file });
@@ -104,16 +125,8 @@ describe('watch mode',{concurrency: true, timeout: 60_0000 }, () =>{
104125
});
105126
});
106127

107-
it('should not watch when running an non-existing file',async()=>{
108-
constfile=fixtures.path('watch-mode/non-existing.js');
109-
const{ stderr, stdout }=awaitspawnWithRestarts({ file,restarts: 0});
110-
111-
assert.match(stderr,/code:'MODULE_NOT_FOUND'/);
112-
assert.strictEqual(stdout,`Failed running ${inspect(file)}\n`);
113-
});
114-
115128
it('should watch when running an non-existing file - when specified under --watch-path',{
116-
skip: !common.isOSX&&!common.isWindows
129+
skip: !supportsRecursive
117130
},async()=>{
118131
constfile=fixtures.path('watch-mode/subdir/non-existing.js');
119132
constwatchedFile=fixtures.path('watch-mode/subdir/file.js');
@@ -220,4 +233,39 @@ describe('watch mode',{concurrency: true, timeout: 60_0000 }, () =>{
220233
messages: {restarted: `Restarting ${inspect(file)}`,completed: `Completed running ${inspect(file)}`},
221234
});
222235
});
236+
237+
// TODO: Remove skip after https://github.com/nodejs/node/pull/45271 lands
238+
it('should not watch when running an missing file',{
239+
skip: !supportsRecursive
240+
},async()=>{
241+
constnonExistingfile=path.join(tmpdir.path,`${tmpFiles++}.js`);
242+
awaitfailWriteSucceed({file: nonExistingfile,watchedFile: nonExistingfile});
243+
});
244+
245+
it('should not watch when running an missing mjs file',{
246+
skip: !supportsRecursive
247+
},async()=>{
248+
constnonExistingfile=path.join(tmpdir.path,`${tmpFiles++}.mjs`);
249+
awaitfailWriteSucceed({file: nonExistingfile,watchedFile: nonExistingfile});
250+
});
251+
252+
it('should watch changes to previously missing dependency',{
253+
skip: !supportsRecursive
254+
},async()=>{
255+
constdependency=path.join(tmpdir.path,`${tmpFiles++}.js`);
256+
constrelativeDependencyPath=`./${path.basename(dependency)}`;
257+
constdependant=createTmpFile(`console.log(require('${relativeDependencyPath}'))`);
258+
259+
awaitfailWriteSucceed({file: dependant,watchedFile: dependency});
260+
});
261+
262+
it('should watch changes to previously missing ESM dependency',{
263+
skip: !supportsRecursive
264+
},async()=>{
265+
constdependency=path.join(tmpdir.path,`${tmpFiles++}.mjs`);
266+
constrelativeDependencyPath=`./${path.basename(dependency)}`;
267+
constdependant=createTmpFile(`import '${relativeDependencyPath}'`,'.mjs');
268+
269+
awaitfailWriteSucceed({file: dependant,watchedFile: dependency});
270+
});
223271
});

0 commit comments

Comments
(0)