Consistently perform bootstrap and encode Brotli config for improved …

…caching/reduced complexity Additionally, this PR streamlines the boostrap mechanism to always call into the child script, resulting in reduced complexity, and also improved caching for user-initiated forked processes. i.e. the tsconfig resolution is not repeated multiple-times because forked processes are expected to preserve the existing ts-node project. More details can be found here TypeStrong#1831. Fixes TypeStrong#1812.
devversion · Jul 14, 2022 · a7e6486 · a7e6486
1 parent 0e0da59
commit a7e6486
Show file tree

Hide file tree

Showing 36 changed files with 467 additions and 217 deletions.
diff --git a/src/bin.ts b/src/bin.ts
diff --git a/src/child/child-entrypoint.ts b/src/child/child-entrypoint.ts
@@ -1,24 +1,11 @@
-import { BootstrapState, bootstrap } from '../bin';
+import { completeBootstrap, BootstrapStateForChild } from '../bin';
 import { argPrefix, compress, decompress } from './argv-payload';
 
 const base64ConfigArg = process.argv[2];
 if (!base64ConfigArg.startsWith(argPrefix)) throw new Error('unexpected argv');
 const base64Payload = base64ConfigArg.slice(argPrefix.length);
-const state = decompress(base64Payload) as BootstrapState;
+const state = decompress(base64Payload) as BootstrapStateForChild;
 
-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);
+completeBootstrap(state);
diff --git a/src/child/child-exec-args.ts b/src/child/child-exec-args.ts
@@ -0,0 +1,40 @@
+import { pathToFileURL } from 'url';
+import { brotliCompressSync } from 'zlib';
+import type { BootstrapStateForChild } from '../bin';
+import { versionGteLt } from '../util';
+import { argPrefix } from './argv-payload';
+
+export function getChildProcessArguments(
+  enableEsmLoader: boolean,
+  state: BootstrapStateForChild
+) {
+  if (enableEsmLoader && !versionGteLt(process.versions.node, '12.17.0')) {
+    throw new Error(
+      '`ts-node-esm` and `ts-node --esm` require node version 12.17.0 or newer.'
+    );
+  }
+
+  const nodeExecArgs = [];
+
+  if (enableEsmLoader) {
+    nodeExecArgs.push(
+      '--require',
+      require.resolve('./child-require.js'),
+      '--loader',
+      // Node on Windows doesn't like `c:\` absolute paths here; must be `file:///c:/`
+      pathToFileURL(require.resolve('../../child-loader.mjs')).toString()
+    );
+  }
+
+  const childScriptArgs = [
+    `${argPrefix}${brotliCompressSync(
+      Buffer.from(JSON.stringify(state), 'utf8')
+    ).toString('base64')}`,
+  ];
+
+  return {
+    nodeExecArgs,
+    childScriptArgs,
+    childScriptPath: require.resolve('./child-entrypoint.js'),
+  };
+}
diff --git a/src/child/spawn-child-with-esm.ts b/src/child/spawn-child-with-esm.ts
@@ -0,0 +1,43 @@
+import { fork } from 'child_process';
+import type { BootstrapStateForChild } from '../bin';
+import { getChildProcessArguments } from './child-exec-args';
+
+/**
+ * @internal
+ * @param state Bootstrap state to be transferred into the child process.
+ * @param enableEsmLoader Whether to enable the ESM loader or not. This option may
+ *   be removed in the future when `--esm` is no longer a choice.
+ * @param targetCwd Working directory to be preserved when transitioning to
+ *   the child process.
+ */
+export function callInChildWithEsm(
+  state: BootstrapStateForChild,
+  targetCwd: string
+) {
+  const { childScriptArgs, childScriptPath, nodeExecArgs } =
+    getChildProcessArguments(/* enableEsmLoader */ true, state);
+
+  childScriptArgs.push(...state.parseArgvResult.restArgs);
+
+  const child = fork(childScriptPath, childScriptArgs, {
+    stdio: 'inherit',
+    execArgv: [...process.execArgv, ...nodeExecArgs],
+    cwd: targetCwd,
+  });
+  child.on('error', (error) => {
+    console.error(error);
+    process.exit(1);
+  });
+  child.on('exit', (code) => {
+    child.removeAllListeners();
+    process.off('SIGINT', sendSignalToChild);
+    process.off('SIGTERM', sendSignalToChild);
+    process.exitCode = code === null ? 1 : code;
+  });
+  // Ignore sigint and sigterm in parent; pass them to child
+  process.on('SIGINT', sendSignalToChild);
+  process.on('SIGTERM', sendSignalToChild);
+  function sendSignalToChild(signal: string) {
+    process.kill(child.pid, signal);
+  }
+}
diff --git a/src/child/spawn-child.ts b/src/child/spawn-child.ts
diff --git a/src/test/esm-loader.spec.ts b/src/test/esm-loader.spec.ts
@@ -362,10 +362,8 @@ 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'),
-            }
+            `${BIN_PATH} --esm index.ts`,
+            { cwd: './working-dir/esm/' }
           );
 
           expect(err).toBe(null);
@@ -377,33 +375,57 @@ test.suite('esm', (test) => {
       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`
+            `${BIN_PATH} --esm index.ts`,
+            { cwd: './esm-child-process/process-forking-js-worker/' }
           );
 
           expect(err).toBe(null);
           expect(stdout.trim()).toBe('Passing: from main');
           expect(stderr).toBe('');
         });
 
-        test('should be able to fork TypeScript script', async () => {
+        test('should be able to fork into a nested TypeScript ESM script', async () => {
           const { err, stdout, stderr } = await exec(
-            `${BIN_PATH} --esm --cwd ./esm-child-process/ ./process-forking-ts/index.ts`
+            `${BIN_PATH} --esm index.ts`,
+            { cwd: './esm-child-process/process-forking-esm-worker/' }
           );
 
           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`
-          );
+        test(
+          'should be possible to fork into a nested TypeScript script with respect to ' +
+            'the working directory',
+          async () => {
+            const { err, stdout, stderr } = await exec(
+              `${BIN_PATH} --esm index.ts`,
+              { cwd: './esm-child-process/process-forking-subdir-relative/' }
+            );
 
-          expect(err).toBe(null);
-          expect(stdout.trim()).toBe('Passing: from main');
-          expect(stderr).toBe('');
-        });
+            expect(err).toBe(null);
+            expect(stdout.trim()).toBe('Passing: from main');
+            expect(stderr).toBe('');
+          }
+        );
+
+        test.suite(
+          'with NodeNext TypeScript resolution and `.mts` extension',
+          (test) => {
+            test.runIf(tsSupportsStableNodeNextNode16);
+
+            test('should be able to fork into a nested TypeScript ESM script', async () => {
+              const { err, stdout, stderr } = await exec(
+                `${BIN_PATH} --esm ./esm-child-process/process-forking-esm-worker-next/index.mts`
+              );
+
+              expect(err).toBe(null);
+              expect(stdout.trim()).toBe('Passing: from main');
+              expect(stderr).toBe('');
+            });
+          }
+        );
       });
 
       test.suite('parent passes signals to child', (test) => {

diff --git a/src/test/helpers.ts b/src/test/helpers.ts
@@ -33,6 +33,10 @@ export const BIN_SCRIPT_PATH = join(
   TEST_DIR,
   'node_modules/.bin/ts-node-script'
 );
+export const CHILD_ENTRY_POINT_SCRIPT = join(
+  TEST_DIR,
+  'node_modules/ts-node/dist/child/child-entrypoint.js'
+);
 export const BIN_CWD_PATH = join(TEST_DIR, 'node_modules/.bin/ts-node-cwd');
 export const BIN_ESM_PATH = join(TEST_DIR, 'node_modules/.bin/ts-node-esm');
 

diff --git a/src/test/index.spec.ts b/src/test/index.spec.ts
@@ -5,6 +5,7 @@ import { tmpdir } from 'os';
 import semver = require('semver');
 import {
   BIN_PATH_JS,
+  CHILD_ENTRY_POINT_SCRIPT,
   CMD_TS_NODE_WITH_PROJECT_TRANSPILE_ONLY_FLAG,
   nodeSupportsEsmHooks,
   nodeSupportsSpawningChildProcess,
@@ -619,30 +620,27 @@ 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'),
-        }
+        `${BIN_PATH} --cwd ./working-dir/cjs/ index.ts`
       );
 
       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`
-      );
+    test(
+      '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('');
-    });
+        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`;
@@ -1132,35 +1130,61 @@ test('Falls back to transpileOnly when ts compiler returns emitSkipped', async (
 
 test.suite('node environment', (test) => {
   test.suite('Sets argv and execArgv correctly in forked processes', (test) => {
-    forkTest(`node --no-warnings ${BIN_PATH_JS}`, BIN_PATH_JS, '--no-warnings');
+    forkTest(`node --no-warnings ${BIN_PATH_JS}`, BIN_PATH_JS, [
+      '--no-warnings',
+    ]);
     forkTest(
       `${BIN_PATH}`,
       process.platform === 'win32' ? BIN_PATH_JS : BIN_PATH
     );
 
+    test.suite('with esm enabled', (test) => {
+      test.runIf(nodeSupportsSpawningChildProcess);
+
+      forkTest(
+        `node --no-warnings ${BIN_PATH_JS} --esm`,
+        CHILD_ENTRY_POINT_SCRIPT,
+        [
+          '--no-warnings',
+          // Forked child processes should directly receive the necessary ESM loader
+          // Node flags through `execArgv`, avoiding doubled child process spawns.
+          '--require',
+          expect.stringMatching(/child-require.js$/),
+          '--loader',
+          expect.stringMatching(/child-loader.mjs$/),
+        ],
+        './recursive-fork-esm/index.ts'
+      );
+    });
+
     function forkTest(
       command: string,
       expectParentArgv0: string,
-      nodeFlag?: string
+      nodeFlags?: (string | ReturnType<typeof expect.stringMatching>)[],
+      testFixturePath = './recursive-fork/index.ts'
     ) {
       test(command, async (t) => {
         const { err, stderr, stdout } = await exec(
-          `${command} --skipIgnore ./recursive-fork/index.ts argv2`
+          `${command} --skipIgnore ${testFixturePath} argv2`
         );
         expect(err).toBeNull();
         expect(stderr).toBe('');
         const generations = stdout.split('\n');
         const expectation = {
-          execArgv: [nodeFlag, BIN_PATH_JS, '--skipIgnore'].filter((v) => v),
+          execArgv: [
+            ...(nodeFlags || []),
+            CHILD_ENTRY_POINT_SCRIPT,
+            expect.stringMatching(/^--brotli-base64-config=.*/),
+          ],
           argv: [
-            // Note: argv[0] is *always* BIN_PATH_JS in child & grandchild
+            // Note: argv[0] is *always* CHILD_ENTRY_POINT_SCRIPT in child & grandchild
             expectParentArgv0,
-            resolve(TEST_DIR, 'recursive-fork/index.ts'),
+            resolve(TEST_DIR, testFixturePath),
             'argv2',
           ],
         };
         expect(JSON.parse(generations[0])).toMatchObject(expectation);
-        expectation.argv[0] = BIN_PATH_JS;
+        expectation.argv[0] = CHILD_ENTRY_POINT_SCRIPT;
         expect(JSON.parse(generations[1])).toMatchObject(expectation);
         expect(JSON.parse(generations[2])).toMatchObject(expectation);
       });

diff --git a/...d-process/process-forking-ts-abs/index.ts → ...process-forking-esm-worker-next/index.mts b/...d-process/process-forking-ts-abs/index.ts → ...process-forking-esm-worker-next/index.mts
@@ -6,13 +6,10 @@ import { fileURLToPath } from 'url';
 // worker process finishes properly with the expected stdout message.
 process.exitCode = 1;
 
-const workerProcess = fork(
-  join(dirname(fileURLToPath(import.meta.url)), 'subfolder/worker.ts'),
-  [],
-  {
-    stdio: 'pipe',
-  }
-);
+const containingDir = dirname(fileURLToPath(import.meta.url));
+const workerProcess = fork(join(containingDir, 'worker.mts'), [], {
+  stdio: 'pipe',
+});
 
 let stdout = '';