Fix extendEnv option not working with shell option (#184)

Fixes #183 Options parsing was not correct for the `extendEnv` option which made it not work when combined with `shell: true` option.
sindresorhus · Mar 6, 2019 · e897f44 · e897f44
1 parent 1fd9db4
commit e897f44
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 24 deletions.
diff --git a/index.js b/index.js
@@ -15,27 +15,31 @@ const stdio = require('./lib/stdio');
 const TEN_MEGABYTES = 1000 * 1000 * 10;
 
 function handleArgs(command, args, options) {
-	options = Object.assign({
-		extendEnv: true,
-		env: {}
-	}, options);
-
-	if (options.extendEnv) {
-		options.env = Object.assign({}, process.env, options.env);
-	}
-
 	const parsed = crossSpawn._parse(command, args, options);
+	command = parsed.command;
+	args = parsed.args;
+	options = parsed.options;
 
 	options = Object.assign({
 		maxBuffer: TEN_MEGABYTES,
 		buffer: true,
 		stripFinalNewline: true,
 		preferLocal: true,
-		localDir: parsed.options.cwd || process.cwd(),
+		localDir: options.cwd || process.cwd(),
 		encoding: 'utf8',
 		reject: true,
 		cleanup: true
-	}, parsed.options, {windowsHide: true});
+	}, options, {
+		windowsHide: true
+	});
+
+	if (options.extendEnv !== false) {
+		options.env = Object.assign({}, process.env, options.env);
+	}
+
+	if (options.preferLocal) {
+		options.env = npmRunPath.env(Object.assign({}, options, {cwd: options.localDir}));
+	}
 
 	// TODO: Remove in the next major release
 	if (options.stripEof === false) {
@@ -44,26 +48,17 @@ function handleArgs(command, args, options) {
 
 	options.stdio = stdio(options);
 
-	if (options.preferLocal) {
-		options.env = npmRunPath.env(Object.assign({}, options, {cwd: options.localDir}));
-	}
-
 	if (options.detached) {
 		// #115
 		options.cleanup = false;
 	}
 
-	if (process.platform === 'win32' && path.basename(parsed.command) === 'cmd.exe') {
+	if (process.platform === 'win32' && path.basename(command) === 'cmd.exe') {
 		// #116
-		parsed.args.unshift('/q');
+		args.unshift('/q');
 	}
 
-	return {
-		command: parsed.command,
-		args: parsed.args,
-		options,
-		parsed
-	};
+	return {command, args, options, parsed};
 }
 
 function handleInput(spawned, input) {

diff --git a/test.js b/test.js
@@ -487,7 +487,7 @@ test('extend environment variables by default', async t => {
 	]);
 });
 
-test('do not extend environment with `extendEnv` option', async t => {
+test('do not extend environment with `extendEnv: false`', async t => {
 	const result = await m.stdout('environment', [], {env: {BAR: 'bar', PATH: process.env.PATH}, extendEnv: false});
 
 	t.deepEqual(result.split('\n'), [
@@ -496,6 +496,14 @@ test('do not extend environment with `extendEnv` option', async t => {
 	]);
 });
 
+test('use extend environment with `extendEnv: true` and `shell: true`', async t => {
+	process.env.TEST = 'test';
+	const command = process.platform === 'win32' ? 'echo %TEST%' : 'echo $TEST';
+	const stdout = await m.stdout(command, {shell: true, env: {}, extendEnv: true});
+	t.is(stdout, 'test');
+	delete process.env.TEST;
+});
+
 test('do not buffer when streaming', async t => {
 	const result = await getStream(m('max-buffer', ['stdout', '21'], {maxBuffer: 10}).stdout);