feat: options.baseUrl (#807)

### Deprecations Client options - `options.headers` - `options.port` - `options.pathPrefix` - `options.host` are now replaced by `options.baseUrl`
octokit · Mar 11, 2018 · 96b9fc6 · 96b9fc6
1 parent 2962dcd
commit 96b9fc6
Show file tree

Hide file tree

Showing 13 changed files with 97 additions and 65 deletions.
diff --git a/README.md b/README.md
@@ -63,11 +63,8 @@ const octokit = require('@octokit/rest')({
     'user-agent': 'octokit/rest.js v1.2.3' // v1.2.3 will be current version
   },
 
-  // change for custom GitHub Enterprise URL
-  host: 'api.github.com',
-  pathPrefix: '',
-  protocol: 'https',
-  port: 443,
+  // custom GitHub Enterprise URL
+  baseUrl: 'https://api.github.com',
 
   // Node only: advanced request options can be passed as http(s) agent
   agent: undefined

diff --git a/lib/defaults.js b/lib/defaults.js
@@ -1,10 +1,7 @@
 module.exports = {
   agent: undefined, // https://nodejs.org/api/https.html#https_class_https_agent
   headers: {},
-  host: 'api.github.com',
-  pathPrefix: '', // for GitHub Enterprise
-  port: undefined,
-  protocol: 'https',
   requestMedia: 'application/vnd.github.v3+json',
-  timeout: 0
+  timeout: 0,
+  baseUrl: 'https://api.github.com'
 }
diff --git a/lib/get-request-agent.js b/lib/get-request-agent.js
@@ -20,28 +20,29 @@ function getRequestAgent (options) {
   if (agentOptionNames.length === 0) {
     return
   }
-
   agentOptionNames.forEach(option => {
     console.warn(`options.${option} is deprecated. Use "options.agent" instead`)
   })
 
   const agentOptions = pick(options, agentOptionNames)
 
+  const protocol = urlParse(options.baseUrl).protocol.replace(':', '')
+
   if ('proxy' in options) {
     const proxyAgentOptions = merge(
       urlParse(agentOptions.proxy),
       omit(agentOptions, 'proxy')
     )
 
-    if (options.protocol === 'http') {
+    if (protocol === 'http') {
       return new HttpProxyAgent(proxyAgentOptions)
     }
 
     return new HttpsProxyAgent(agentOptions)
   }
 
   /* istanbul ignore if */
-  if (options.protocol === 'http') {
+  if (protocol === 'http') {
     return new HttpAgent(agentOptions)
   }
 

diff --git a/lib/parse-client-options.js b/lib/parse-client-options.js
@@ -7,10 +7,7 @@ const getRequestAgent = require('./get-request-agent')
 const DEFAULTS = require('./defaults')
 const OPTION_NAMES = [
   'timeout',
-  'host',
-  'pathPrefix',
-  'protocol',
-  'port',
+  'baseUrl',
   'agent',
   'headers',
   'requestMedia'
@@ -25,31 +22,49 @@ function parseOptions (userOptions) {
     console.warn('DEPRECATED: followRedirects option is no longer supported. All redirects are followed correctly')
   }
 
+  if ('protocol' in userOptions) {
+    console.warn('DEPRECATED: protocol option is no longer supported. All redirects are followed correctly')
+  }
+
+  if ('host' in userOptions) {
+    console.warn('DEPRECATED: host option is no longer supported. All redirects are followed correctly')
+  }
+
+  if ('port' in userOptions) {
+    console.warn('DEPRECATED: port option is no longer supported. All redirects are followed correctly')
+  }
+
+  if ('pathPrefix' in userOptions) {
+    console.warn('DEPRECATED: pathPrefix option is no longer supported. All redirects are followed correctly')
+  }
+
   if ('Promise' in userOptions) {
     console.warn('DEPRECATED: Promise option is no longer supported. The native Promise API is used')
   }
 
   const options = defaults(pick(userOptions, OPTION_NAMES), DEFAULTS)
 
   const clientDefaults = {
+    baseUrl: options.baseUrl,
     headers: options.headers,
     request: {
       timeout: options.timeout
     }
   }
+  if (userOptions.protocol) {
+    clientDefaults.baseUrl = `${userOptions.protocol}://${userOptions.host}`
 
-  clientDefaults.baseUrl = `${options.protocol}://${options.host}`
-
-  if (options.port) {
-    clientDefaults.baseUrl += `:${options.port}`
-  }
+    if (userOptions.port) {
+      clientDefaults.baseUrl += `:${userOptions.port}`
+    }
 
-  // Check if a prefix is passed in the options and strip any leading or trailing slashes from it.
-  if (options.pathPrefix) {
-    clientDefaults.baseUrl += '/' + options.pathPrefix.replace(/(^[/]+|[/]+$)/g, '')
+    // Check if a prefix is passed in the options and strip any leading or trailing slashes from it.
+    if (userOptions.pathPrefix) {
+      clientDefaults.baseUrl += '/' + userOptions.pathPrefix.replace(/(^[/]+|[/]+$)/g, '')
+    }
   }
-
   /* istanbul ignore else */
+
   if (!process.browser) {
     clientDefaults.request.agent = getRequestAgent(userOptions)
   }

diff --git a/test/integration/agent-ca/agent-ca-test.js b/test/integration/agent-ca/agent-ca-test.js
@@ -27,9 +27,7 @@ describe('custom client certificate', () => {
 
   it('options.ca', () => {
     const octokit = new Octokit({
-      protocol: 'https',
-      host: 'localhost',
-      port: server.address().port,
+      baseUrl: 'https://localhost:' + server.address().port,
       ca
     })
 
@@ -41,9 +39,7 @@ describe('custom client certificate', () => {
 
   it('options.ca & options.rejectUnauthorized', () => {
     const octokit = new Octokit({
-      protocol: 'https',
-      host: 'localhost',
-      port: server.address().port,
+      baseUrl: 'https://localhost:' + server.address().port,
       ca: 'invalid',
       rejectUnauthorized: false
     })
@@ -59,9 +55,7 @@ describe('custom client certificate', () => {
       ca
     })
     const octokit = new Octokit({
-      protocol: 'https',
-      host: 'localhost',
-      port: server.address().port,
+      baseUrl: 'https://localhost:' + server.address().port,
       agent
     })
 
@@ -77,9 +71,7 @@ describe('custom client certificate', () => {
       rejectUnauthorized: false
     })
     const octokit = new Octokit({
-      protocol: 'https',
-      host: 'localhost',
-      port: server.address().port,
+      baseUrl: 'https://localhost:' + server.address().port,
       agent
     })
 

diff --git a/test/integration/authentication-test.js b/test/integration/authentication-test.js
@@ -9,7 +9,7 @@ describe('authentication', () => {
 
   beforeEach(() => {
     github = new GitHub({
-      host: 'authentication-test-host.com'
+      baseUrl: 'https://authentication-test-host.com'
     })
   })
 

diff --git a/test/integration/conditional-request-test.js b/test/integration/conditional-request-test.js
@@ -9,7 +9,7 @@ describe('request 304s', () => {
 
   beforeEach(() => {
     github = new GitHub({
-      host: 'request-errors-test.com'
+      baseUrl: 'https://request-errors-test.com'
     })
   })
 

diff --git a/test/integration/deprecations-test.js b/test/integration/deprecations-test.js
@@ -9,7 +9,7 @@ describe('deprecations', () => {
 
   beforeEach(() => {
     github = new GitHub({
-      host: 'deprecations-test.com'
+      baseUrl: 'https://deprecations-test.com'
     })
     cy.stub(console, 'warn')
   })
@@ -33,6 +33,41 @@ describe('deprecations', () => {
     expect(console.warn.callCount).to.equal(1)
   })
 
+  it('deprecated protocol option', () => {
+    GitHub({
+      protocol: 'https',
+      host: 'deprecations-test.com'
+    })
+    expect(console.warn.callCount).to.equal(2)
+  })
+
+  it('deprecated host option', () => {
+    GitHub({
+      protocol: 'https',
+      host: 'deprecations-test.com'
+    })
+    expect(console.warn.callCount).to.equal(2)
+  })
+
+  it('deprecated port option', () => {
+    GitHub({
+      protocol: 'https',
+      host: 'deprecations-test.com',
+      port: 1234
+    })
+    expect(console.warn.callCount).to.equal(3)
+  })
+
+  it('deprecated pathPrefix option', () => {
+    GitHub({
+      protocol: 'https',
+      host: 'deprecations-test.com',
+      port: 1234,
+      pathPrefix: '/deprecations-test.com/'
+    })
+    expect(console.warn.callCount).to.equal(4)
+  })
+
   it('deprecated Promise option', () => {
     GitHub({
       Promise: {}
@@ -42,27 +77,31 @@ describe('deprecations', () => {
 
   it('deprecated ca option', () => {
     GitHub({
+      baseUrl: 'https://api.github.com',
       ca: 'certificate123'
     })
     expect(console.warn.callCount).to.equal(1)
   })
 
   it('deprecated proxy option', () => {
     GitHub({
+      baseUrl: 'https://api.github.com',
       proxy: 'http://localhost:1234'
     })
     expect(console.warn.callCount).to.equal(1)
   })
 
   it('deprecated family option', () => {
     GitHub({
+      baseUrl: 'https://api.github.com',
       family: 6
     })
     expect(console.warn.callCount).to.equal(1)
   })
 
   it('deprecated rejectUnauthorized option', () => {
     GitHub({
+      baseUrl: 'https://api.github.com',
       rejectUnauthorized: false
     })
     expect(console.warn.callCount).to.equal(1)

diff --git a/test/integration/experimentals-test.js b/test/integration/experimentals-test.js
@@ -9,7 +9,7 @@ describe('authentication plugin', () => {
 
   beforeEach(() => {
     github = new GitHub({
-      host: 'authentication-plugin-test-host.com'
+      baseUrl: 'https://authentication-plugin-test-host.com'
     })
   })
 

diff --git a/test/integration/params-validations-test.js b/test/integration/params-validations-test.js
@@ -22,8 +22,7 @@ describe('params validations', () => {
 
   it('request error', () => {
     const github = new GitHub({
-      host: '127.0.0.1',
-      port: 8 // officially unassigned port. See https://en.wikipedia.org/wiki/List_of_TCP_and_UDP_port_numbers
+      baseUrl: 'https://127.0.0.1:8' // port: 8 // officially unassigned port. See https://en.wikipedia.org/wiki/List_of_TCP_and_UDP_port_numbers
     })
 
     return github.orgs.get({org: 'foo'})
@@ -106,7 +105,7 @@ describe('params validations', () => {
 
   it('Date object for github.issues.createMilestone({..., due_on})', () => {
     const github = new GitHub({
-      host: 'milestones-test-host.com'
+      baseUrl: 'https://milestones-test-host.com'
     })
 
     nock('https://milestones-test-host.com')
@@ -126,7 +125,7 @@ describe('params validations', () => {
 
   it('Date is passed in correct format for notifications (#716)', () => {
     const github = new GitHub({
-      host: 'notifications-test-host.com'
+      baseUrl: 'https://notifications-test-host.com'
     })
 
     nock('https://notifications-test-host.com')
@@ -146,7 +145,7 @@ describe('params validations', () => {
 
   it('does not alter passed options', () => {
     const github = new GitHub({
-      host: 'params-test-host.com'
+      baseUrl: 'https://params-test-host.com'
     })
 
     nock('https://params-test-host.com')

diff --git a/test/integration/request-errors-test.js b/test/integration/request-errors-test.js
@@ -12,7 +12,7 @@ describe('request errors', () => {
       .reply(200, {})
 
     const github = new GitHub({
-      host: 'request-errors-test.com',
+      baseUrl: 'https://request-errors-test.com',
       timeout: 1000
     })
 
@@ -31,7 +31,7 @@ describe('request errors', () => {
       .replyWithError('ooops')
 
     const github = new GitHub({
-      host: 'request-errors-test.com'
+      baseUrl: 'https://request-errors-test.com'
     })
 
     return github.orgs.get({org: 'myorg'})
@@ -49,7 +49,7 @@ describe('request errors', () => {
       .reply(404, 'not found')
 
     const github = new GitHub({
-      host: 'request-errors-test.com',
+      baseUrl: 'https://request-errors-test.com',
       timeout: 1000
     })
 
@@ -68,7 +68,7 @@ describe('request errors', () => {
       .reply(401)
 
     const github = new GitHub({
-      host: 'request-errors-test.com',
+      baseUrl: 'https://request-errors-test.com',
       timeout: 1000
     })
 
@@ -89,7 +89,7 @@ describe('request errors', () => {
       })
 
     const github = new GitHub({
-      host: 'request-errors-test.com',
+      baseUrl: 'https://request-errors-test.com',
       timeout: 1000
     })