From b3f1ac9c9725459f4ffab9baa20b9459d3495fba Mon Sep 17 00:00:00 2001 From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> Date: Mon, 9 Dec 2019 15:48:44 +0100 Subject: [PATCH] Make `options.path` backwards-compatible Also added a note on `options.auth` --- documentation/migration-guides.md | 1 + readme.md | 4 ++- source/utils/options-to-url.ts | 37 ++++++++++++++++++++---- test/options-to-url.ts | 48 ++++++++++++++++++------------- test/timeout.ts | 10 +++---- 5 files changed, 69 insertions(+), 31 deletions(-) diff --git a/documentation/migration-guides.md b/documentation/migration-guides.md index 4040a8a69..7959325a1 100644 --- a/documentation/migration-guides.md +++ b/documentation/migration-guides.md @@ -79,6 +79,7 @@ To use streams, just call `got.stream(url, options)` or `got(url, {isStream: tru - No `agentClass`/`agentOptions`/`pool` option. - No `forever` option. You need to use [forever-agent](https://github.com/request/forever-agent). - No `proxy` option. You need to [pass a custom agent](readme.md#proxies). +- No `auth` option. You need to use `username` / `password` instead. - No `baseUrl` option. Instead, there is `prefixUrl` which appends a trailing slash if not present. It will be always prepended unless `url` is an instance of URL. - No `removeRefererHeader` option. You can remove the referer header in a [`beforeRequest` hook](https://github.com/sindresorhus/got#hooksbeforeRequest): diff --git a/readme.md b/readme.md index f37158f4c..3fdc09169 100644 --- a/readme.md +++ b/readme.md @@ -98,7 +98,7 @@ const pipeline = promisify(stream.pipeline); It's a `GET` request by default, but can be changed by using different methods or via `options.method`. -**By default, Got will retry on failure. To disable this option, set [`retry`](#retry) to `0`.** +**By default, Got will retry on failure. To disable this option, set [`options.retry`](#retry) to `0`.** #### got(url?, options?) @@ -122,6 +122,8 @@ Type: `object` Any of the [`https.request`](https://nodejs.org/api/https.html#https_https_request_options_callback) options. +**Note:** Legacy Url support is disabled. `options.path` is supported only for backwards compatibility. Instead of the `options.auth`, you should use `options.username` and/or `options.password`. + ###### prefixUrl Type: `string | URL` diff --git a/source/utils/options-to-url.ts b/source/utils/options-to-url.ts index e43a1a7c4..8b7f7e0cf 100644 --- a/source/utils/options-to-url.ts +++ b/source/utils/options-to-url.ts @@ -19,9 +19,12 @@ export interface URLOptions { search?: string; searchParams?: Record | URLSearchParams | string; hash?: string; + + // The only accepted legacy Url options + path?: string; } -const keys: Array> = [ +const keys: Array> = [ 'protocol', 'username', 'password', @@ -36,12 +39,22 @@ const keys: Array> export default (options: URLOptions): URL => { let origin: string; - if (Reflect.has(options, 'path')) { - throw new TypeError('Parameter `path` is deprecated. Use `pathname` instead.'); + if (options.path) { + if (options.pathname) { + throw new TypeError('Parameters `path` and `pathname` are mutually exclusive.'); + } + + if (options.search) { + throw new TypeError('Parameters `path` and `search` are mutually exclusive.'); + } + + if (options.searchParams) { + throw new TypeError('Parameters `path` and `searchParams` are mutually exclusive.'); + } } if (Reflect.has(options, 'auth')) { - throw new TypeError('Parameter `auth` is deprecated. Use `username`/`password` instead.'); + throw new TypeError('Parameter `auth` is deprecated. Use `username` / `password` instead.'); } if (options.search && options.searchParams) { @@ -64,13 +77,27 @@ export default (options: URLOptions): URL => { const url = new URL(origin); + if (options.path) { + const searchIndex = options.path.indexOf('?'); + if (searchIndex === -1) { + options.pathname = options.path; + } else { + options.pathname = options.path.slice(0, searchIndex); + options.search = options.path.slice(searchIndex + 1); + } + } + + if (Reflect.has(options, 'path')) { + delete options.path; + } + for (const key of keys) { if (Reflect.has(options, key)) { url[key] = options[key].toString(); } } - if (Reflect.has(options, 'searchParams')) { + if (options.searchParams) { if (typeof options.searchParams !== 'string' && !(options.searchParams instanceof URLSearchParams)) { validateSearchParams(options.searchParams); } diff --git a/test/options-to-url.ts b/test/options-to-url.ts index bbd5d1fe6..ade694e4d 100644 --- a/test/options-to-url.ts +++ b/test/options-to-url.ts @@ -2,18 +2,40 @@ import test from 'ava'; import is from '@sindresorhus/is'; import optionsToUrl from '../source/utils/options-to-url'; -test('`path` is deprecated', t => { +const origin = 'https://google.com'; + +test('`path` and `pathname` are mutually exclusive', t => { + t.throws(() => { + // @ts-ignore Error tests + optionsToUrl({path: 'a', pathname: 'a'}); + }, 'Parameters `path` and `pathname` are mutually exclusive.'); +}); + +test('`path` and `search` are mutually exclusive', t => { t.throws(() => { // @ts-ignore Error tests - optionsToUrl({path: ''}); - }, 'Parameter `path` is deprecated. Use `pathname` instead.'); + optionsToUrl({path: 'a', search: 'a'}); + }, 'Parameters `path` and `search` are mutually exclusive.'); +}); + +test('`path` and `searchParams` are mutually exclusive', t => { + t.throws(() => { + // @ts-ignore Error tests + optionsToUrl({path: 'a', searchParams: {}}); + }, 'Parameters `path` and `searchParams` are mutually exclusive.'); +}); + +test('`path` option', t => { + const url = optionsToUrl({origin, path: '/?a=1'}); + t.is(url.href, `${origin}/?a=1`); + t.true(is.urlInstance(url)); }); test('`auth` is deprecated', t => { t.throws(() => { // @ts-ignore Error tests optionsToUrl({auth: ''}); - }, 'Parameter `auth` is deprecated. Use `username`/`password` instead.'); + }, 'Parameter `auth` is deprecated. Use `username` / `password` instead.'); }); test('`search` and `searchParams` are mutually exclusive', t => { @@ -24,16 +46,12 @@ test('`search` and `searchParams` are mutually exclusive', t => { }); test('`href` option', t => { - const href = 'https://google.com/'; - - const url = optionsToUrl({href}); - t.is(url.href, href); + const url = optionsToUrl({href: origin}); + t.is(url.href, `${origin}/`); t.true(is.urlInstance(url)); }); test('`origin` option', t => { - const origin = 'https://google.com'; - const url = optionsToUrl({origin}); t.is(url.href, `${origin}/`); t.true(is.urlInstance(url)); @@ -46,40 +64,30 @@ test('throws if no protocol specified', t => { }); test('`port` option', t => { - const origin = 'https://google.com'; - const url = optionsToUrl({origin, port: 8888}); t.is(url.href, `${origin}:8888/`); t.true(is.urlInstance(url)); }); test('`protocol` option', t => { - const origin = 'https://google.com'; - const url = optionsToUrl({origin, protocol: 'http:'}); t.is(url.href, 'http://google.com/'); t.true(is.urlInstance(url)); }); test('`username` option', t => { - const origin = 'https://google.com'; - const url = optionsToUrl({origin, username: 'username'}); t.is(url.href, 'https://username@google.com/'); t.true(is.urlInstance(url)); }); test('`password` option', t => { - const origin = 'https://google.com'; - const url = optionsToUrl({origin, password: 'password'}); t.is(url.href, 'https://:password@google.com/'); t.true(is.urlInstance(url)); }); test('`username` option combined with `password` option', t => { - const origin = 'https://google.com'; - const url = optionsToUrl({origin, username: 'username', password: 'password'}); t.is(url.href, 'https://username:password@google.com/'); t.true(is.urlInstance(url)); diff --git a/test/timeout.ts b/test/timeout.ts index 5dd9fe259..dc13c4bac 100644 --- a/test/timeout.ts +++ b/test/timeout.ts @@ -606,12 +606,12 @@ test.serial('cancelling the request removes timeouts', withServer, async (t, ser response.write('hello'); }); - const promise = got({ - timeout: 500, + const promise = got({ + timeout: 500, retry: 0 - }).on('downloadProgress', () => { - promise.cancel(); - }).on('request', request => { + }).on('downloadProgress', () => { + promise.cancel(); + }).on('request', request => { request.on('error', error => { if (error.message === 'Timeout awaiting \'request\' for 500ms') { t.fail(error.message);