Skip to content

Commit

Permalink
Revert serachParameters to searchParams
Browse files Browse the repository at this point in the history
  • Loading branch information
@szmarczak
szmarczak committed Apr 11, 2021
1 parent 3c23eea commit 7b3cbfd
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 53 deletions.
46 changes: 33 additions & 13 deletions source/core/options.ts
View file
Expand Up @@ -498,7 +498,18 @@ All parsing methods supported by Got.
*/
export type ResponseType = 'json' | 'buffer' | 'text';

export type InternalsType = Except<Options, 'followRedirects' | 'auth' | 'toJSON' | 'merge' | 'createNativeRequestOptions' | 'getRequestFunction' | 'getFallbackRequestFunction' | 'freeze'>;
type OptionsToSkip =
'searchParameters' |
'followRedirects' |
'auth' |
'toJSON' |
'merge' |
'createNativeRequestOptions' |
'getRequestFunction' |
'getFallbackRequestFunction' |
'freeze';

export type InternalsType = Except<Options, OptionsToSkip>;

export type OptionsError = NodeJS.ErrnoException & {options: Options};

Expand Down Expand Up @@ -545,7 +556,7 @@ const defaultInternals: Options['_internals'] = {
json: undefined,
cookieJar: undefined,
ignoreInvalidCookies: false,
searchParameters: undefined,
searchParams: undefined,
dnsLookup: undefined,
dnsCache: undefined,
context: {},
Expand Down Expand Up @@ -689,7 +700,7 @@ const cloneInternals = (internals: typeof defaultInternals): typeof defaultInter
beforeRetry: [...hooks.beforeRetry],
afterResponse: [...hooks.afterResponse]
},
searchParameters: internals.searchParameters ? new URLSearchParams(internals.searchParameters as URLSearchParams) : undefined,
searchParameters: internals.searchParams ? new URLSearchParams(internals.searchParams as URLSearchParams) : undefined,
pagination: {...internals.pagination}
};

Expand Down Expand Up @@ -1164,9 +1175,9 @@ export default class Options {
this._internals.password = '';
}

if (this._internals.searchParameters) {
url.search = (this._internals.searchParameters as URLSearchParams).toString();
this._internals.searchParameters = undefined;
if (this._internals.searchParams) {
url.search = (this._internals.searchParams as URLSearchParams).toString();
this._internals.searchParams = undefined;
}

if (url.hostname === 'unix') {
Expand Down Expand Up @@ -1260,21 +1271,21 @@ export default class Options {
//=> 'key=a&key=b'
```
*/
get searchParameters(): string | SearchParameters | URLSearchParams | undefined {
get searchParams(): string | SearchParameters | URLSearchParams | undefined {
if (this._internals.url) {
return (this._internals.url as URL).searchParams;
}

return this._internals.searchParameters;
return this._internals.searchParams;
}

set searchParameters(value: string | SearchParameters | URLSearchParams | undefined) {
set searchParams(value: string | SearchParameters | URLSearchParams | undefined) {
assert.any([is.string, is.object, is.undefined], value);

const url = this._internals.url as URL;

if (value === undefined) {
this._internals.searchParameters = undefined;
this._internals.searchParams = undefined;

if (url) {
url.search = '';
Expand All @@ -1283,7 +1294,7 @@ export default class Options {
return;
}

let searchParameters = (this.searchParameters ?? new URLSearchParams()) as URLSearchParams;
let searchParameters = (this.searchParams ?? new URLSearchParams()) as URLSearchParams;
let updated;

if (is.string(value) || (value instanceof URLSearchParams)) {
Expand Down Expand Up @@ -1315,10 +1326,18 @@ export default class Options {
}

if (!url) {
this._internals.searchParameters = searchParameters;
this._internals.searchParams = searchParameters;
}
}

get searchParameters() {
throw new Error('The `searchParameters` option does not exist. Use `searchParams` instead.');
}

set searchParameters(_value: unknown) {
throw new Error('The `searchParameters` option does not exist. Use `searchParams` instead.');
}

get dnsLookup(): CacheableLookup['lookup'] | undefined {
return this._internals.dnsLookup;
}
Expand Down Expand Up @@ -2168,12 +2187,13 @@ const nonEnumerableProperties = new Set([
// Getters that always throw
'auth',
'followRedirects',
'searchParameters',

// May contain sensitive data
'username',
'password',
'headers',
'searchParameters',
'searchParams',
'url',

// Privates
Expand Down
30 changes: 16 additions & 14 deletions test/arguments.ts
View file
Expand Up @@ -123,7 +123,7 @@ test('overrides `searchParams` from options', withServer, async (t, server, got)
const {body} = await got(
'?drop=this',
{
searchParameters: {
searchParams: {
test: 'wow'
}
}
Expand All @@ -136,7 +136,7 @@ test('does not duplicate `searchParams`', withServer, async (t, server, got) =>
server.get('/', echoUrl);

const instance = got.extend({
searchParameters: new URLSearchParams({foo: '123'})
searchParams: new URLSearchParams({foo: '123'})
});

const body = await instance('?bar=456').text();
Expand All @@ -148,7 +148,7 @@ test('escapes `searchParams` parameter values', withServer, async (t, server, go
server.get('/', echoUrl);

const {body} = await got({
searchParameters: {
searchParams: {
test: 'it鈥檚 ok'
}
});
Expand All @@ -159,15 +159,16 @@ test('escapes `searchParams` parameter values', withServer, async (t, server, go
test('the `searchParams` option can be a URLSearchParams', withServer, async (t, server, got) => {
server.get('/', echoUrl);

const searchParameters = new URLSearchParams({test: 'wow'});
const {body} = await got({searchParameters});
// eslint-disable-next-line unicorn/prevent-abbreviations
const searchParams = new URLSearchParams({test: 'wow'});
const {body} = await got({searchParams});
t.is(body, '/?test=wow');
});

test('ignores empty searchParams object', withServer, async (t, server, got) => {
server.get('/test', echoUrl);

t.is((await got('test', {searchParameters: {}})).requestUrl.toString(), `${server.url}/test`);
t.is((await got('test', {searchParams: {}})).requestUrl.toString(), `${server.url}/test`);
});

test('throws when passing body with a non payload method', async t => {
Expand Down Expand Up @@ -289,9 +290,9 @@ test('`prefixUrl` can be changed if the URL contains the old one', withServer, a
t.is(body, '/');
});

test('throws if the `searchParameters` value is invalid', async t => {
test('throws if the `searchParams` value is invalid', async t => {
await t.throwsAsync(got('https://example.com', {
searchParameters: {
searchParams: {
// @ts-expect-error Error tests
foo: []
}
Expand Down Expand Up @@ -392,15 +393,16 @@ test('throws a helpful error when passing `followRedirects`', async t => {

test('merges `searchParams` instances', t => {
const instance = got.extend({
searchParameters: new URLSearchParams('a=1')
searchParams: new URLSearchParams('a=1')
}, {
searchParameters: new URLSearchParams('b=2')
searchParams: new URLSearchParams('b=2')
});

const searchParameters = instance.defaults.options.searchParameters as URLSearchParams;
// eslint-disable-next-line unicorn/prevent-abbreviations
const searchParams = instance.defaults.options.searchParams as URLSearchParams;

t.is(searchParameters.get('a'), '1');
t.is(searchParameters.get('b'), '2');
t.is(searchParams.get('a'), '1');
t.is(searchParams.get('b'), '2');
});

test('throws a helpful error when passing `auth`', async t => {
Expand Down Expand Up @@ -477,7 +479,7 @@ test('encodes query string included in input', t => {
test('normalizes search params included in options', t => {
const {url} = new Options({
url: new URL('https://example.com'),
searchParameters: 'a=b c'
searchParams: 'a=b c'
});

t.is(url!.search, '?a=b+c');
Expand Down
4 changes: 2 additions & 2 deletions test/http.ts
View file
Expand Up @@ -136,8 +136,8 @@ test('`searchParams` option', withServer, async (t, server, got) => {
response.end('recent');
});

t.is((await got({searchParameters: {recent: true}})).body, 'recent');
t.is((await got({searchParameters: 'recent=true'})).body, 'recent');
t.is((await got({searchParams: {recent: true}})).body, 'recent');
t.is((await got({searchParams: 'recent=true'})).body, 'recent');
});

test('response contains url', withServer, async (t, server, got) => {
Expand Down
17 changes: 9 additions & 8 deletions test/normalize-arguments.ts
View file
Expand Up @@ -13,19 +13,20 @@ test('should merge options replacing responseType', t => {

test('no duplicated searchParams values', t => {
const options = new Options({
searchParameters: 'string=true&noDuplication=true'
searchParams: 'string=true&noDuplication=true'
}, {
searchParameters: new URLSearchParams({
searchParams: new URLSearchParams({
instance: 'true',
noDuplication: 'true'
})
});

const searchParameters = options.searchParameters as URLSearchParams;
// eslint-disable-next-line unicorn/prevent-abbreviations
const searchParams = options.searchParams as URLSearchParams;

t.is(searchParameters.get('string'), 'true');
t.is(searchParameters.get('instance'), 'true');
t.is(searchParameters.getAll('noDuplication').length, 1);
t.is(searchParams.get('string'), 'true');
t.is(searchParams.get('instance'), 'true');
t.is(searchParams.getAll('noDuplication').length, 1);
});

test('should copy non-numerable properties', t => {
Expand Down Expand Up @@ -77,7 +78,7 @@ test('should get username and password from the merged options', t => {
test('null value in search params means empty', t => {
const options = new Options({
url: new URL('http://localhost'),
searchParameters: {
searchParams: {
foo: null
}
});
Expand All @@ -88,7 +89,7 @@ test('null value in search params means empty', t => {
test('undefined value in search params means it does not exist', t => {
const options = new Options({
url: new URL('http://localhost'),
searchParameters: {
searchParams: {
foo: undefined
}
});
Expand Down
33 changes: 18 additions & 15 deletions test/pagination.ts
View file
Expand Up @@ -19,8 +19,9 @@ const resetPagination = {

const attachHandler = (server: ExtendedHttpTestServer, count: number): void => {
server.get('/', (request, response) => {
const searchParameters = new URLSearchParams(request.url.split('?')[1]);
const page = Number(searchParameters.get('page')) || 1;
// eslint-disable-next-line unicorn/prevent-abbreviations
const searchParams = new URLSearchParams(request.url.split('?')[1]);
const page = Number(searchParams.get('page')) || 1;

if (page < count) {
response.setHeader('link', `<${server.url}/?page=${page + 1}>; rel="next"`);
Expand Down Expand Up @@ -548,7 +549,7 @@ test('next url in json response', withServer, async (t, server, got) => {
}

const all = await got.paginate.all('', {
searchParameters: {
searchParams: {
page: 0
},
responseType: 'json',
Expand All @@ -566,7 +567,7 @@ test('next url in json response', withServer, async (t, server, got) => {
return {
url: next,
prefixUrl: '',
searchParameters: undefined
searchParams: undefined
};
}
}
Expand All @@ -580,7 +581,7 @@ test('next url in json response', withServer, async (t, server, got) => {
]);
});

test('pagination using searchParameters', withServer, async (t, server, got) => {
test('pagination using searchParams', withServer, async (t, server, got) => {
server.get('/', (request, response) => {
const parameters = new URLSearchParams(request.url.slice(2));
const page = Number(parameters.get('page') ?? 0);
Expand All @@ -597,7 +598,7 @@ test('pagination using searchParameters', withServer, async (t, server, got) =>
}

const all = await got.paginate.all('', {
searchParameters: {
searchParams: {
page: 0
},
responseType: 'json',
Expand All @@ -607,15 +608,16 @@ test('pagination using searchParameters', withServer, async (t, server, got) =>
},
paginate: ({response}) => {
const {next} = response.body;
const searchParameters = response.request.options.searchParameters as URLSearchParams;
const previousPage = Number(searchParameters.get('page'));
// eslint-disable-next-line unicorn/prevent-abbreviations
const searchParams = response.request.options.searchParams as URLSearchParams;
const previousPage = Number(searchParams.get('page'));

if (!next) {
return false;
}

return {
searchParameters: {
searchParams: {
page: previousPage + 1
}
};
Expand All @@ -631,7 +633,7 @@ test('pagination using searchParameters', withServer, async (t, server, got) =>
]);
});

test('pagination using extended searchParameters', withServer, async (t, server, got) => {
test('pagination using extended searchParams', withServer, async (t, server, got) => {
server.get('/', (request, response) => {
const parameters = new URLSearchParams(request.url.slice(2));
const page = Number(parameters.get('page') ?? 0);
Expand All @@ -648,13 +650,13 @@ test('pagination using extended searchParameters', withServer, async (t, server,
}

const client = got.extend({
searchParameters: {
searchParams: {
limit: 10
}
});

const all = await client.paginate.all('', {
searchParameters: {
searchParams: {
page: 0
},
responseType: 'json',
Expand All @@ -664,15 +666,16 @@ test('pagination using extended searchParameters', withServer, async (t, server,
},
paginate: ({response}) => {
const {next} = response.body;
const searchParameters = response.request.options.searchParameters as URLSearchParams;
const previousPage = Number(searchParameters.get('page'));
// eslint-disable-next-line unicorn/prevent-abbreviations
const searchParams = response.request.options.searchParams as URLSearchParams;
const previousPage = Number(searchParams.get('page'));

if (!next) {
return false;
}

return {
searchParameters: {
searchParams: {
page: previousPage + 1
}
};
Expand Down
2 changes: 1 addition & 1 deletion test/redirects.ts
View file
Expand Up @@ -111,7 +111,7 @@ test('searchParams are not breaking redirects', withServer, async (t, server, go
response.end();
});

t.is((await got('relativeSearchParam', {searchParameters: 'bang=1'})).body, 'reached');
t.is((await got('relativeSearchParam', {searchParams: 'bang=1'})).body, 'reached');
});

test('redirects GET and HEAD requests', withServer, async (t, server, got) => {
Expand Down

0 comments on commit 7b3cbfd

Please sign in to comment.