diff --git a/README.md b/README.md index 6463f7bf..7796fc35 100644 --- 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 index f3c1a847..dd2495d4 100644 --- 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 index f788a061..bdb9f692 100644 --- a/lib/get-request-agent.js +++ b/lib/get-request-agent.js @@ -20,20 +20,21 @@ 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) } @@ -41,7 +42,7 @@ function getRequestAgent (options) { } /* 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 index 00927202..fd21691c 100644 --- 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,6 +22,22 @@ 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') } @@ -32,24 +45,26 @@ function parseOptions (userOptions) { 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 index 2f8ba86d..44af318d 100644 --- 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 index 3e3180a1..754c2137 100644 --- 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 index 8b6fe02f..af9a7160 100644 --- 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 index bc41cde9..ab4898c4 100644 --- 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,6 +77,7 @@ describe('deprecations', () => { it('deprecated ca option', () => { GitHub({ + baseUrl: 'https://api.github.com', ca: 'certificate123' }) expect(console.warn.callCount).to.equal(1) @@ -49,6 +85,7 @@ describe('deprecations', () => { it('deprecated proxy option', () => { GitHub({ + baseUrl: 'https://api.github.com', proxy: 'http://localhost:1234' }) expect(console.warn.callCount).to.equal(1) @@ -56,6 +93,7 @@ describe('deprecations', () => { it('deprecated family option', () => { GitHub({ + baseUrl: 'https://api.github.com', family: 6 }) expect(console.warn.callCount).to.equal(1) @@ -63,6 +101,7 @@ describe('deprecations', () => { 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 index 653b182b..8107a725 100644 --- 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 index 126954ec..abb5ddbb 100644 --- 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 index 850e6d18..c78ff0f1 100644 --- 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 }) diff --git a/test/integration/smoke-test.js b/test/integration/smoke-test.js index 01b98ea4..07ee2cbb 100644 --- a/test/integration/smoke-test.js +++ b/test/integration/smoke-test.js @@ -15,9 +15,7 @@ describe('smoke', () => { .reply(200, {}) const github = new GitHub({ - host: 'myhost.com', - protocol: 'http', - pathPrefix: '/my/api/' + baseUrl: 'http://myhost.com/my/api' }) return github.orgs.get({org: 'myorg'}) @@ -29,7 +27,7 @@ describe('smoke', () => { .reply(200, {}) const github = new GitHub({ - host: 'smoke-test.com' + baseUrl: 'https://smoke-test.com' }) github.orgs.get({org: 'myorg'}, done) @@ -41,7 +39,7 @@ describe('smoke', () => { .reply(200, {}) const github = new GitHub({ - host: 'smoke-test.com' + baseUrl: 'https://smoke-test.com' }) const customHeaders = { @@ -78,7 +76,7 @@ describe('smoke', () => { .reply(404, {}) const github = new GitHub({ - host: 'smoke-test.com' + baseUrl: 'https://smoke-test.com' }) github.authenticate({ diff --git a/test/util.js b/test/util.js index 010c8157..97fc9231 100644 --- a/test/util.js +++ b/test/util.js @@ -4,8 +4,6 @@ module.exports = { getInstance } -const parseUrl = require('url').parse - const fetch = require('node-fetch') const merge = require('lodash/merge') @@ -30,12 +28,8 @@ function loadFixture (scenario) { } function fixtureToInstace ({url}, options) { - url = parseUrl(url) - return new GitHub(merge(options, { - host: url.host, - protocol: url.protocol.replace(/:$/, ''), - pathPrefix: url.path + baseUrl: url })) }