Ambiguity in env/extendEnv documentation

[Izhaki]

The docs:

For extendEnv

Set to false if you don't want to extend the environment variables when providing the env property.

For env:

Environment key-value pairs. Extends automatically from process.env. Set extendEnv to false if you don't want this.

Having read this, I was under the impression that if you set extendEnv to false, env has no effect at all (which is exactly the behaviour I needed).

Here is the code:

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

So the correction:

For extendEnv:

If set to true, the existing process.env to be extended by the env property, and the result will be passed to the child process. If set to false, only the env object will be passed to the child process.

For env:

Environment key-value pairs. By default, process.env is extended by this object, and the result is given to the child process. When extendEnv is set to false, only this object will be given to the child process.

(Something like that)

[ehmicky]

Hi @lzhaki, Thanks for the feedback! I agree with you, this is confusing. Would you like to add your suggestion as a PR? :)

[Izhaki]

Sure. Will try to slot this in tomorrow.

[wesleytodd]

Hi! I just hit this behavior, and it took a bit to find this issue. IMO extendEnv should be false by default to be like the behavior of the child_process.*versions. In lieu of that I agree that that it needs better docs.

It would be a breaking change, but the change would probably just be:

https://github.com/sindresorhus/execa/blob/717d29d539e0bdf1666c1a4bc10fa406d996e7b3/index.js#L18

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

[ehmicky]

@wesleytodd thanks your feedback. However this is a separate concern (changing the default value of extendEnv) which should be a separate issue.

[wesleytodd]

I am happy to open a separate issue, but if I we went that direction it would probably supersede this issue since documenting the current behavior would be unnecessary. Right?

[ehmicky]

Yes it would, so we can link to that separate issue.

[wesleytodd]

You are marking these as off topic, but @Izhaki (nor anyone else who lands here) should probably not move forward on making a documentation update if you are considering changing the behavior. Maybe it would be good to leave at least one of these as visible so others do not come here and re-hash the same conversation (which is relevant to the issue)?

[ehmicky]

@wesleytodd Sorry for putting this off-topic. I am just trying to separate those two separate issues (changing the default value and changing the documentation). Those are connected (I am going to post a link below) but are separate.

[ehmicky]

Note: there's also a related discussion at #394 about whether the extendEnv option default value should be true or false. I think the documentation can be changed regardless of #394. We can update it as part of #394 if we wanted to go that road.

[ehmicky]

#394 was closed since this issue is superseding it. Basically all we need is clarifying the env and extendEnv options. @lzhaki let us know what you think.