Skip to content

Commit

Permalink
Consistently perform bootstrap and encode Brotli config for improved …
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
devversion committed Jul 14, 2022
1 parent 0e0da59 commit 6f0ee70
Show file tree
Hide file tree
Showing 36 changed files with 470 additions and 217 deletions.
233 changes: 152 additions & 81 deletions src/bin.ts

Large diffs are not rendered by default.

19 changes: 3 additions & 16 deletions src/child/child-entrypoint.ts
Original file line number Diff line number Diff line change
@@ -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);
40 changes: 40 additions & 0 deletions src/child/child-exec-args.ts
Original file line number Diff line number Diff line change
@@ -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'),
};
}
43 changes: 43 additions & 0 deletions src/child/spawn-child-with-esm.ts
Original file line number Diff line number Diff line change
@@ -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);
}
}
52 changes: 0 additions & 52 deletions src/child/spawn-child.ts

This file was deleted.

52 changes: 37 additions & 15 deletions src/test/esm-loader.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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) => {
Expand Down
4 changes: 4 additions & 0 deletions src/test/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down
75 changes: 51 additions & 24 deletions src/test/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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`;
Expand Down Expand Up @@ -1132,35 +1130,64 @@ 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(test, `node --no-warnings ${BIN_PATH_JS}`, BIN_PATH_JS, [
'--no-warnings',
]);
forkTest(
test,
`${BIN_PATH}`,
process.platform === 'win32' ? BIN_PATH_JS : BIN_PATH
);

test.suite('with esm enabled', (test) => {
test.runIf(nodeSupportsSpawningChildProcess);

forkTest(
test,
`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(
testFn: typeof test,
command: string,
expectParentArgv0: string,
nodeFlag?: string
nodeFlags?: (string | ReturnType<typeof expect.stringMatching>)[],
testFixturePath = './recursive-fork/index.ts'
) {
test(command, async (t) => {
testFn(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);
});
Expand Down
Loading

0 comments on commit 6f0ee70

Please sign in to comment.