feat: fetch proxy configuration from NPM if any

[aslushnikov]

Fixes #8

[coveralls]

Coverage increased (+0.6%) to 98.611% when pulling efa5034 on aslushnikov:fetch-env-from-npm into e0d07a9 on Rob--W:master.

[Rob--W]

Instead of adding new getEnv(...) calls everywhere, could you modify the getEnv to fall back to npm_config_* if needed?

And please add some unit tests as well.

[aslushnikov]

Instead of adding new getEnv(...) calls everywhere, could you modify the getEnv to fall back to npm_config_* if needed?

Not all proxy-related env variables are mirrored in NPM config, e.g. there's no such thing as npm_config_all_proxy. I can whitelist allowed values in getEnv, but having these ad-hoc feels more straight-forward.

And please add some unit tests as well.

My bad! Done.

[Rob--W]

Not all proxy-related env variables are mirrored in NPM config, e.g. there's no such thing as npm_config_all_proxy

I see. There is no 1:1 relation between the variable names used here and the one used by npm.

Could you use the npm_config_ variants as a fallback? The HTTPS_PROXY (etc) variable is the more common form, so they should be preferred over the npm_config variants.

And please add some unit tests as well.

My bad! Done.

Thaanks!

[aslushnikov]

Could you use the npm_config_ variants as a fallback? The HTTPS_PROXY (etc) variable is the more common form, so they should be preferred over the npm_config variants.

I was driven by an opposite logic: preferring npm_config_* over default variants since they're more specific.

I consider the following use case: I have HTTPS_PROXY somewhere in my .bashrc for general-purpose web surfing, and if NPM doesn't work for some reason with these settings then I go figure all the npm-config business. WDYT?

[Rob--W]

Could you use the npm_config_ variants as a fallback? The HTTPS_PROXY (etc) variable is the more common form, so they should be preferred over the npm_config variants.

I was driven by an opposite logic: preferring npm_config_* over default variants since they're more specific.

I consider the following use case: I have HTTPS_PROXY somewhere in my .bashrc for general-purpose web surfing, and if NPM doesn't work for some reason with these settings then I go figure all the npm-config business. WDYT?

This sounds reasonable. I also looked up the origin of the npm config documentation, and found that it was introduced here: npm/npm#6525

In that implementation, the default value of npm_config_*proxy is unset, to allow the underlying (request) library to observe the standard *_PROXY environment variable. If npm_config_*proxy is set, the *_proxy environment variable is shadowed.

In your current patch, you're using the npm-specific environment variables before falling back to the normal ones, i.e.:

1. npm_config_[proto]_proxy
2. npm_config_proxy
3. [proto]_proxy
4. all_proxy

Could you swap 2 and 3, so that the protocol-specific environment variables take precedence over the ultimate fallback?

[aslushnikov]

Could you swap 2 and 3, so that the protocol-specific environment variables take precedence over the ultimate fallback?

Indeed, good catch! Done.

[aslushnikov]

is there anything else I can do?

[Rob--W]

Thanks for your patch. Is there anything else before I publish an update?

[aslushnikov]

Thanks for your patch. Is there anything else before I publish an update?

No, I don't think so. Once you release a new version, we'll bump it right away in Playwright.

Thank you!