Skip to content

Commit 61e47c5

Browse files
committed
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.
1 parent bf13086 commit 61e47c5

File tree

9 files changed

+174
-33
lines changed

9 files changed

+174
-33
lines changed

src/bin.ts

Lines changed: 90 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ export function main(
4848
const state: BootstrapState = {
4949
shouldUseChildProcess: false,
5050
isInChildProcess: false,
51-
entrypoint: __filename,
51+
tsNodeScript: __filename,
5252
parseArgvResult: args,
5353
};
5454
return bootstrap(state);
@@ -62,7 +62,7 @@ export function main(
6262
export interface BootstrapState {
6363
isInChildProcess: boolean;
6464
shouldUseChildProcess: boolean;
65-
entrypoint: string;
65+
tsNodeScript: string;
6666
parseArgvResult: ReturnType<typeof parseArgv>;
6767
phase2Result?: ReturnType<typeof phase2>;
6868
phase3Result?: ReturnType<typeof phase3>;
@@ -319,28 +319,16 @@ Options:
319319
process.exit(0);
320320
}
321321

322-
// Figure out which we are executing: piped stdin, --eval, REPL, and/or entrypoint
323-
// This is complicated because node's behavior is complicated
324-
// `node -e code -i ./script.js` ignores -e
325-
const executeEval = code != null && !(interactive && restArgs.length);
326-
const executeEntrypoint = !executeEval && restArgs.length > 0;
327-
const executeRepl =
328-
!executeEntrypoint &&
329-
(interactive || (process.stdin.isTTY && !executeEval));
330-
const executeStdin = !executeEval && !executeRepl && !executeEntrypoint;
331-
332322
const cwd = cwdArg || process.cwd();
333-
/** Unresolved. May point to a symlink, not realpath. May be missing file extension */
334-
const scriptPath = executeEntrypoint ? resolve(cwd, restArgs[0]) : undefined;
335323

336-
if (esm) payload.shouldUseChildProcess = true;
324+
// If ESM is explicitly enabled through the flag, stage3 should be run in a child process
325+
// with the ESM loaders configured.
326+
if (esm) {
327+
payload.shouldUseChildProcess = true;
328+
}
329+
337330
return {
338-
executeEval,
339-
executeEntrypoint,
340-
executeRepl,
341-
executeStdin,
342331
cwd,
343-
scriptPath,
344332
};
345333
}
346334

@@ -372,7 +360,18 @@ function phase3(payload: BootstrapState) {
372360
esm,
373361
experimentalSpecifierResolution,
374362
} = payload.parseArgvResult;
375-
const { cwd, scriptPath } = payload.phase2Result!;
363+
const { cwd } = payload.phase2Result!;
364+
365+
// NOTE: When we transition to a child process for ESM, the entry-point script determined
366+
// here might not be the one used later in `phase4`. This can happen when we execute the
367+
// original entry-point but then the process forks itself using e.g. `child_process.fork`.
368+
// We will always use the original TS project in forked processes anyway, so it is
369+
// expected and acceptable to retrieve the entry-point information here in `phase2`.
370+
// See: https://github.com/TypeStrong/ts-node/issues/1812.
371+
const { entryPointPath } = getEntryPointInfo(
372+
payload.parseArgvResult!,
373+
payload.phase2Result!
374+
);
376375

377376
const preloadedConfig = findAndReadConfig({
378377
cwd,
@@ -387,7 +386,12 @@ function phase3(payload: BootstrapState) {
387386
compilerHost,
388387
ignore,
389388
logError,
390-
projectSearchDir: getProjectSearchDir(cwd, scriptMode, cwdMode, scriptPath),
389+
projectSearchDir: getProjectSearchDir(
390+
cwd,
391+
scriptMode,
392+
cwdMode,
393+
entryPointPath
394+
),
391395
project,
392396
skipProject,
393397
skipIgnore,
@@ -403,23 +407,76 @@ function phase3(payload: BootstrapState) {
403407
experimentalSpecifierResolution as ExperimentalSpecifierResolution,
404408
});
405409

406-
if (preloadedConfig.options.esm) payload.shouldUseChildProcess = true;
410+
// If ESM is enabled through the parsed tsconfig, stage4 should be run in a child
411+
// process with the ESM loaders configured.
412+
if (preloadedConfig.options.esm) {
413+
payload.shouldUseChildProcess = true;
414+
}
415+
407416
return { preloadedConfig };
408417
}
409418

419+
/**
420+
* Determines the entry-point information from the argv and phase2 result. This
421+
* method will be invoked in two places:
422+
*
423+
* 1. In phase 3 to be able to find a project from the potential entry-point script.
424+
* 2. In phase 4 to determine the actual entry-point script.
425+
*
426+
* Note that we need to explicitly re-resolve the entry-point information in the final
427+
* stage because the previous stage information could be modified when the bootstrap
428+
* invocation transitioned into a child process for ESM.
429+
*
430+
* Stages before (phase 4) can and will be cached by the child process through the Brotli
431+
* configuration and entry-point information is only reliable in the final phase. More
432+
* details can be found in here: https://github.com/TypeStrong/ts-node/issues/1812.
433+
*/
434+
function getEntryPointInfo(
435+
argvResult: NonNullable<BootstrapState['parseArgvResult']>,
436+
phase2Result: NonNullable<BootstrapState['phase2Result']>
437+
) {
438+
const { code, interactive, restArgs } = argvResult;
439+
const { cwd } = phase2Result;
440+
441+
// Figure out which we are executing: piped stdin, --eval, REPL, and/or entrypoint
442+
// This is complicated because node's behavior is complicated
443+
// `node -e code -i ./script.js` ignores -e
444+
const executeEval = code != null && !(interactive && restArgs.length);
445+
const executeEntrypoint = !executeEval && restArgs.length > 0;
446+
const executeRepl =
447+
!executeEntrypoint &&
448+
(interactive || (process.stdin.isTTY && !executeEval));
449+
const executeStdin = !executeEval && !executeRepl && !executeEntrypoint;
450+
451+
/** Unresolved. May point to a symlink, not realpath. May be missing file extension */
452+
const entryPointPath = executeEntrypoint
453+
? resolve(cwd, restArgs[0])
454+
: undefined;
455+
456+
return {
457+
executeEval,
458+
executeEntrypoint,
459+
executeRepl,
460+
executeStdin,
461+
entryPointPath,
462+
};
463+
}
464+
410465
function phase4(payload: BootstrapState) {
411-
const { isInChildProcess, entrypoint } = payload;
466+
const { isInChildProcess, tsNodeScript } = payload;
412467
const { version, showConfig, restArgs, code, print, argv } =
413468
payload.parseArgvResult;
469+
const { cwd } = payload.phase2Result!;
470+
const { preloadedConfig } = payload.phase3Result!;
471+
414472
const {
473+
entryPointPath,
474+
executeEntrypoint,
415475
executeEval,
416-
cwd,
417-
executeStdin,
418476
executeRepl,
419-
executeEntrypoint,
420-
scriptPath,
421-
} = payload.phase2Result!;
422-
const { preloadedConfig } = payload.phase3Result!;
477+
executeStdin,
478+
} = getEntryPointInfo(payload.parseArgvResult!, payload.phase2Result!);
479+
423480
/**
424481
* <repl>, [stdin], and [eval] are all essentially virtual files that do not exist on disc and are backed by a REPL
425482
* service to handle eval-ing of code.
@@ -566,12 +623,13 @@ function phase4(payload: BootstrapState) {
566623

567624
// Prepend `ts-node` arguments to CLI for child processes.
568625
process.execArgv.push(
569-
entrypoint,
626+
tsNodeScript,
570627
...argv.slice(2, argv.length - restArgs.length)
571628
);
629+
572630
// TODO this comes from BoostrapState
573631
process.argv = [process.argv[1]]
574-
.concat(executeEntrypoint ? ([scriptPath] as string[]) : [])
632+
.concat(executeEntrypoint ? ([entryPointPath] as string[]) : [])
575633
.concat(restArgs.slice(executeEntrypoint ? 1 : 0));
576634

577635
// Execute the main contents (either eval, script or piped).

src/child/child-entrypoint.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,9 @@ const base64Payload = base64ConfigArg.slice(argPrefix.length);
88
const payload = JSON.parse(
99
brotliDecompressSync(Buffer.from(base64Payload, 'base64')).toString()
1010
) as BootstrapState;
11+
1112
payload.isInChildProcess = true;
12-
payload.entrypoint = __filename;
13+
payload.tsNodeScript = __filename;
1314
payload.parseArgvResult.argv = process.argv;
1415
payload.parseArgvResult.restArgs = process.argv.slice(3);
1516

src/test/esm-loader.spec.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,28 @@ test.suite('esm', (test) => {
358358
});
359359
}
360360

361+
test.suite('esm child process and forking', (test) => {
362+
test('should be able to fork vanilla NodeJS script', async () => {
363+
const { err, stdout, stderr } = await exec(
364+
`${BIN_PATH} --esm ./esm-child-process/process-forking/index.mts`
365+
);
366+
367+
expect(err).toBe(null);
368+
expect(stdout.trim()).toBe('Passing: from main');
369+
expect(stderr).toBe('');
370+
});
371+
372+
test('should be able to fork into a nested TypeScript ESM script', async () => {
373+
const { err, stdout, stderr } = await exec(
374+
`${BIN_PATH} --esm ./esm-child-process/process-forking-nested-esm/index.mts`
375+
);
376+
377+
expect(err).toBe(null);
378+
expect(stdout.trim()).toBe('Passing: from main');
379+
expect(stderr).toBe('');
380+
});
381+
});
382+
361383
test.suite('parent passes signals to child', (test) => {
362384
test.runSerially();
363385

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import { fork } from 'child_process';
2+
import { dirname, join } from 'path';
3+
import { fileURLToPath } from 'url';
4+
5+
// Initially set the exit code to non-zero. We only set it to `0` when the
6+
// worker process finishes properly with the expected stdout message.
7+
process.exitCode = 1;
8+
9+
const projectDir = dirname(fileURLToPath(import.meta.url));
10+
const workerProcess = fork(join(projectDir, 'worker.mts'), [], {
11+
stdio: 'pipe',
12+
});
13+
14+
let stdout = '';
15+
16+
workerProcess.stdout.on('data', (chunk) => (stdout += chunk.toString('utf8')));
17+
workerProcess.on('error', () => (process.exitCode = 1));
18+
workerProcess.on('close', (status, signal) => {
19+
if (status === 0 && signal === null && stdout.trim() === 'Works') {
20+
console.log('Passing: from main');
21+
process.exitCode = 0;
22+
}
23+
});
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"compilerOptions": {
3+
"module": "Node16"
4+
}
5+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
const message: string = 'Works';
2+
3+
console.log(message);
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import { fork } from 'child_process';
2+
import { dirname, join } from 'path';
3+
import { fileURLToPath } from 'url';
4+
5+
// Initially set the exit code to non-zero. We only set it to `0` when the
6+
// worker process finishes properly with the expected stdout message.
7+
process.exitCode = 1;
8+
9+
const projectDir = dirname(fileURLToPath(import.meta.url));
10+
const workerProcess = fork(join(projectDir, 'worker.mjs'), [], {
11+
stdio: 'pipe',
12+
});
13+
14+
let stdout = '';
15+
16+
workerProcess.stdout.on('data', (chunk) => (stdout += chunk.toString('utf8')));
17+
workerProcess.on('error', () => (process.exitCode = 1));
18+
workerProcess.on('close', (status, signal) => {
19+
if (status === 0 && signal === null && stdout.trim() === 'Works') {
20+
console.log('Passing: from main');
21+
process.exitCode = 0;
22+
}
23+
});
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"compilerOptions": {
3+
"module": "Node16"
4+
}
5+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
console.log('Works');

0 commit comments

Comments
 (0)