Skip to content

Commit

Permalink
Fix encoding option not working with ignore and inherit
Browse files Browse the repository at this point in the history
  • Loading branch information
ehmicky committed Jan 21, 2024
1 parent 85a5882 commit 8f751d8
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 16 deletions.
8 changes: 7 additions & 1 deletion lib/stdio/encoding.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import {StringDecoder} from 'node:string_decoder';
import {Buffer} from 'node:buffer';
import {isUint8Array} from './utils.js';
import {willPipeStreams} from './pipe.js';

// Apply the `encoding` option using an implicit generator.
// This encodes the final output of `stdout`/`stderr`.
export const handleStreamsEncoding = (stdioStreams, {encoding}, isSync) => {
const newStdioStreams = stdioStreams.map(stdioStream => ({...stdioStream, encoding}));
if (newStdioStreams[0].direction === 'input' || IGNORED_ENCODINGS.has(encoding) || isSync) {
if (!shouldEncodeOutput(newStdioStreams, encoding, isSync)) {
return newStdioStreams;
}

Expand All @@ -24,6 +25,11 @@ export const handleStreamsEncoding = (stdioStreams, {encoding}, isSync) => {
];
};

const shouldEncodeOutput = (stdioStreams, encoding, isSync) => stdioStreams[0].direction === 'output'
&& !IGNORED_ENCODINGS.has(encoding)
&& !isSync
&& willPipeStreams(stdioStreams);

// eslint-disable-next-line unicorn/text-encoding-identifier-case
const IGNORED_ENCODINGS = new Set(['utf8', 'utf-8', 'buffer']);

Expand Down
17 changes: 2 additions & 15 deletions lib/stdio/handle.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {handleNativeStream} from './native.js';
import {handleInputOptions} from './input.js';
import {handleStreamsEncoding} from './encoding.js';
import {normalizeGenerators} from './generator.js';
import {updateStdio} from './pipe.js';

// Handle `input`, `inputFile`, `stdin`, `stdout` and `stderr` options, before spawning, in async/sync mode
export const handleInput = (addProperties, options, isSync) => {
Expand All @@ -16,7 +17,7 @@ export const handleInput = (addProperties, options, isSync) => {
.map(stdioStreams => handleStreamsEncoding(stdioStreams, options, isSync))
.map(stdioStreams => normalizeGenerators(stdioStreams))
.map(stdioStreams => addStreamsProperties(stdioStreams, addProperties));
options.stdio = transformStdio(stdioStreamsGroups);
options.stdio = updateStdio(stdioStreamsGroups);
return stdioStreamsGroups;
};

Expand Down Expand Up @@ -91,17 +92,3 @@ const addStreamsProperties = (stdioStreams, addProperties) => stdioStreams.map(s
...stdioStream,
...addProperties[stdioStream.direction][stdioStream.type]?.(stdioStream),
}));

// When the `std*: Iterable | WebStream | URL | filePath`, `input` or `inputFile` option is used, we pipe to `spawned.std*`.
// When the `std*: Array` option is used, we emulate some of the native values ('inherit', Node.js stream and file descriptor integer). To do so, we also need to pipe to `spawned.std*`.
// Therefore the `std*` options must be either `pipe` or `overlapped`. Other values do not set `spawned.std*`.
const transformStdio = stdioStreamsGroups => stdioStreamsGroups.map(stdioStreams => transformStdioItem(stdioStreams));

const transformStdioItem = stdioStreams => {
if (stdioStreams.length > 1) {
return stdioStreams.some(({value}) => value === 'overlapped') ? 'overlapped' : 'pipe';
}

const [stdioStream] = stdioStreams;
return stdioStream.type !== 'native' && stdioStream.value !== 'overlapped' ? 'pipe' : stdioStream.value;
};
18 changes: 18 additions & 0 deletions lib/stdio/pipe.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// When the `std*: Iterable | WebStream | URL | filePath`, `input` or `inputFile` option is used, we pipe to `childProcess.std*`.
// When the `std*: Array` option is used, we emulate some of the native values ('inherit', Node.js stream and file descriptor integer). To do so, we also need to pipe to `childProcess.std*`.
// Therefore the `std*` options must be either `pipe` or `overlapped`. Other values do not set `childProcess.std*`.
export const updateStdio = stdioStreamsGroups => stdioStreamsGroups.map(stdioStreams => updateStdioItem(stdioStreams));

// Whether `childProcess.std*` will be set
export const willPipeStreams = stdioStreams => PIPED_STDIO_VALUES.has(updateStdioItem(stdioStreams));

const PIPED_STDIO_VALUES = new Set(['pipe', 'overlapped', undefined, null]);

const updateStdioItem = stdioStreams => {
if (stdioStreams.length > 1) {
return stdioStreams.some(({value}) => value === 'overlapped') ? 'overlapped' : 'pipe';
}

const [stdioStream] = stdioStreams;
return stdioStream.type !== 'native' && stdioStream.value !== 'overlapped' ? 'pipe' : stdioStream.value;
};
24 changes: 24 additions & 0 deletions test/stdio/encoding.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {Buffer} from 'node:buffer';
import {exec} from 'node:child_process';
import process from 'node:process';
import {promisify} from 'node:util';
import test from 'ava';
import getStream, {getStreamAsBuffer} from 'get-stream';
Expand Down Expand Up @@ -147,3 +148,26 @@ test('Other encodings work with transforms that return objects', async t => {
const {stdout} = await execa('noop.js', {stdout: outputObjectGenerator, encoding: 'base64'});
t.deepEqual(stdout, [foobarObject]);
});

const testIgnoredEncoding = async (t, stdoutOption, isUndefined) => {
const {stdout} = await execa('empty.js', {stdout: stdoutOption, encoding: 'base64'});
t.is(stdout === undefined, isUndefined);
};

test('Is ignored with other encodings and "ignore"', testIgnoredEncoding, 'ignore', true);
test('Is ignored with other encodings and ["ignore"]', testIgnoredEncoding, ['ignore'], true);
test('Is ignored with other encodings and "ipc"', testIgnoredEncoding, 'ipc', true);
test('Is ignored with other encodings and ["ipc"]', testIgnoredEncoding, ['ipc'], true);
test('Is ignored with other encodings and "inherit"', testIgnoredEncoding, 'inherit', true);
test('Is ignored with other encodings and ["inherit"]', testIgnoredEncoding, ['inherit'], true);
test('Is ignored with other encodings and 1', testIgnoredEncoding, 1, true);
test('Is ignored with other encodings and [1]', testIgnoredEncoding, [1], true);
test('Is ignored with other encodings and process.stdout', testIgnoredEncoding, process.stdout, true);
test('Is ignored with other encodings and [process.stdout]', testIgnoredEncoding, [process.stdout], true);
test('Is not ignored with other encodings and "pipe"', testIgnoredEncoding, 'pipe', false);
test('Is not ignored with other encodings and ["pipe"]', testIgnoredEncoding, ['pipe'], false);
test('Is not ignored with other encodings and "overlapped"', testIgnoredEncoding, 'overlapped', false);
test('Is not ignored with other encodings and ["overlapped"]', testIgnoredEncoding, ['overlapped'], false);
test('Is not ignored with other encodings and ["inherit", "pipe"]', testIgnoredEncoding, ['inherit', 'pipe'], false);
test('Is not ignored with other encodings and undefined', testIgnoredEncoding, undefined, false);
test('Is not ignored with other encodings and null', testIgnoredEncoding, null, false);

0 comments on commit 8f751d8

Please # to comment.