8000 Fix `got.mergeOptions()` regression · sindresorhus/got@157e02b · GitHub
[go: up one dir, main page]
Skip to content

Commit 157e02b

Browse files
szmarczakszmarczak
committed
Fix got.mergeOptions() regression
Fixes #1211
1 parent 7b19e8f commit 157e02b

File tree

4 files changed

+103
-55
lines changed

4 files changed

+103
-55
lines changed

‎readme.md

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1132,17 +1132,12 @@ const handler = (options, next) => {
11321132
const instance = got.extend({handlers: [handler]});
11331133
```
11341134

1135-
#### got.extend(...instances)
1135+
#### got.extend(...options, ...instances, ...)
11361136

11371137
Merges many instances into a single one:
1138-
- options are merged using [`got.mergeOptions()`](#gotmergeoptionsparentoptions-newoptions) (+ hooks are merged too),
1138+
- options are merged using [`got.mergeOptions()`](#gotmergeoptionsparentoptions-newoptions) (including hooks),
11391139
- handlers are stored in an array (you can access them through `instance.defaults.handlers`).
11401140

1141-
#### got.extend(...options, ...instances, ...)
1142-
1143-
It's possible to combine options and instances.\
1144-
It gives the same effect as `got.extend(...options).extend(...instances)`:
1145-
11461141
```js
11471142
const a = {headers: {cat: 'meow'}};
11481143
const b = got.extend({
@@ -1159,7 +1154,7 @@ got.extend(a, b);
11591154
//=> {headers: {cat: 'meow', cow: 'moo'}}
11601155
```
11611156

1162-
#### got.mergeOptions(parentOptions, newOptions)
1157+
#### got.mergeOptions(parent, ...sources)
11631158

11641159
Extends parent options. Avoid using [object spread](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax#Spread_in_object_literals) as it doesn't work recursively:
11651160

@@ -1171,24 +1166,31 @@ const b = {headers: {cow: 'moo', wolf: ['auuu']}};
11711166
got.mergeOptions(a, b) // => {headers: {cat: 'meow', cow: 'moo', wolf: ['auuu']}}
11721167
```
11731168

1169+
**Note:** Only Got options are merged! Custom user options should be defined via [`options.context`](#context).
1170+
11741171
Options are deeply merged to a new object. The value of each key is determined as follows:
11751172

1176-
- If the new property is set to `undefined`, it keeps the old one.
1177-
- If both properties are an instances of `URLSearchParams`, a new URLSearchParams instance is created. The values are merged using [`urlSearchParams.append(key, value)`](https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams/append).
1178-
- If the parent property is an instance of `URL` and the new value is a `string` or `URL`, a new URL instance is created: [`new URL(new, parent)`](https://developer.mozilla.org/en-US/docs/Web/API/URL/URL#Syntax).
1173+
- If the new property is not defined, the old value is used.
1174+
- If the new property is explicitly set to `undefined`:
1175+
- If the parent property is a plain `object`, the parent value is deeply cloned.
1176+
- Otherwise, `undefined` is used.
1177+
- If the parent value is an instance of `URLSearchParams`:
1178+
- If the new value is a `string`, an `object` or an instance of `URLSearchParams`, a new `URLSearchParams` instance is created. The values are merged using [`urlSearchParams.append(key, value)`](https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams/append).
1179+
- Otherwise, the only available value is `undefined`.
11791180
- If the new property is a plain `object`:
11801181
- If the parent property is a plain `object` too, both values are merged recursively into a new `object`.
11811182
- Otherwise, only the new value is deeply cloned.
11821183
- If the new property is an `Array`, it overwrites the old one with a deep clone of the new property.
11831184
- Properties that are not enumerable, such as `context`, `body`, `json`, and `form`, will not be merged.
1185+
- Otherwise, the new value is assigned to the key.
1186+
11841187
```js
11851188
const a = {json: {cat: 'meow'}};
11861189
const b = {json: {cow: 'moo'}};
11871190

11881191
got.mergeOptions(a, b);
11891192
//=> {json: {cow: 'moo'}}
11901193
```
1191-
- Otherwise, the new value is assigned to the key.
11921194

11931195
#### got.defaults
11941196

@@ -1203,7 +1205,7 @@ The Got defaults used in that instance.
12031205
Type: `Function[]`\
12041206
Default: `[]`
12051207

1206-
An array of functions. You execute them directly by calling `got()`. They are some sort of "global hooks" - these functions are called first. The last handler (*it's hidden*) is either [`asPromise`](source/as-promise.ts) or [`asStream`](source/as-stream.ts), depending on the `options.isStream` property.
1208+
An array of functions. You execute them directly by calling `got()`. They are some sort of "global hooks" - these functions are called first. The last handler (*it's hidden*) is either [`asPromise`](source/core/as-promise/index.ts) or [`asStream`](source/core/index.ts), depending on the `options.isStream` property.
12071209

12081210
Each handler takes two arguments:
12091211

‎source/core/index.ts

Lines changed: 14 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -572,8 +572,6 @@ export default class Request extends Duplex implements RequestEvents<Request> {
572572

573573
static normalizeArguments(url?: string | URL, options?: Options, defaults?: Defaults): NormalizedOptions {
574574
const rawOptions = options;
575-
const searchParameters = options?.searchParams;
576-
const hooks = options?.hooks;
577575

578576
if (is.object(url) && !is.urlInstance(url)) {
579577
options = {...defaults, ...(url as Options), ...options};
@@ -589,30 +587,6 @@ export default class Request extends Duplex implements RequestEvents<Request> {
589587
}
590588
}
591589

592-
// Prevent duplicating default search params & hooks
593-
if (searchParameters === undefined) {
594-
delete options.searchParams;
595-
} else {
596-
options.searchParams = searchParameters;
597-
}
598-
599-
if (hooks === undefined) {
600-
delete options.hooks;
601-
} else {
602-
options.hooks = hooks;
603-
}
604-
605-
// Setting options to `undefined` turns off its functionalities
606-
if (rawOptions && defaults) {
607-
for (const key in rawOptions) {
608-
// @ts-ignore Dear TypeScript, all object keys are strings (or symbols which are NOT enumerable).
609-
if (is.undefined(rawOptions[key]) && !is.undefined(defaults[key])) {
610-
// @ts-ignore See the note above
611-
options[key] = defaults[key];
612-
}
613-
}
614-
}
615-
616590
// TODO: Deprecate URL options in Got 12.
617591

618592
// Support extend-specific options
@@ -647,9 +621,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
647621
}
648622

649623
// `options.headers`
650-
if (is.undefined(options.headers)) {
651-
options.headers = {};
652-
} else if (options.headers === defaults?.headers) {
624+
if (options.headers === defaults?.headers) {
653625
options.headers = {...options.headers};
654626
} else {
655627
options.headers = lowercaseKeys({...(defaults?.headers), ...options.headers});
@@ -666,19 +638,19 @@ export default class Request extends Duplex implements RequestEvents<Request> {
666638
}
667639

668640
// `options.searchParams`
669-
if (options.searchParams) {
670-
if (!is.string(options.searchParams) && !(options.searchParams instanceof URLSearchParams)) {
671-
validateSearchParameters(options.searchParams);
672-
}
641+
if ('searchParams' in options) {
642+
if (options.searchParams && options.searchParams !== defaults 10000 ?.searchParams) {
643+
if (!is.string(options.searchParams) && !(options.searchParams instanceof URLSearchParams)) {
644+
validateSearchParameters(options.searchParams);
645+
}
673646

674-
options.searchParams = new URLSearchParams(options.searchParams as Record<string, string>);
647+
options.searchParams = new URLSearchParams(options.searchParams as Record<string, string>);
675648

676-
// `normalizeArguments()` is also used to merge options
677-
defaults?.searchParams?.forEach((value, key) => {
678-
(options!.searchParams as URLSearchParams).append(key, value);
679-
});
680-
} else {
681-
options.searchParams = defaults?.searchParams;
649+
// `normalizeArguments()` is also used to merge options
650+
defaults?.searchParams?.forEach((value, key) => {
651+
(options!.searchParams as URLSearchParams).append(key, value);
652+
});
653+
}
682654
}
683655

684656
// `options.username` & `options.password`
@@ -811,6 +783,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
811783
}
812784

813785
// `options.hooks`
786+
const areHooksDefault = options.hooks === defaults?.hooks;
814787
options.hooks = {...options.hooks};
815788

816789
for (const event of knownHookEvents) {
@@ -826,7 +799,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
826799
}
827800
}
828801

829-
if (defaults) {
802+
if (defaults && !areHooksDefault) {
830803
for (const event of knownHookEvents) {
831804
const defaultHooks = defaults.hooks[event];
832805

‎test/normalize-arguments.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import {URL, URLSearchParams} from 'url';
12
import test from 'ava';
23
import got from '../source';
34

@@ -20,3 +21,26 @@ test('should copy non-numerable properties', t => {
2021

2122
t.is(mergedTwice.json, options.json);
2223
});
24+
25+
test('should replace URLs', t => {
26+
const options = got.mergeOptions({
27+
url: new URL('http://localhost:41285'),
28+
searchParams: new URLSearchParams('page=0')
29+
}, {
30+
url: 'http://localhost:41285/?page=1',
31+
searchParams: undefined
32+
});
33+
34+
const otherOptions = got.mergeOptions({
35+
url: new URL('http://localhost:41285'),
36+
searchParams: {
37+
page: 0
38+
}
39+
}, {
40+
url: 'http://localhost:41285/?page=1',
41+
searchParams: undefined
42+
});
43+
44+
t.is(options.url.href, 'http://localhost:41285/?page=1');
45+
t.is(otherOptions.url.href, 'http://localhost:41285/?page=1');
46+
});

‎test/pagination.ts

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import {URL} from 'url';
22
import test from 'ava';
3-
import got from '../source';
3+
import got, {Response} from '../source';
44
import withServer, {withBodyParsingServer} from './helpers/with-server';
55
import {ExtendedTestServer} from './helpers/types';
66

@@ -406,3 +406,52 @@ test('`stackAllItems` set to false', withServer, async (t, server, got) => {
406406

407407
t.deepEqual(result, [1, 2, 3]);
408408
});
409+
410+
test('next url in json response', withServer, async (t, server, got) => {
411+
server.get('/', (request, response) => {
412+
const parameters = new URLSearchParams(request.url.slice(2));
413+
const page = Number(parameters.get('page') ?? 0);
414+
415+
response.end(JSON.stringify({
416+
currentUrl: request.url,
417+
next: page < 3 ? `${server.url}/?page=${page + 1}` : undefined
418+
}));
419+
});
420+
421+
interface Page {
422+
currentUrl: string;
423+
next?: string;
424+
}
425+
426+
const all = await got.paginate.all('', {
427+
searchParams: {
428+
page: 0
429+
},
430+
responseType: 'json',
431+
pagination: {
432+
transform: (response: Response<Page>) => {
433+
return [response.body.currentUrl];
434+
},
435+
paginate: (response: Response<Page>) => {
436+
const {next} = response.body;
437+
438+
if (!next) {
439+
return false;
440+
}
441+
442+
return {
443+
url: next,
444+
prefixUrl: '',
445+
searchParams: undefined
446+
};
447+
}
448+
}
449+
});
450+
451+
t.deepEqual(all, [
452+
'/?page=0',
453+
'/?page=1',
454+
'/?page=2',
455+
'/?page=3'
456+
]);
457+
});

0 commit comments

Comments
 (0)
0