Fix Node 24-specific deprecation warning (#1199)

Author: ehmicky

.github/workflows/main.yml

@@ -10,7 +10,7 @@ jobs:
       fail-fast: false
       matrix:
         node-version:
-          - 23
+          - 24
           - 18
         os:
           - ubuntu
@@ -31,7 +31,7 @@ jobs:
         with:
           args: --cache --verbose --no-progress --include-fragments --exclude packagephobia --exclude /pull/ --exclude linkedin --exclude stackoverflow --exclude stackexchange --exclude github.com/nodejs/node --exclude file:///test --exclude invalid.com '*.md' 'docs/*.md' '.github/**/*.md' '*.json' '*.js' 'lib/**/*.js' 'test/**/*.js' '*.ts' 'test-d/**/*.ts'
           fail: true
-        if: ${{ matrix.os == 'ubuntu' && matrix.node-version == 23 }}
+        if: ${{ matrix.os == 'ubuntu' && matrix.node-version == 24 }}
       - run: npm run lint
       - run: npm run type
       - run: npm run unit

lib/arguments/shell.js

@@ -0,0 +1,12 @@
+// When the `shell` option is set, any command argument is concatenated as a single string by Node.js:
+// https://github.com/nodejs/node/blob/e38ce27f3ca0a65f68a31cedd984cddb927d4002/lib/child_process.js#L614-L624
+// However, since Node 24, it also prints a deprecation warning.
+// To avoid this warning, we perform that same operation before calling `node:child_process`.
+// Shells only understand strings, which is why Node.js performs that concatenation.
+// However, we rely on users splitting command arguments as an array.
+// For example, this allows us to easily detect which arguments are passed.
+// So we do want users to pass array of arguments even with `shell: true`, but we also want to avoid any warning.
+export const concatenateShell = (file, commandArguments, options) => options.shell && commandArguments.length > 0
+	? [[file, ...commandArguments].join(' '), [], options]
+	: [file, commandArguments, options];
+

lib/methods/main-async.js

@@ -4,6 +4,7 @@ import {MaxBufferError} from 'get-stream';
 import {handleCommand} from '../arguments/command.js';
 import {normalizeOptions} from '../arguments/options.js';
 import {SUBPROCESS_OPTIONS} from '../arguments/fd-options.js';
+import {concatenateShell} from '../arguments/shell.js';
 import {addIpcMethods} from '../ipc/methods.js';
 import {makeError, makeSuccessResult} from '../return/result.js';
 import {handleResult} from '../return/reject.js';
@@ -75,7 +76,7 @@ const handleAsyncOptions = ({timeout, signal, ...options}) => {
 const spawnSubprocessAsync = ({file, commandArguments, options, startTime, verboseInfo, command, escapedCommand, fileDescriptors}) => {
 	let subprocess;
 	try {
-		subprocess = spawn(file, commandArguments, options);
+		subprocess = spawn(...concatenateShell(file, commandArguments, options));
 	} catch (error) {
 		return handleEarlyError({
 			error,

lib/methods/main-sync.js

@@ -1,6 +1,7 @@
 import {spawnSync} from 'node:child_process';
 import {handleCommand} from '../arguments/command.js';
 import {normalizeOptions} from '../arguments/options.js';
+import {concatenateShell} from '../arguments/shell.js';
 import {makeError, makeEarlyError, makeSuccessResult} from '../return/result.js';
 import {handleResult} from '../return/reject.js';
 import {handleStdioSync} from '../stdio/handle-sync.js';
@@ -115,7 +116,7 @@ const runSubprocessSync = ({file, commandArguments, options, command, escapedCom
 	try {
 		addInputOptionsSync(fileDescriptors, options);
 		const normalizedOptions = normalizeSpawnSyncOptions(options);
-		return spawnSync(file, commandArguments, normalizedOptions);
+		return spawnSync(...concatenateShell(file, commandArguments, normalizedOptions));
 	} catch (error) {
 		return makeEarlyError({
 			error,

package.json

@@ -71,6 +71,7 @@
 		"get-node": "^15.0.0",
 		"is-in-ci": "^1.0.0",
 		"is-running": "^2.1.0",
+		"log-process-errors": "^12.0.1",
 		"path-exists": "^5.0.0",
 		"path-key": "^4.0.0",
 		"tempfile": "^5.0.0",

test/helpers/fixtures-directory.js

@@ -1,8 +1,18 @@
 import path from 'node:path';
 import process from 'node:process';
 import {fileURLToPath} from 'node:url';
+import logProcessErrors from 'log-process-errors';
 import pathKey from 'path-key';
 
+// Make tests fail if any warning (such as a deprecation warning) is emitted
+logProcessErrors({
+	onError(error, event) {
+		if (event === 'warning') {
+			throw error;
+		}
+	},
+});
+
 export const PATH_KEY = pathKey();
 export const FIXTURES_DIRECTORY_URL = new URL('../fixtures/', import.meta.url);
 // @todo: use import.meta.dirname after dropping support for Node <20.11.0