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]>
nodejs · Jan 3, 2023 · 24bfe54 · 24bfe54
1 parent 7641044
commit 24bfe54
Show file tree

Hide file tree

Showing 6 changed files with 84 additions and 22 deletions.
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
@@ -28,6 +28,7 @@ const {
   ArrayPrototypeIncludes,
   ArrayPrototypeIndexOf,
   ArrayPrototypeJoin,
+  ArrayPrototypeMap,
   ArrayPrototypePush,
   ArrayPrototypeSlice,
   ArrayPrototypeSplice,
@@ -191,7 +192,13 @@ function updateChildren(parent, child, scan) {
 
 function reportModuleToWatchMode(filename) {
   if (shouldReportRequiredModules && process.send) {
-    process.send({ 'watch:require': filename });
+    process.send({ 'watch:require': [filename] });
+  }
+}
+
+function reportModuleNotFoundToWatchMode(basePath, extensions) {
+  if (shouldReportRequiredModules && process.send) {
+    process.send({ 'watch:require': ArrayPrototypeMap(extensions, (ext) => path.resolve(`${basePath}${ext}`)) });
   }
 }
 
@@ -648,6 +655,7 @@ Module._findPath = function(request, paths, isMain) {
       Module._pathCache[cacheKey] = filename;
       return filename;
     }
+    reportModuleNotFoundToWatchMode(basePath, ArrayPrototypeConcat([''], exts));
   }
 
   return false;

diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
@@ -474,7 +474,7 @@ class ESMLoader {
     );
 
     if (process.env.WATCH_REPORT_DEPENDENCIES && process.send) {
-      process.send({ 'watch:import': url });
+      process.send({ 'watch:import': [url] });
     }
 
     const job = new ModuleJob(

diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js
@@ -320,6 +320,9 @@ function finalizeResolution(resolved, base, preserveSymlinks) {
     err.url = String(resolved);
     throw err;
   } else if (!stats.isFile()) {
+    if (process.env.WATCH_REPORT_DEPENDENCIES && process.send) {
+      process.send({ 'watch:require': [path || resolved.pathname] });
+    }
     throw new ERR_MODULE_NOT_FOUND(
       path || resolved.pathname, base && fileURLToPath(base), 'module');
   }

diff --git a/lib/internal/watch_mode/files_watcher.js b/lib/internal/watch_mode/files_watcher.js
@@ -1,6 +1,8 @@
 'use strict';
 
 const {
+  ArrayIsArray,
+  ArrayPrototypeForEach,
   SafeMap,
   SafeSet,
   StringPrototypeStartsWith,
@@ -94,6 +96,7 @@ class FilesWatcher extends EventEmitter {
   }
 
   filterFile(file) {
+    if (!file) return;
     if (supportsRecursiveWatching) {
       this.watchPath(dirname(file));
     } else {
@@ -109,11 +112,11 @@ class FilesWatcher extends EventEmitter {
     }
     child.on('message', (message) => {
       try {
-        if (message['watch:require']) {
-          this.filterFile(message['watch:require']);
+        if (ArrayIsArray(message['watch:require'])) {
+          ArrayPrototypeForEach(message['watch:require'], (file) => this.filterFile(file));
         }
-        if (message['watch:import']) {
-          this.filterFile(fileURLToPath(message['watch:import']));
+        if (ArrayIsArray(message['watch:import'])) {
+          ArrayPrototypeForEach(message['watch:import'], (file) => this.filterFile(fileURLToPath(file)));
         }
       } catch {
         // Failed watching file. ignore

diff --git a/test/fixtures/watch-mode/ipc.js b/test/fixtures/watch-mode/ipc.js
@@ -6,7 +6,7 @@ const tmpdir = require('../../common/tmpdir');
 const tmpfile = path.join(tmpdir.path, 'file');
 fs.writeFileSync(tmpfile, '');
 
-process.send({ 'watch:require': path.resolve(__filename) });
-process.send({ 'watch:import': url.pathToFileURL(path.resolve(__filename)).toString() });
-process.send({ 'watch:import': url.pathToFileURL(tmpfile).toString() });
-process.send({ 'watch:import': new URL('http://invalid.com').toString() });
+process.send({ 'watch:require': [path.resolve(__filename)] });
+process.send({ 'watch:import': [url.pathToFileURL(path.resolve(__filename)).toString()] });
+process.send({ 'watch:import': [url.pathToFileURL(tmpfile).toString()] });
+process.send({ 'watch:import': [new URL('http://invalid.com').toString()] });
diff --git a/test/sequential/test-watch-mode.mjs b/test/sequential/test-watch-mode.mjs
@@ -9,10 +9,13 @@ import { spawn } from 'node:child_process';
 import { writeFileSync, readFileSync } from 'node:fs';
 import { inspect } from 'node:util';
 import { once } from 'node:events';
+import { createInterface } from 'node:readline/promises';
 
 if (common.isIBMi)
   common.skip('IBMi does not support `fs.watch()`');
 
+const supportsRecursive = common.isOSX || common.isWindows;
+
 function restart(file) {
   // To avoid flakiness, we save the file repeatedly until test is done
   writeFileSync(file, readFileSync(file));
@@ -59,8 +62,8 @@ async function spawnWithRestarts({
 }
 
 let tmpFiles = 0;
-function createTmpFile(content = 'console.log("running");') {
-  const file = path.join(tmpdir.path, `${tmpFiles++}.js`);
+function createTmpFile(content = 'console.log("running");', ext = '.js') {
+  const file = path.join(tmpdir.path, `${tmpFiles++}${ext}`);
   writeFileSync(file, content);
   return file;
 }
@@ -74,11 +77,29 @@ function assertRestartedCorrectly({ stdout, messages: { inner, completed, restar
   assert.deepStrictEqual(lines.slice(-end.length), end);
 }
 
+async function failWriteSucceed({ file, watchedFile }) {
+  const child = spawn(execPath, ['--watch', '--no-warnings', file], { encoding: 'utf8' });
+
+  try {
+    // Break the chunks into lines
+    for await (const data of createInterface({ input: child.stdout })) {
+      if (data.startsWith('Completed running')) {
+        break;
+      }
+      if (data.startsWith('Failed running')) {
+        writeFileSync(watchedFile, 'console.log("test has ran");');
+      }
+    }
+  } finally {
+    child.kill();
+  }
+}
+
 tmpdir.refresh();
 
 // Warning: this suite can run safely with concurrency: true
 // only if tests do not watch/depend on the same files
-describe('watch mode', { concurrency: true, timeout: 60_0000 }, () => {
+describe('watch mode', { concurrency: true, timeout: 60_000 }, () => {
   it('should watch changes to a file - event loop ended', async () => {
     const file = createTmpFile();
     const { stderr, stdout } = await spawnWithRestarts({ file });
@@ -104,16 +125,8 @@ describe('watch mode', { concurrency: true, timeout: 60_0000 }, () => {
     });
   });
 
-  it('should not watch when running an non-existing file', async () => {
-    const file = fixtures.path('watch-mode/non-existing.js');
-    const { stderr, stdout } = await spawnWithRestarts({ file, restarts: 0 });
-
-    assert.match(stderr, /code: 'MODULE_NOT_FOUND'/);
-    assert.strictEqual(stdout, `Failed running ${inspect(file)}\n`);
-  });
-
   it('should watch when running an non-existing file - when specified under --watch-path', {
-    skip: !common.isOSX && !common.isWindows
+    skip: !supportsRecursive
   }, async () => {
     const file = fixtures.path('watch-mode/subdir/non-existing.js');
     const watchedFile = fixtures.path('watch-mode/subdir/file.js');
@@ -222,4 +235,39 @@ describe('watch mode', { concurrency: true, timeout: 60_0000 }, () => {
       messages: { restarted: `Restarting ${inspect(file)}`, completed: `Completed running ${inspect(file)}` },
     });
   });
+
+  // TODO: Remove skip after https://github.com/nodejs/node/pull/45271 lands
+  it('should not watch when running an missing file', {
+    skip: !supportsRecursive
+  }, async () => {
+    const nonExistingfile = path.join(tmpdir.path, `${tmpFiles++}.js`);
+    await failWriteSucceed({ file: nonExistingfile, watchedFile: nonExistingfile });
+  });
+
+  it('should not watch when running an missing mjs file', {
+    skip: !supportsRecursive
+  }, async () => {
+    const nonExistingfile = path.join(tmpdir.path, `${tmpFiles++}.mjs`);
+    await failWriteSucceed({ file: nonExistingfile, watchedFile: nonExistingfile });
+  });
+
+  it('should watch changes to previously missing dependency', {
+    skip: !supportsRecursive
+  }, async () => {
+    const dependency = path.join(tmpdir.path, `${tmpFiles++}.js`);
+    const relativeDependencyPath = `./${path.basename(dependency)}`;
+    const dependant = createTmpFile(`console.log(require('${relativeDependencyPath}'))`);
+
+    await failWriteSucceed({ file: dependant, watchedFile: dependency });
+  });
+
+  it('should watch changes to previously missing ESM dependency', {
+    skip: !supportsRecursive
+  }, async () => {
+    const dependency = path.join(tmpdir.path, `${tmpFiles++}.mjs`);
+    const relativeDependencyPath = `./${path.basename(dependency)}`;
+    const dependant = createTmpFile(`import '${relativeDependencyPath}'`, '.mjs');
+
+    await failWriteSucceed({ file: dependant, watchedFile: dependency });
+  });
 });