esm: ensure watch mode restarts after syntax errors

Xstoudi · aduh95 · commit 13af640d9447 · 2026-01-27T22:50:38.000+01:00
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>
diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
@@ -548,6 +548,12 @@ class ModuleLoader {
    */
   #getOrCreateModuleJobAfterResolve(parentURL, resolveResult, request, requestType) {
     const { url, format } = resolveResult;
+
+    if (process.env.WATCH_REPORT_DEPENDENCIES && process.send) {
+      const type = requestType === kRequireInImportedCJS ? 'require' : 'import';
+      process.send({ [`watch:${type}`]: [url] });
+    }
+
     // TODO(joyeecheung): update the module requests to use importAttributes as property names.
     const importAttributes = resolveResult.importAttributes ?? request.attributes;
     let job = this.loadCache.get(url, importAttributes.type);
@@ -576,11 +582,6 @@ class ModuleLoader {
       assert(moduleOrModulePromise instanceof ModuleWrap, `Expected ModuleWrap for loading ${url}`);
     }
 
-    if (process.env.WATCH_REPORT_DEPENDENCIES && process.send) {
-      const type = requestType === kRequireInImportedCJS ? 'require' : 'import';
-      process.send({ [`watch:${type}`]: [url] });
-    }
-
     const { ModuleJob, ModuleJobSync } = require('internal/modules/esm/module_job');
     // TODO(joyeecheung): use ModuleJobSync for kRequireInImportedCJS too.
     const ModuleJobCtor = (requestType === kImportInRequiredESM ? ModuleJobSync : ModuleJob);
diff --git a/test/sequential/test-watch-mode-restart-esm-loading-error.mjs b/test/sequential/test-watch-mode-restart-esm-loading-error.mjs
@@ -0,0 +1,126 @@
+import * as common from '../common/index.mjs';
+import tmpdir from '../common/tmpdir.js';
+import assert from 'node:assert';
+import path from 'node:path';
+import { execPath } from 'node:process';
+import { spawn } from 'node:child_process';
+import { writeFileSync } from 'node:fs';
+import { inspect } from 'node:util';
+import { createInterface } from 'node:readline';
+
+if (common.isIBMi)
+  common.skip('IBMi does not support `fs.watch()`');
+
+let tmpFiles = 0;
+function createTmpFile(content = 'console.log("running");', ext = '.js', basename = tmpdir.path) {
+  const file = path.join(basename, `${tmpFiles++}${ext}`);
+  writeFileSync(file, content);
+  return file;
+}
+
+function runInBackground({ args = [], options = {}, completed = 'Completed running', shouldFail = false }) {
+  let future = Promise.withResolvers();
+  let child;
+  let stderr = '';
+  let stdout = [];
+
+  const run = () => {
+    args.unshift('--no-warnings');
+    child = spawn(execPath, args, { encoding: 'utf8', stdio: 'pipe', ...options });
+
+    child.stderr.on('data', (data) => {
+      stderr += data;
+    });
+
+    const rl = createInterface({ input: child.stdout });
+    rl.on('line', (data) => {
+      if (!data.startsWith('Waiting for graceful termination') && !data.startsWith('Gracefully restarted')) {
+        stdout.push(data);
+        if (data.startsWith(completed)) {
+          future.resolve({ stderr, stdout });
+          future = Promise.withResolvers();
+          stdout = [];
+          stderr = '';
+        } else if (data.startsWith('Failed running')) {
+          if (shouldFail) {
+            future.resolve({ stderr, stdout });
+          } else {
+            future.reject({ stderr, stdout });
+          }
+          future = Promise.withResolvers();
+          stdout = [];
+          stderr = '';
+        }
+      }
+    });
+  };
+
+  return {
+    async done() {
+      child?.kill();
+      future.resolve();
+      return { stdout, stderr };
+    },
+    restart(timeout = 1000) {
+      if (!child) {
+        run();
+      }
+      const timer = setTimeout(() => {
+        if (!future.resolved) {
+          child.kill();
+          future.reject(new Error('Timed out waiting for restart'));
+        }
+      }, timeout);
+      return future.promise.finally(() => {
+        clearTimeout(timer);
+      });
+    },
+  };
+}
+
+tmpdir.refresh();
+
+// Create initial file with valid code
+const initialContent = `console.log('hello, world');`;
+const file = createTmpFile(initialContent, '.mjs');
+
+const { done, restart } = runInBackground({
+  args: ['--watch', file],
+  completed: 'Completed running',
+  shouldFail: true,
+});
+
+try {
+  const { stdout, stderr } = await restart();
+  assert.strictEqual(stderr, '');
+  assert.deepStrictEqual(stdout, [
+    'hello, world',
+    `Completed running ${inspect(file)}. Waiting for file changes before restarting...`,
+  ]);
+
+  // Update file with syntax error
+  const syntaxErrorContent = `console.log('hello, wor`;
+  writeFileSync(file, syntaxErrorContent);
+
+  // Wait for the failed restart
+  const { stderr: stderr2, stdout: stdout2 } = await restart();
+  assert.match(stderr2, /SyntaxError: Invalid or unexpected token/);
+  assert.deepStrictEqual(stdout2, [
+    `Restarting ${inspect(file)}`,
+    `Failed running ${inspect(file)}. Waiting for file changes before restarting...`,
+  ]);
+
+  writeFileSync(file, `console.log('hello again, world');`);
+
+  const { stderr: stderr3, stdout: stdout3 } = await restart();
+
+  // Verify it recovered and ran successfully
+  assert.strictEqual(stderr3, '');
+  assert.deepStrictEqual(stdout3, [
+    `Restarting ${inspect(file)}`,
+    'hello again, world',
+    `Completed running ${inspect(file)}. Waiting for file changes before restarting...`,
+  ]);
+} finally {
+  await done();
+}
