Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix deno node:process execPath compatibility #1160

Merged
merged 5 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion lib/arguments/file-url.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {fileURLToPath} from 'node:url';

// Allow some arguments/options to be either a file path string or a file URL
export const safeNormalizeFileUrl = (file, name) => {
const fileString = normalizeFileUrl(file);
const fileString = normalizeFileUrl(normalizeDenoExecPath(file));

if (typeof fileString !== 'string') {
throw new TypeError(`${name} must be a string or a file URL: ${fileString}.`);
Expand All @@ -11,5 +11,15 @@ export const safeNormalizeFileUrl = (file, name) => {
return fileString;
};

// In Deno node:process execPath is a special object, not just a string:
// https://github.com/denoland/deno/blob/f460188e583f00144000aa0d8ade08218d47c3c1/ext/node/polyfills/process.ts#L344
const normalizeDenoExecPath = file => isDenoExecPath(file)
? file.toString()
: file;

export const isDenoExecPath = file => typeof file !== 'string'
&& file
&& Object.getPrototypeOf(file) === String.prototype;

// Same but also allows other values, e.g. `boolean` for the `shell` option
export const normalizeFileUrl = file => file instanceof URL ? fileURLToPath(file) : file;
3 changes: 2 additions & 1 deletion lib/pipe/pipe-arguments.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {normalizeParameters} from '../methods/parameters.js';
import {getStartTime} from '../return/duration.js';
import {SUBPROCESS_OPTIONS, getToStream, getFromStream} from '../arguments/fd-options.js';
import {isDenoExecPath} from '../arguments/file-url.js';

// Normalize and validate arguments passed to `source.pipe(destination)`
export const normalizePipeArguments = ({source, sourcePromise, boundOptions, createNested}, ...pipeArguments) => {
Expand Down Expand Up @@ -56,7 +57,7 @@ const getDestination = (boundOptions, createNested, firstArgument, ...pipeArgume
return {destination, pipeOptions: boundOptions};
}

if (typeof firstArgument === 'string' || firstArgument instanceof URL) {
if (typeof firstArgument === 'string' || firstArgument instanceof URL || isDenoExecPath(firstArgument)) {
if (Object.keys(boundOptions).length > 0) {
throw new TypeError('Please use .pipe("file", ..., options) or .pipe(execa("file", ..., options)) instead of .pipe(options)("file", ...).');
}
Expand Down
11 changes: 11 additions & 0 deletions test/helpers/file-path.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,15 @@
import path from 'node:path';
import process from 'node:process';

export const getAbsolutePath = file => ({file});
export const getRelativePath = filePath => ({file: path.relative('.', filePath)});
// Defined as getter so call to toString is not cached
export const getDenoNodePath = () => Object.freeze({
__proto__: String.prototype,
toString() {
return process.execPath;
},
get length() {
return this.toString().length;
},
});
14 changes: 14 additions & 0 deletions test/methods/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {execa, execaSync, execaNode} from '../../index.js';
import {FIXTURES_DIRECTORY} from '../helpers/fixtures-directory.js';
import {identity, fullStdio} from '../helpers/stdio.js';
import {foobarString} from '../helpers/input.js';
import {getDenoNodePath} from '../helpers/file-path.js';

process.chdir(FIXTURES_DIRECTORY);

Expand Down Expand Up @@ -73,6 +74,9 @@ test('Cannot use "node" as binary - "node" option sync', testDoubleNode, 'node',
test('Cannot use path to "node" as binary - execaNode()', testDoubleNode, process.execPath, execaNode);
test('Cannot use path to "node" as binary - "node" option', testDoubleNode, process.execPath, runWithNodeOption);
test('Cannot use path to "node" as binary - "node" option sync', testDoubleNode, process.execPath, runWithNodeOptionSync);
test('Cannot use deno style nodePath as binary - execaNode()', testDoubleNode, getDenoNodePath(), execaNode);
test('Cannot use deno style nodePath as binary - "node" option', testDoubleNode, getDenoNodePath(), runWithNodeOption);
test('Cannot use deno style nodePath as binary - "node" option sync', testDoubleNode, getDenoNodePath(), runWithNodeOptionSync);

const getNodePath = async () => {
const {path} = await getNode(TEST_NODE_VERSION);
Expand Down Expand Up @@ -174,6 +178,16 @@ test.serial('The "nodePath" option is relative to "cwd" - execaNode()', testCwdN
test.serial('The "nodePath" option is relative to "cwd" - "node" option', testCwdNodePath, runWithNodeOption);
test.serial('The "nodePath" option is relative to "cwd" - "node" option sync', testCwdNodePath, runWithNodeOptionSync);

const testDenoExecPath = async (t, execaMethod) => {
const {exitCode, stdout} = await execaMethod('noop.js', [], {nodePath: getDenoNodePath()});
t.is(exitCode, 0);
t.is(stdout, foobarString);
};

test('The deno style "nodePath" option can be used - execaNode()', testDenoExecPath, execaNode);
test('The deno style "nodePath" option can be used - "node" option', testDenoExecPath, runWithNodeOption);
test('The deno style "nodePath" option can be used - "node" option sync', testDenoExecPath, runWithNodeOptionSync);

const testNodeOptions = async (t, execaMethod) => {
const {stdout} = await execaMethod('empty.js', [], {nodeOptions: ['--version']});
t.is(stdout, process.version);
Expand Down
11 changes: 11 additions & 0 deletions test/pipe/pipe-arguments.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import test from 'ava';
import {$, execa} from '../../index.js';
import {setFixtureDirectory, FIXTURES_DIRECTORY} from '../helpers/fixtures-directory.js';
import {foobarString} from '../helpers/input.js';
import {getDenoNodePath} from '../helpers/file-path.js';

setFixtureDirectory();

Expand Down Expand Up @@ -73,6 +74,16 @@ test('execa.$.pipe("file", commandArguments, options)`', async t => {
t.is(stdout, foobarString);
});

test('execa.$.pipe("file", commandArguments, options with denoNodePath)`', async t => {
const {stdout} = await execa('noop.js', [foobarString]).pipe('node', ['stdin.js'], {cwd: FIXTURES_DIRECTORY, nodePath: getDenoNodePath()});
t.is(stdout, foobarString);
});

test('execa.$.pipe("file", commandArguments, denoNodePath)`', async t => {
const {stdout} = await execa('noop.js', [foobarString]).pipe(getDenoNodePath(), ['stdin.js'], {cwd: FIXTURES_DIRECTORY});
t.is(stdout, foobarString);
});

test('$.pipe.pipe("file", commandArguments, options)', async t => {
const {stdout} = await $`noop.js ${foobarString}`
.pipe`stdin.js`
Expand Down