From d78e437f21dfcaf5fe6399360e7a9fd61167c505 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Sun, 3 Jul 2022 10:37:06 +0000 Subject: [PATCH] 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`. --- src/bin.ts | 89 +++++++++++++------ src/child/spawn-child.ts | 10 ++- src/test/esm-loader.spec.ts | 43 +++++++++ src/test/index.spec.ts | 24 +++++ .../process-forking-nested-relative/index.ts | 22 +++++ .../package.json | 3 + .../subfolder/worker.ts | 3 + .../tsconfig.json | 5 ++ tests/working-dir/cjs/index.ts | 7 ++ tests/working-dir/esm-node-next/index.ts | 11 +++ tests/working-dir/esm-node-next/package.json | 3 + tests/working-dir/esm-node-next/tsconfig.json | 5 ++ tests/working-dir/esm/index.ts | 8 ++ tests/working-dir/esm/package.json | 3 + tests/working-dir/esm/tsconfig.json | 5 ++ tests/working-dir/forking/index.ts | 22 +++++ tests/working-dir/forking/subfolder/worker.ts | 3 + 17 files changed, 235 insertions(+), 31 deletions(-) create mode 100644 tests/esm-child-process/process-forking-nested-relative/index.ts create mode 100644 tests/esm-child-process/process-forking-nested-relative/package.json create mode 100644 tests/esm-child-process/process-forking-nested-relative/subfolder/worker.ts create mode 100644 tests/esm-child-process/process-forking-nested-relative/tsconfig.json create mode 100644 tests/working-dir/cjs/index.ts create mode 100644 tests/working-dir/esm-node-next/index.ts create mode 100644 tests/working-dir/esm-node-next/package.json create mode 100644 tests/working-dir/esm-node-next/tsconfig.json create mode 100644 tests/working-dir/esm/index.ts create mode 100644 tests/working-dir/esm/package.json create mode 100644 tests/working-dir/esm/tsconfig.json create mode 100644 tests/working-dir/forking/index.ts create mode 100644 tests/working-dir/forking/subfolder/worker.ts diff --git a/src/bin.ts b/src/bin.ts index 93f078658..e740bf79c 100644 --- a/src/bin.ts +++ b/src/bin.ts @@ -73,13 +73,17 @@ export function bootstrap(state: BootstrapState) { if (!state.phase2Result) { state.phase2Result = phase2(state); if (state.shouldUseChildProcess && !state.isInChildProcess) { - return callInChild(state); + // Note: When transitioning into the child-process after `phase2`, + // the updated working directory needs to be preserved. + return callInChild(state, state.phase2Result.targetCwd); } } if (!state.phase3Result) { state.phase3Result = phase3(state); if (state.shouldUseChildProcess && !state.isInChildProcess) { - return callInChild(state); + // Note: When transitioning into the child-process after `phase2`, + // the updated working directory needs to be preserved. + return callInChild(state, state.phase2Result.targetCwd); } } return phase4(state); @@ -293,7 +297,8 @@ Options: -D, --ignoreDiagnostics [code] Ignore TypeScript warnings by diagnostic code -O, --compilerOptions [opts] JSON object to merge with compiler options - --cwd Behave as if invoked within this working directory. + --cwd Sets the working directory of the spawned script. Also useful for the + automatic discovering of the \`tsconfig.json\` project. --files Load \`files\`, \`include\` and \`exclude\` from \`tsconfig.json\` on startup --pretty Use pretty diagnostic formatter (usually enabled by default) --cwdMode Use current directory instead of for config resolution @@ -318,7 +323,15 @@ Options: process.exit(0); } - const cwd = cwdArg || process.cwd(); + let targetCwd: string; + + // Switch to the target `--cwd` if specified. + if (cwdArg !== undefined) { + targetCwd = resolve(cwdArg); + process.chdir(targetCwd); + } else { + targetCwd = process.cwd(); + } // If ESM is explicitly enabled through the flag, stage3 should be run in a child process // with the ESM loaders configured. @@ -327,7 +340,7 @@ Options: } return { - cwd, + targetCwd, }; } @@ -359,7 +372,7 @@ function phase3(payload: BootstrapState) { esm, experimentalSpecifierResolution, } = payload.parseArgvResult; - const { cwd } = payload.phase2Result!; + const { targetCwd } = 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 @@ -367,13 +380,10 @@ function phase3(payload: BootstrapState) { // 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.parseArgvResult!, - payload.phase2Result! - ); + const { entryPointPath } = getEntryPointInfo(payload.parseArgvResult!); const preloadedConfig = findAndReadConfig({ - cwd, + cwd: targetCwd, emit, files, pretty, @@ -386,7 +396,7 @@ function phase3(payload: BootstrapState) { ignore, logError, projectSearchDir: getProjectSearchDir( - cwd, + targetCwd, scriptMode, cwdMode, entryPointPath @@ -431,11 +441,9 @@ function phase3(payload: BootstrapState) { * details can be found in here: https://github.com/TypeStrong/ts-node/issues/1812. */ function getEntryPointInfo( - argvResult: NonNullable, - phase2Result: NonNullable + argvResult: NonNullable ) { const { code, interactive, restArgs } = argvResult; - const { cwd } = phase2Result; // Figure out which we are executing: piped stdin, --eval, REPL, and/or entrypoint // This is complicated because node's behavior is complicated @@ -447,10 +455,8 @@ function getEntryPointInfo( (interactive || (process.stdin.isTTY && !executeEval)); const executeStdin = !executeEval && !executeRepl && !executeEntrypoint; - /** Unresolved. May point to a symlink, not realpath. May be missing file extension */ - const entryPointPath = executeEntrypoint - ? resolve(cwd, restArgs[0]) - : undefined; + /** Unresolved. May point to a symlink, not realpath. May be missing file extension */ + const entryPointPath = executeEntrypoint ? resolve(restArgs[0]) : undefined; return { executeEval, @@ -465,7 +471,7 @@ function phase4(payload: BootstrapState) { const { isInChildProcess, tsNodeScript } = payload; const { version, showConfig, restArgs, code, print, argv } = payload.parseArgvResult; - const { cwd } = payload.phase2Result!; + const { targetCwd } = payload.phase2Result!; const { preloadedConfig } = payload.phase3Result!; const { @@ -474,7 +480,7 @@ function phase4(payload: BootstrapState) { executeEval, executeRepl, executeStdin, - } = getEntryPointInfo(payload.parseArgvResult!, payload.phase2Result!); + } = getEntryPointInfo(payload.parseArgvResult!); /** * , [stdin], and [eval] are all essentially virtual files that do not exist on disc and are backed by a REPL @@ -490,7 +496,7 @@ function phase4(payload: BootstrapState) { let stdinStuff: VirtualFileState | undefined; let evalAwarePartialHost: EvalAwarePartialHost | undefined = undefined; if (executeEval) { - const state = new EvalState(join(cwd, EVAL_FILENAME)); + const state = new EvalState(join(targetCwd, EVAL_FILENAME)); evalStuff = { state, repl: createRepl({ @@ -503,10 +509,10 @@ function phase4(payload: BootstrapState) { // Create a local module instance based on `cwd`. const module = (evalStuff.module = new Module(EVAL_NAME)); module.filename = evalStuff.state.path; - module.paths = (Module as any)._nodeModulePaths(cwd); + module.paths = (Module as any)._nodeModulePaths(targetCwd); } if (executeStdin) { - const state = new EvalState(join(cwd, STDIN_FILENAME)); + const state = new EvalState(join(targetCwd, STDIN_FILENAME)); stdinStuff = { state, repl: createRepl({ @@ -519,10 +525,10 @@ function phase4(payload: BootstrapState) { // Create a local module instance based on `cwd`. const module = (stdinStuff.module = new Module(STDIN_NAME)); module.filename = stdinStuff.state.path; - module.paths = (Module as any)._nodeModulePaths(cwd); + module.paths = (Module as any)._nodeModulePaths(targetCwd); } if (executeRepl) { - const state = new EvalState(join(cwd, REPL_FILENAME)); + const state = new EvalState(join(targetCwd, REPL_FILENAME)); replStuff = { state, repl: createRepl({ @@ -607,7 +613,8 @@ function phase4(payload: BootstrapState) { }, ...ts.convertToTSConfig( service.config, - service.configFilePath ?? join(cwd, 'ts-node-implicit-tsconfig.json'), + service.configFilePath ?? + join(targetCwd, 'ts-node-implicit-tsconfig.json'), service.ts.sys ), }; @@ -623,10 +630,10 @@ function phase4(payload: BootstrapState) { // Prepend `ts-node` arguments to CLI for child processes. process.execArgv.push( tsNodeScript, - ...argv.slice(2, argv.length - restArgs.length) + ...sanitizeArgvForChildForking(argv.slice(2, argv.length - restArgs.length)) ); - // TODO this comes from BoostrapState + // TODO this comes from BootstrapState process.argv = [process.argv[1]] .concat(executeEntrypoint ? ([entryPointPath] as string[]) : []) .concat(restArgs.slice(executeEntrypoint ? 1 : 0)); @@ -749,6 +756,30 @@ function requireResolveNonCached(absoluteModuleSpecifier: string) { }); } +/** + * Sanitizes the specified argv string array to be useful for child processes + * which may be created using `child_process.fork`. Some initial `ts-node` options + * should not be preserved and forwarded to child process forks. + * + * * `--cwd` should not override the working directory in forked processes. + */ +function sanitizeArgvForChildForking(argv: string[]): string[] { + let result: string[] = []; + let omitNext = false; + + for (const value of argv) { + if (value === '--cwd' || value === '--dir') { + omitNext = true; + } else if (!omitNext) { + result.push(value); + } else { + omitNext = false; + } + } + + return result; +} + /** * Evaluate an [eval] or [stdin] script */ diff --git a/src/child/spawn-child.ts b/src/child/spawn-child.ts index 12368fcef..198c96f58 100644 --- a/src/child/spawn-child.ts +++ b/src/child/spawn-child.ts @@ -6,8 +6,13 @@ import { versionGteLt } from '../util'; const argPrefix = '--brotli-base64-config='; -/** @internal */ -export function callInChild(state: BootstrapState) { +/** + * @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, targetCwd: string) { if (!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.' @@ -29,6 +34,7 @@ export function callInChild(state: BootstrapState) { ], { stdio: 'inherit', + cwd: targetCwd, argv0: process.argv0, } ); diff --git a/src/test/esm-loader.spec.ts b/src/test/esm-loader.spec.ts index f12e04821..765c5948e 100644 --- a/src/test/esm-loader.spec.ts +++ b/src/test/esm-loader.spec.ts @@ -359,6 +359,35 @@ 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 ./working-dir/esm/ index.ts` + ); + + expect(err).toBe(null); + expect(stdout.trim()).toBe('Passing'); + expect(stderr).toBe(''); + }); + + test.suite( + 'with NodeNext TypeScript resolution and `.mts` extension', + (test) => { + test.runIf(tsSupportsStableNodeNextNode16); + + test('should have the correct working directory in the user entry-point', async () => { + const { err, stdout, stderr } = await exec( + `${BIN_PATH} --esm --cwd ./working-dir/esm-node-next/ index.ts` + ); + + 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( @@ -380,6 +409,20 @@ test.suite('esm', (test) => { expect(stderr).toBe(''); }); + 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 --cwd ./esm-child-process/process-forking-nested-relative/ index.ts` + ); + + expect(err).toBe(null); + expect(stdout.trim()).toBe('Passing: from main'); + expect(stderr).toBe(''); + } + ); + test.suite( 'with NodeNext TypeScript resolution and `.mts` extension', (test) => { diff --git a/src/test/index.spec.ts b/src/test/index.spec.ts index ca4c2cf85..3f8555d16 100644 --- a/src/test/index.spec.ts +++ b/src/test/index.spec.ts @@ -617,6 +617,30 @@ 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 ./working-dir/cjs/ index.ts` + ); + + expect(err).toBe(null); + expect(stdout.trim()).toBe('Passing'); + expect(stderr).toBe(''); + }); + + 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(''); + } + ); + test.suite('should read ts-node options from tsconfig.json', (test) => { const BIN_EXEC = `"${BIN_PATH}" --project tsconfig-options/tsconfig.json`; diff --git a/tests/esm-child-process/process-forking-nested-relative/index.ts b/tests/esm-child-process/process-forking-nested-relative/index.ts new file mode 100644 index 000000000..e0b27f3cf --- /dev/null +++ b/tests/esm-child-process/process-forking-nested-relative/index.ts @@ -0,0 +1,22 @@ +import { fork } from 'child_process'; +import { join } from 'path'; + +// Initially set the exit code to non-zero. We only set it to `0` when the +// worker process finishes properly with the expected stdout message. +process.exitCode = 1; + +const workerProcess = fork('./worker.ts', [], { + stdio: 'pipe', + cwd: join(process.cwd(), 'subfolder/'), +}); + +let stdout = ''; + +workerProcess.stdout.on('data', (chunk) => (stdout += chunk.toString('utf8'))); +workerProcess.on('error', () => (process.exitCode = 1)); +workerProcess.on('close', (status, signal) => { + if (status === 0 && signal === null && stdout.trim() === 'Works') { + console.log('Passing: from main'); + process.exitCode = 0; + } +}); diff --git a/tests/esm-child-process/process-forking-nested-relative/package.json b/tests/esm-child-process/process-forking-nested-relative/package.json new file mode 100644 index 000000000..3dbc1ca59 --- /dev/null +++ b/tests/esm-child-process/process-forking-nested-relative/package.json @@ -0,0 +1,3 @@ +{ + "type": "module" +} diff --git a/tests/esm-child-process/process-forking-nested-relative/subfolder/worker.ts b/tests/esm-child-process/process-forking-nested-relative/subfolder/worker.ts new file mode 100644 index 000000000..4114d5ab0 --- /dev/null +++ b/tests/esm-child-process/process-forking-nested-relative/subfolder/worker.ts @@ -0,0 +1,3 @@ +const message: string = 'Works'; + +console.log(message); diff --git a/tests/esm-child-process/process-forking-nested-relative/tsconfig.json b/tests/esm-child-process/process-forking-nested-relative/tsconfig.json new file mode 100644 index 000000000..1ac61592b --- /dev/null +++ b/tests/esm-child-process/process-forking-nested-relative/tsconfig.json @@ -0,0 +1,5 @@ +{ + "compilerOptions": { + "module": "ESNext" + } +} diff --git a/tests/working-dir/cjs/index.ts b/tests/working-dir/cjs/index.ts new file mode 100644 index 000000000..982e5195d --- /dev/null +++ b/tests/working-dir/cjs/index.ts @@ -0,0 +1,7 @@ +import { strictEqual } from 'assert'; +import { normalize } from 'path'; + +// Expect the working directory to be the current directory. +strictEqual(normalize(process.cwd()), normalize(__dirname)); + +console.log('Passing'); diff --git a/tests/working-dir/esm-node-next/index.ts b/tests/working-dir/esm-node-next/index.ts new file mode 100644 index 000000000..f290171a9 --- /dev/null +++ b/tests/working-dir/esm-node-next/index.ts @@ -0,0 +1,11 @@ +import { strictEqual } from 'assert'; +import { normalize, dirname } from 'path'; +import { fileURLToPath } from 'url'; + +// Expect the working directory to be the current directory. +strictEqual( + normalize(process.cwd()), + normalize(dirname(fileURLToPath(import.meta.url))) +); + +console.log('Passing'); diff --git a/tests/working-dir/esm-node-next/package.json b/tests/working-dir/esm-node-next/package.json new file mode 100644 index 000000000..3dbc1ca59 --- /dev/null +++ b/tests/working-dir/esm-node-next/package.json @@ -0,0 +1,3 @@ +{ + "type": "module" +} diff --git a/tests/working-dir/esm-node-next/tsconfig.json b/tests/working-dir/esm-node-next/tsconfig.json new file mode 100644 index 000000000..3998b5074 --- /dev/null +++ b/tests/working-dir/esm-node-next/tsconfig.json @@ -0,0 +1,5 @@ +{ + "compilerOptions": { + "module": "NodeNext" + } +} diff --git a/tests/working-dir/esm/index.ts b/tests/working-dir/esm/index.ts new file mode 100644 index 000000000..0f6220303 --- /dev/null +++ b/tests/working-dir/esm/index.ts @@ -0,0 +1,8 @@ +import { ok } from 'assert'; + +// Expect the working directory to be the current directory. +// Note: Cannot use `import.meta.url` in this variant of the test +// because older TypeScript versions do not know about this syntax. +ok(/working-dir[\/\\]esm[\/\\]?/.test(process.cwd())); + +console.log('Passing'); diff --git a/tests/working-dir/esm/package.json b/tests/working-dir/esm/package.json new file mode 100644 index 000000000..3dbc1ca59 --- /dev/null +++ b/tests/working-dir/esm/package.json @@ -0,0 +1,3 @@ +{ + "type": "module" +} diff --git a/tests/working-dir/esm/tsconfig.json b/tests/working-dir/esm/tsconfig.json new file mode 100644 index 000000000..1ac61592b --- /dev/null +++ b/tests/working-dir/esm/tsconfig.json @@ -0,0 +1,5 @@ +{ + "compilerOptions": { + "module": "ESNext" + } +} diff --git a/tests/working-dir/forking/index.ts b/tests/working-dir/forking/index.ts new file mode 100644 index 000000000..45ff8afd7 --- /dev/null +++ b/tests/working-dir/forking/index.ts @@ -0,0 +1,22 @@ +import { fork } from 'child_process'; +import { join } from 'path'; + +// Initially set the exit code to non-zero. We only set it to `0` when the +// worker process finishes properly with the expected stdout message. +process.exitCode = 1; + +const workerProcess = fork('./worker.ts', [], { + stdio: 'pipe', + cwd: join(__dirname, 'subfolder'), +}); + +let stdout = ''; + +workerProcess.stdout!.on('data', (chunk) => (stdout += chunk.toString('utf8'))); +workerProcess.on('error', () => (process.exitCode = 1)); +workerProcess.on('close', (status, signal) => { + if (status === 0 && signal === null && stdout.trim() === 'Works') { + console.log('Passing: from main'); + process.exitCode = 0; + } +}); diff --git a/tests/working-dir/forking/subfolder/worker.ts b/tests/working-dir/forking/subfolder/worker.ts new file mode 100644 index 000000000..4114d5ab0 --- /dev/null +++ b/tests/working-dir/forking/subfolder/worker.ts @@ -0,0 +1,3 @@ +const message: string = 'Works'; + +console.log(message);