refactor: proxy

.changeset/chatty-penguins-explain.md

@@ -2,4 +2,4 @@
 "@pnpm/config": major
 ---
 
-Remove httpsProxy from the object returned by @pnpm/config. httpsProxy is assigned to proxy.
+Remove proxy from the object returned by @pnpm/config. httpsProxy and httpProxy are returned instead.

.changeset/clean-grapes-rescue.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/npm-registry-agent": major
+---
+
+Not reading the env variables anymore. The env variables are read by @pnpm/config.

packages/client/src/index.ts

@@ -7,12 +7,14 @@ import getCredentialsByURI = require('credentials-by-uri')
 import mem = require('mem')
 
 export default function (opts: {
+  authConfig: Record<string, string>,
   ca?: string,
   cert?: string,
   key?: string,
   localAddress?: string,
-  proxy?: string,
-  authConfig: Record<string, string>,
+  noProxy?: string | boolean,
+  httpProxy?: string,
+  httpsProxy?: string,
   retry?: RetryTimeoutOptions,
   strictSSL?: boolean,
   userAgent?: string,

packages/config/src/Config.ts

@@ -64,8 +64,10 @@ export interface Config {
   recursive?: boolean,
 
   // proxy
-  proxy?: string,
+  httpProxy?: string,
+  httpsProxy?: string,
   localAddress?: string,
+  noProxy?: string | boolean,
 
   // ssl
   cert?: string,
@@ -116,7 +118,7 @@ export interface Config {
 export interface ConfigWithDeprecatedSettings extends Config {
   frozenShrinkwrap?: boolean,
   globalPrefix?: string,
-  httpsProxy?: string,
+  proxy?: string,
   lockfileDirectory?: string,
   preferFrozenShrinkwrap?: boolean,
   sharedWorkspaceShrinkwrap?: boolean,

packages/config/src/index.ts

@@ -334,8 +334,20 @@ export default async (
         break
     }
   }
-  if (pnpmConfig.httpsProxy) {
-    pnpmConfig.proxy = pnpmConfig.httpsProxy
+  if (!pnpmConfig.httpsProxy) {
+    pnpmConfig.httpsProxy = pnpmConfig.proxy ?? getProcessEnv('https_proxy')
+  }
+  if (!pnpmConfig.httpProxy) {
+    pnpmConfig.httpProxy = pnpmConfig.httpsProxy ?? getProcessEnv('http_proxy') ?? getProcessEnv('proxy')
+  }
+  if (!pnpmConfig.noProxy) {
+    pnpmConfig.noProxy = getProcessEnv('no_proxy')
   }
   return { config: pnpmConfig, warnings }
 }
+
+function getProcessEnv (env: string) {
+  return process.env[env] ||
+    process.env[env.toUpperCase()] ||
+    process.env[env.toLowerCase()]
+}

packages/npm-registry-agent/src/getProcessEnv.ts

@@ -1,22 +0,0 @@
-export default function getProcessEnv (env: string[] | string) {
-  if (!env) { return }
-
-  let value
-
-  if (Array.isArray(env)) {
-    for (let e of env) {
-      value = process.env[e] ||
-        process.env[e.toUpperCase()] ||
-        process.env[e.toLowerCase()]
-      if (typeof value !== 'undefined') { break }
-    }
-  }
-
-  if (typeof env === 'string') {
-    value = process.env[env] ||
-      process.env[env.toUpperCase()] ||
-      process.env[env.toLowerCase()]
-  }
-
-  return value
-}

packages/npm-registry-agent/src/index.ts

@@ -4,7 +4,6 @@ import HttpsProxyAgent = require('https-proxy-agent')
 import LRU = require('lru-cache')
 import SocksProxyAgent = require('socks-proxy-agent')
 import { URL } from 'url'
-import getProcessEnv from './getProcessEnv'
 
 const HttpsAgent = HttpAgent.HttpsAgent
 
@@ -20,8 +19,9 @@ export default function getAgent (
     key?: string,
     maxSockets?: number,
     timeout?: number,
-    proxy: string,
-    noProxy: boolean,
+    httpProxy?: string,
+    httpsProxy?: string,
+    noProxy?: boolean | string,
   }
 ) {
   const parsedUri = new URL(uri)
@@ -76,11 +76,10 @@ export default function getAgent (
   return agent
 }
 
-function checkNoProxy (uri: string, opts: { noProxy?: boolean }) {
+function checkNoProxy (uri: string, opts: { noProxy?: boolean | string }) {
   const host = new URL(uri).hostname!.split('.').filter(x => x).reverse()
-  let noproxy = (opts.noProxy || getProcessEnv('no_proxy'))
-  if (typeof noproxy === 'string') {
-    const noproxyArr = noproxy.split(/\s*,\s*/g)
+  if (typeof opts.noProxy === 'string') {
+    const noproxyArr = opts.noProxy.split(/\s*,\s*/g)
     return noproxyArr.some(no => {
       const noParts = no.split('.').filter(x => x).reverse()
       if (!noParts.length) { return false }
@@ -92,23 +91,31 @@ function checkNoProxy (uri: string, opts: { noProxy?: boolean }) {
       return true
     })
   }
-  return noproxy
+  return opts.noProxy
 }
 
 function getProxyUri (
   uri: string,
   opts: {
-    proxy?: string,
-    noProxy?: boolean,
+    httpProxy?: string,
+    httpsProxy?: string,
+    noProxy?: boolean | string,
   }
 ) {
   const { protocol } = new URL(uri)
 
-  let proxy = opts.proxy || (
-    protocol === 'https:' && getProcessEnv('https_proxy')
-  ) || (
-      protocol === 'http:' && getProcessEnv(['https_proxy', 'http_proxy', 'proxy'])
-    )
+  let proxy: string | void = undefined
+  switch (protocol) {
+    case 'http:': {
+      proxy = opts.httpProxy
+      break
+    }
+    case 'https:': {
+      proxy = opts.httpsProxy
+      break
+    }
+  }
+
   if (!proxy) { return null }
 
   if (!proxy.startsWith('http')) {

packages/npm-registry-agent/test/index.ts

@@ -1,7 +1,6 @@
 ///<reference path="../../../typings/index.d.ts"/>
 import proxiquire = require('proxyquire')
 import test = require('tape')
-import getProcessEnv from '../lib/getProcessEnv'
 
 const MockHttp = mockHttpAgent('http')
 MockHttp['HttpsAgent'] = mockHttpAgent('https')
@@ -16,19 +15,6 @@ function mockHttpAgent (type: string) {
   }
 }
 
-test('extracts process env variables', t => {
-  process.env = { TEST_ENV: 'test', ANOTHER_ENV: 'no' }
-
-  t.deepEqual(getProcessEnv('test_ENV'), 'test', 'extracts single env')
-
-  t.deepEqual(
-    getProcessEnv(['not_existing_env', 'test_ENV', 'another_env']),
-    'test',
-    'extracts env from array of env names'
-  )
-  t.end()
-})
-
 const OPTS = {
   agent: null,
   ca: 'ca',
@@ -66,7 +52,7 @@ test('all expected options passed down to HttpsAgent', t => {
 
 test('all expected options passed down to proxy agent', t => {
   const opts = Object.assign({
-    proxy: 'https://user:pass@my.proxy:1234/foo',
+    httpsProxy: 'https://user:pass@my.proxy:1234/foo',
   }, OPTS)
   t.deepEqual(agent('https://foo.com/bar', opts), {
     __type: 'https-proxy',

packages/plugin-commands-outdated/src/outdated.ts

@@ -133,14 +133,16 @@ export type OutdatedCommandOptions = {
   | 'fetchRetryMaxtimeout'
   | 'fetchRetryMintimeout'
   | 'global'
+  | 'httpProxy'
+  | 'httpsProxy'
   | 'key'
   | 'localAddress'
   | 'lockfileDir'
   | 'networkConcurrency'
+  | 'noProxy'
   | 'offline'
   | 'optional'
   | 'production'
-  | 'proxy'
   | 'rawConfig'
   | 'registries'
   | 'selectedProjectsGraph'

packages/plugin-commands-publishing/src/recursivePublish.ts

@@ -30,11 +30,13 @@ Partial<Pick<Config,
   | 'fetchRetryMaxtimeout'
   | 'fetchRetryMintimeout'
   | 'key'
+  | 'httpProxy'
+  | 'httpsProxy'
   | 'localAddress'
   | 'lockfileDir'
+  | 'noProxy'
   | 'npmPath'
   | 'offline'
-  | 'proxy'
   | 'selectedProjectsGraph'
   | 'storeDir'
   | 'strictSsl'

packages/store-connection-manager/src/createNewStoreController.ts

@@ -19,13 +19,15 @@ type CreateResolverOptions = Pick<Config,
 export type CreateNewStoreControllerOptions = CreateResolverOptions & Pick<Config,
     | 'ca'
     | 'cert'
+    | 'httpProxy'
+    | 'httpsProxy'
     | 'key'
     | 'localAddress'
     | 'networkConcurrency'
+    | 'noProxy'
     | 'offline'
     | 'packageImportMethod'
     | 'preferOffline'
-    | 'proxy'
     | 'registry'
     | 'strictSsl'
     | 'userAgent'
@@ -42,15 +44,17 @@ export default async (
     ca: opts.ca,
     cert: opts.cert,
     fullMetadata: false,
+    httpProxy: opts.httpProxy,
+    httpsProxy: opts.httpsProxy,
     key: opts.key,
     localAddress: opts.localAddress,
     metaCache: new LRU({
       max: 10000,
       maxAge: 120 * 1000, // 2 minutes
     }) as any, // tslint:disable-line:no-any
+    noProxy: opts.noProxy,
     offline: opts.offline,
     preferOffline: opts.preferOffline,
-    proxy: opts.proxy,
     retry: {
       factor: opts.fetchRetryFactor,
       maxTimeout: opts.fetchRetryMaxtimeout,