Fix ESM node processes being unable to fork into other scripts (#1814)

* Fix ESM node processes being unable to fork into other scripts

Currently, Node processes instantiated through the `--esm` flag result
in a child process being created so that the ESM loader can be
registered. This works fine and is reasonable.

The child process approach to register ESM hooks currently prevents
the NodeJS `fork` method from being used because the `execArgv`
propagated into forked processes causes `ts-node` (which is also
propagated as child exec script -- this is good because it allows nested
type resolution to work) to always execute the original entry-point,
causing potential infinite loops because the designated fork module
script is not executed as expected.

This commit fixes this by not encoding the entry-point information into
the state that is captured as part of the `execArgv`. Instead the
entry-point information is always retrieved from the parsed rest command
line arguments in the final stage (`phase4`).

Fixes #1812.

* Fix `--cwd` to actually set the working directory and work with ESM child process

Currently the `--esm` option does not necessarily do what the
documentation suggests. i.e. the script does not run as if the working
directory is the specified directory.

This commit fixes this, so that the option is useful for TSConfig
resolution, as well as for controlling the script working directory.

Also fixes that the CWD encoded in the bootstrap brotli state for the
ESM child process messes with the entry-point resolution, if e.g. the
entry-point in `child_process.fork` is relative to a specified `cwd`.

* changes based on review

* lint-fix

* enable transpileOnly in new tests for performance

* Tweak basic working dir tests to verify that --cwd affects entrypoint resolution but not process.cwd()

* update forking tests:

disable non --esm test with comment about known bug and link to tickets
make tests set cwd for fork() call, to be sure it is respected and not overridden by --cwd

* use swc compiler to avoid issue with ancient TS versions not understanding import.meta.url syntax

* Remove tests that I think are redundant (but I've asked for confirmation in code review)

* fix another issue with old TS

* final review updates

Co-authored-by: Andrew Bradley <[email protected]>

Author: devversion

src/bin.ts

@@ -48,7 +48,8 @@ export function main(
   const state: BootstrapState = {
     shouldUseChildProcess: false,
     isInChildProcess: false,
-    entrypoint: __filename,
+    isCli: true,
+    tsNodeScript: __filename,
     parseArgvResult: args,
   };
   return bootstrap(state);
@@ -62,7 +63,12 @@ export function main(
 export interface BootstrapState {
   isInChildProcess: boolean;
   shouldUseChildProcess: boolean;
-  entrypoint: string;
+  /**
+   * True if bootstrapping the ts-node CLI process or the direct child necessitated by `--esm`.
+   * false if bootstrapping a subsequently `fork()`ed child.
+   */
+  isCli: boolean;
+  tsNodeScript: string;
   parseArgvResult: ReturnType<typeof parseArgv>;
   phase2Result?: ReturnType<typeof phase2>;
   phase3Result?: ReturnType<typeof phase3>;
@@ -73,12 +79,16 @@ export function bootstrap(state: BootstrapState) {
   if (!state.phase2Result) {
     state.phase2Result = phase2(state);
     if (state.shouldUseChildProcess && !state.isInChildProcess) {
+      // Note: When transitioning into the child-process after `phase2`,
+      // the updated working directory needs to be preserved.
       return callInChild(state);
     }
   }
   if (!state.phase3Result) {
     state.phase3Result = phase3(state);
     if (state.shouldUseChildProcess && !state.isInChildProcess) {
+      // Note: When transitioning into the child-process after `phase2`,
+      // the updated working directory needs to be preserved.
       return callInChild(state);
     }
   }
@@ -264,8 +274,7 @@ function parseArgv(argv: string[], entrypointArgs: Record<string, any>) {
 }
 
 function phase2(payload: BootstrapState) {
-  const { help, version, code, interactive, cwdArg, restArgs, esm } =
-    payload.parseArgvResult;
+  const { help, version, cwdArg, esm } = payload.parseArgvResult;
 
   if (help) {
     console.log(`
@@ -319,28 +328,14 @@ Options:
     process.exit(0);
   }
 
-  // Figure out which we are executing: piped stdin, --eval, REPL, and/or entrypoint
-  // This is complicated because node's behavior is complicated
-  // `node -e code -i ./script.js` ignores -e
-  const executeEval = code != null && !(interactive && restArgs.length);
-  const executeEntrypoint = !executeEval && restArgs.length > 0;
-  const executeRepl =
-    !executeEntrypoint &&
-    (interactive || (process.stdin.isTTY && !executeEval));
-  const executeStdin = !executeEval && !executeRepl && !executeEntrypoint;
-
-  const cwd = cwdArg || process.cwd();
-  /** Unresolved.  May point to a symlink, not realpath.  May be missing file extension */
-  const scriptPath = executeEntrypoint ? resolve(cwd, restArgs[0]) : undefined;
+  const cwd = cwdArg ? resolve(cwdArg) : process.cwd();
 
+  // If ESM is explicitly enabled through the flag, stage3 should be run in a child process
+  // with the ESM loaders configured.
   if (esm) payload.shouldUseChildProcess = true;
+
   return {
-    executeEval,
-    executeEntrypoint,
-    executeRepl,
-    executeStdin,
     cwd,
-    scriptPath,
   };
 }
 
@@ -372,7 +367,15 @@ function phase3(payload: BootstrapState) {
     esm,
     experimentalSpecifierResolution,
   } = payload.parseArgvResult;
-  const { cwd, scriptPath } = payload.phase2Result!;
+  const { cwd } = payload.phase2Result!;
+
+  // NOTE: When we transition to a child process for ESM, the entry-point script determined
+  // here might not be the one used later in `phase4`. This can happen when we execute the
+  // original entry-point but then the process forks itself using e.g. `child_process.fork`.
+  // We will always use the original TS project in forked processes anyway, so it is
+  // expected and acceptable to retrieve the entry-point information here in `phase2`.
+  // See: https://github.com/TypeStrong/ts-node/issues/1812.
+  const { entryPointPath } = getEntryPointInfo(payload);
 
   const preloadedConfig = findAndReadConfig({
     cwd,
@@ -387,7 +390,12 @@ function phase3(payload: BootstrapState) {
     compilerHost,
     ignore,
     logError,
-    projectSearchDir: getProjectSearchDir(cwd, scriptMode, cwdMode, scriptPath),
+    projectSearchDir: getProjectSearchDir(
+      cwd,
+      scriptMode,
+      cwdMode,
+      entryPointPath
+    ),
     project,
     skipProject,
     skipIgnore,
@@ -403,23 +411,77 @@ function phase3(payload: BootstrapState) {
       experimentalSpecifierResolution as ExperimentalSpecifierResolution,
   });
 
+  // If ESM is enabled through the parsed tsconfig, stage4 should be run in a child
+  // process with the ESM loaders configured.
   if (preloadedConfig.options.esm) payload.shouldUseChildProcess = true;
+
   return { preloadedConfig };
 }
 
+/**
+ * Determines the entry-point information from the argv and phase2 result. This
+ * method will be invoked in two places:
+ *
+ *   1. In phase 3 to be able to find a project from the potential entry-point script.
+ *   2. In phase 4 to determine the actual entry-point script.
+ *
+ * Note that we need to explicitly re-resolve the entry-point information in the final
+ * stage because the previous stage information could be modified when the bootstrap
+ * invocation transitioned into a child process for ESM.
+ *
+ * Stages before (phase 4) can and will be cached by the child process through the Brotli
+ * configuration and entry-point information is only reliable in the final phase. More
+ * details can be found in here: https://github.com/TypeStrong/ts-node/issues/1812.
+ */
+function getEntryPointInfo(state: BootstrapState) {
+  const { code, interactive, restArgs } = state.parseArgvResult!;
+  const { cwd } = state.phase2Result!;
+  const { isCli } = state;
+
+  // Figure out which we are executing: piped stdin, --eval, REPL, and/or entrypoint
+  // This is complicated because node's behavior is complicated
+  // `node -e code -i ./script.js` ignores -e
+  const executeEval = code != null && !(interactive && restArgs.length);
+  const executeEntrypoint = !executeEval && restArgs.length > 0;
+  const executeRepl =
+    !executeEntrypoint &&
+    (interactive || (process.stdin.isTTY && !executeEval));
+  const executeStdin = !executeEval && !executeRepl && !executeEntrypoint;
+
+  /**
+   * Unresolved. May point to a symlink, not realpath. May be missing file extension
+   * NOTE: resolution relative to cwd option (not `process.cwd()`) is legacy backwards-compat; should be changed in next major: https://github.com/TypeStrong/ts-node/issues/1834
+   */
+  const entryPointPath = executeEntrypoint
+    ? isCli
+      ? resolve(cwd, restArgs[0])
+      : resolve(restArgs[0])
+    : undefined;
+
+  return {
+    executeEval,
+    executeEntrypoint,
+    executeRepl,
+    executeStdin,
+    entryPointPath,
+  };
+}
+
 function phase4(payload: BootstrapState) {
-  const { isInChildProcess, entrypoint } = payload;
+  const { isInChildProcess, tsNodeScript } = payload;
   const { version, showConfig, restArgs, code, print, argv } =
     payload.parseArgvResult;
+  const { cwd } = payload.phase2Result!;
+  const { preloadedConfig } = payload.phase3Result!;
+
   const {
+    entryPointPath,
+    executeEntrypoint,
     executeEval,
-    cwd,
-    executeStdin,
     executeRepl,
-    executeEntrypoint,
-    scriptPath,
-  } = payload.phase2Result!;
-  const { preloadedConfig } = payload.phase3Result!;
+    executeStdin,
+  } = getEntryPointInfo(payload);
+
   /**
    * <repl>, [stdin], and [eval] are all essentially virtual files that do not exist on disc and are backed by a REPL
    * service to handle eval-ing of code.
@@ -566,12 +628,13 @@ function phase4(payload: BootstrapState) {
 
   // Prepend `ts-node` arguments to CLI for child processes.
   process.execArgv.push(
-    entrypoint,
+    tsNodeScript,
     ...argv.slice(2, argv.length - restArgs.length)
   );
-  // TODO this comes from BoostrapState
+
+  // TODO this comes from BootstrapState
   process.argv = [process.argv[1]]
-    .concat(executeEntrypoint ? ([scriptPath] as string[]) : [])
+    .concat(executeEntrypoint ? ([entryPointPath] as string[]) : [])
     .concat(restArgs.slice(executeEntrypoint ? 1 : 0));
 
   // Execute the main contents (either eval, script or piped).

src/child/argv-payload.ts

@@ -0,0 +1,18 @@
+import { brotliCompressSync, brotliDecompressSync, constants } from 'zlib';
+
+/** @internal */
+export const argPrefix = '--brotli-base64-config=';
+
+/** @internal */
+export function compress(object: any) {
+  return brotliCompressSync(Buffer.from(JSON.stringify(object), 'utf8'), {
+    [constants.BROTLI_PARAM_QUALITY]: constants.BROTLI_MIN_QUALITY,
+  }).toString('base64');
+}
+
+/** @internal */
+export function decompress(str: string) {
+  return JSON.parse(
+    brotliDecompressSync(Buffer.from(str, 'base64')).toString()
+  );
+}

src/child/child-entrypoint.ts

@@ -1,16 +1,24 @@
 import { BootstrapState, bootstrap } from '../bin';
-import { brotliDecompressSync } from 'zlib';
+import { argPrefix, compress, decompress } from './argv-payload';
 
 const base64ConfigArg = process.argv[2];
-const argPrefix = '--brotli-base64-config=';
 if (!base64ConfigArg.startsWith(argPrefix)) throw new Error('unexpected argv');
 const base64Payload = base64ConfigArg.slice(argPrefix.length);
-const payload = JSON.parse(
-  brotliDecompressSync(Buffer.from(base64Payload, 'base64')).toString()
-) as BootstrapState;
-payload.isInChildProcess = true;
-payload.entrypoint = __filename;
-payload.parseArgvResult.argv = process.argv;
-payload.parseArgvResult.restArgs = process.argv.slice(3);
+const state = decompress(base64Payload) as BootstrapState;
 
-bootstrap(payload);
+state.isInChildProcess = true;
+state.tsNodeScript = __filename;
+state.parseArgvResult.argv = process.argv;
+state.parseArgvResult.restArgs = process.argv.slice(3);
+
+// Modify and re-compress the payload delivered to subsequent child processes.
+// This logic may be refactored into bin.ts by https://github.com/TypeStrong/ts-node/issues/1831
+if (state.isCli) {
+  const stateForChildren: BootstrapState = {
+    ...state,
+    isCli: false,
+  };
+  state.parseArgvResult.argv[2] = `${argPrefix}${compress(stateForChildren)}`;
+}
+
+bootstrap(state);

src/child/spawn-child.ts

@@ -1,12 +1,15 @@
 import type { BootstrapState } from '../bin';
 import { spawn } from 'child_process';
-import { brotliCompressSync } from 'zlib';
 import { pathToFileURL } from 'url';
 import { versionGteLt } from '../util';
+import { argPrefix, compress } from './argv-payload';
 
-const argPrefix = '--brotli-base64-config=';
-
-/** @internal */
+/**
+ * @internal
+ * @param state Bootstrap state to be transferred into the child process.
+ * @param targetCwd Working directory to be preserved when transitioning to
+ *   the child process.
+ */
 export function callInChild(state: BootstrapState) {
   if (!versionGteLt(process.versions.node, '12.17.0')) {
     throw new Error(
@@ -22,9 +25,7 @@ export function callInChild(state: BootstrapState) {
       // Node on Windows doesn't like `c:\` absolute paths here; must be `file:///c:/`
       pathToFileURL(require.resolve('../../child-loader.mjs')).toString(),
       require.resolve('./child-entrypoint.js'),
-      `${argPrefix}${brotliCompressSync(
-        Buffer.from(JSON.stringify(state), 'utf8')
-      ).toString('base64')}`,
+      `${argPrefix}${compress(state)}`,
       ...state.parseArgvResult.restArgs,
     ],
     {

src/test/esm-loader.spec.ts

@@ -22,6 +22,7 @@ import {
   TEST_DIR,
   tsSupportsImportAssertions,
   tsSupportsResolveJsonModule,
+  tsSupportsStableNodeNextNode16,
 } from './helpers';
 import { createExec, createSpawn, ExecReturn } from './exec-helpers';
 import { join, resolve } from 'path';
@@ -358,6 +359,53 @@ test.suite('esm', (test) => {
         });
       }
 
+      test.suite('esm child process working directory', (test) => {
+        test('should have the correct working directory in the user entry-point', async () => {
+          const { err, stdout, stderr } = await exec(
+            `${BIN_PATH} --esm --cwd ./esm/ index.ts`,
+            {
+              cwd: resolve(TEST_DIR, 'working-dir'),
+            }
+          );
+
+          expect(err).toBe(null);
+          expect(stdout.trim()).toBe('Passing');
+          expect(stderr).toBe('');
+        });
+      });
+
+      test.suite('esm child process and forking', (test) => {
+        test('should be able to fork vanilla NodeJS script', async () => {
+          const { err, stdout, stderr } = await exec(
+            `${BIN_PATH} --esm --cwd ./esm-child-process/ ./process-forking-js/index.ts`
+          );
+
+          expect(err).toBe(null);
+          expect(stdout.trim()).toBe('Passing: from main');
+          expect(stderr).toBe('');
+        });
+
+        test('should be able to fork TypeScript script', async () => {
+          const { err, stdout, stderr } = await exec(
+            `${BIN_PATH} --esm --cwd ./esm-child-process/ ./process-forking-ts/index.ts`
+          );
+
+          expect(err).toBe(null);
+          expect(stdout.trim()).toBe('Passing: from main');
+          expect(stderr).toBe('');
+        });
+
+        test('should be able to fork TypeScript script by absolute path', async () => {
+          const { err, stdout, stderr } = await exec(
+            `${BIN_PATH} --esm --cwd ./esm-child-process/ ./process-forking-ts-abs/index.ts`
+          );
+
+          expect(err).toBe(null);
+          expect(stdout.trim()).toBe('Passing: from main');
+          expect(stderr).toBe('');
+        });
+      });
+
       test.suite('parent passes signals to child', (test) => {
         test.runSerially();
 

src/test/index.spec.ts

@@ -617,6 +617,33 @@ test.suite('ts-node', (test) => {
       }
     });
 
+    test('should have the correct working directory in the user entry-point', async () => {
+      const { err, stdout, stderr } = await exec(
+        `${BIN_PATH} --cwd ./cjs index.ts`,
+        {
+          cwd: resolve(TEST_DIR, 'working-dir'),
+        }
+      );
+
+      expect(err).toBe(null);
+      expect(stdout.trim()).toBe('Passing');
+      expect(stderr).toBe('');
+    });
+
+    // Disabled due to bug:
+    // --cwd is passed to forked children when not using --esm, erroneously affects their entrypoint resolution.
+    // tracked/fixed by either https://github.com/TypeStrong/ts-node/issues/1834
+    // or https://github.com/TypeStrong/ts-node/issues/1831
+    test.skip('should be able to fork into a nested TypeScript script with a modified working directory', async () => {
+      const { err, stdout, stderr } = await exec(
+        `${BIN_PATH} --cwd ./working-dir/forking/ index.ts`
+      );
+
+      expect(err).toBe(null);
+      expect(stdout.trim()).toBe('Passing: from main');
+      expect(stderr).toBe('');
+    });
+
     test.suite('should read ts-node options from tsconfig.json', (test) => {
       const BIN_EXEC = `"${BIN_PATH}" --project tsconfig-options/tsconfig.json`;