Change default behavior of extendEnv to false

[wesleytodd]

As discussed in #390, the behavior is documented a bit confusingly, which for my use meant I was getting unexpected behavior when just passing a new env object which I assumed behaved like child_process and would fully replace. I brought up there the idea of changing the default behavior to align with node core's behavior. If noone has objections I can open a PR with the following change:

execa/index.js

Line 18 in 717d29d

const env = extendEnv ? {...process.env, ...envOption} : envOption;

const env = extendEnv === true ? {...process.env, ...envOption} : envOption;

[ehmicky]

I think the idea behind the extendEnv option was that the default Node.js behavior was counter-intuitive and that it would be more user-friendly to extend environment options by default. This is at least what I get from reading the initial issue that brought this feature up (#87).

I personally agree with that view so I would not modify the default value.

However I would like to know what's the opinion of @sindresorhus about this?

[wesleytodd]

In a void I might agree, but because this module defers to the documentation for child_process a bit on how it works, I assumed that that also worked this way.

My use case is specifically that I need to pass a subset of the current process.env to the child process, so I did what I always do:

const env = Object.keys(opts.env || process.env).reduce((e, key) => {
  // remove npm environment
  if (key.startsWith('npm_')) {
    return e
  }
  e[key] = process.env[key]
  return e
}, {})
await execa(/* ... */, { env })

Obviously, this was not the behavior I expected because, well, here I am :)

So my case here is not really that one approach is better, but more that because node core is the canonical implementation and any deviation is unexpected unless very clearly documentd (as #390 would hopefully do).

[ehmicky]

@wesleytodd I hear you :)
I would like to know what @sindresorhus thinks about this one.

[sindresorhus]

I think the current behavior is more user-friendly, so I wouldn't want to change that. We should rather more clearly indicate the how the behavior is different and also defer less to the child_process docs.

[ehmicky]

I agree. Changing the documentation is covered by #390: I will update that issue accordingly.

[wesleytodd]

I can see how for small additions to the env it is nice the way it is. And I agree that with better docs I might not have had this expectation. Thanks for considering it!

[ehmicky]

Thanks @wesleytodd for bringing this issue up!