Skip to content

Commit

Permalink
Fix deno node:process execPath compatibility (#1160)
Browse files Browse the repository at this point in the history
Co-authored-by: ehmicky <[email protected]>
  • Loading branch information
w3cj and ehmicky authored Oct 16, 2024
1 parent d3a146e commit f3a1bf3
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 2 deletions.
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

0 comments on commit f3a1bf3

Please sign in to comment.