Proxy support for amp-optimizer#857
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
|
@googlebot I signed it! |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
|
Thanks for creating this! One suggestion: as the code for integrating this is trivial, would you mind implementing this directly instead of adding an additional dependency? |
This reverts commit 8e129fc.
|
@sebastianbenz I searched for this fix that doesn't require any additional libraries. |
|
Sorry for not being more clear. Integrating |
When environment variable name https_proxy is available, HttpsProxyAgent is used as node-fetch agent.
|
@sebastianbenz Sorry for late reaction. |
sebastianbenz
left a comment
There was a problem hiding this comment.
Thanks a lot! This looks good! One request: can you also add this here https://github.com/ampproject/amp-toolbox/blob/main/packages/optimizer/lib/DomTransformer.js#L156 to enable proxy support at runtime.
|
@sebastianbenz I checked Should I replace cross-fetch to node-fetch? or any ideas? |
|
Hm - that's annoying. I think it's fine to replace cross-fetch in that case and we don't release a browser bundle yet which would benefit from cross fetch. |
|
@sebastianbenz Ok, so let's try replacing "cross-fetch" with "node-fetch" once in a separate branch. |
* Proxy support for amp-optimizer * Environment variable https_proxy support * Revert "Proxy support for amp-optimizer" This reverts commit 8e129fc. * Use https-proxy-agent. When environment variable name https_proxy is available, HttpsProxyAgent is used as node-fetch agent. Co-authored-by: Sebastian Benz <sebastian.benz@gmail.com>
Proxy support using node-fetch-with-proxy instead of node-fetch.
If you need to use proxy, just set the environment variable
https_proxy.Thanks.