Fix extend() dropping numeric retry limit when merging with an object (#867)

Author: chatman-media

source/utils/merge.ts

@@ -204,7 +204,7 @@ const appendSearchParameters = (target: any, source: any): URLSearchParams => {
 };
 
 // TODO: Make this strongly-typed (no `any`).
-export const deepMerge = <T>(...sources: Array<Partial<T> | undefined>): T => {
+const deepMergeInternal = <T>(isRoot: boolean, ...sources: Array<Partial<T> | undefined>): T => {
 	let returnValue: any = {};
 	let headers: KyHeadersInit = {};
 	let hooks = {};
@@ -264,8 +264,17 @@ export const deepMerge = <T>(...sources: Array<Partial<T> | undefined>): T => {
 					continue;
 				}
 
+				// `retry` accepts a number as shorthand for `{limit: number}`. Expand it before
+				// merging so extending a numeric `retry` with an object keeps the limit instead
+				// of dropping it (e.g. `ky.create({retry: 3}).extend({retry: {methods: ['get']}})`).
+				// Scoped to the root options level so it never rewrites nested user data that
+				// happens to contain a `retry` key (e.g. a `json` request body).
+				if (isRoot && key === 'retry' && isObject(value) && !isReplace && typeof returnValue[key] === 'number') {
+					returnValue = {...returnValue, [key]: {limit: returnValue[key]}};
+				}
+
 				if (isObject(value) && !isReplace && key in returnValue) {
-					value = deepMerge(returnValue[key], value);
+					value = deepMergeInternal(false, returnValue[key], value);
 				}
 
 				returnValue = {...returnValue, [key]: value};
@@ -310,3 +319,6 @@ export const deepMerge = <T>(...sources: Array<Partial<T> | undefined>): T => {
 
 	return returnValue;
 };
+
+export const deepMerge = <T>(...sources: Array<Partial<T> | undefined>): T =>
+	deepMergeInternal<T>(true, ...sources);

test/retry.ts

@@ -468,6 +468,39 @@ test('retry - can provide retry as number', async t => {
 	t.is(requestCount, 5);
 });
 
+test('retry - extending a numeric `retry` with an object keeps the limit', async t => {
+	let requestCount = 0;
+
+	const server = await createHttpTestServer(t);
+	server.get('/', async (_request, response) => {
+		requestCount++;
+		response.sendStatus(408);
+	});
+
+	// `retry: 3` is shorthand for `{limit: 3}`. Extending it with an object
+	// should preserve that limit instead of falling back to the default.
+	const extended = ky.create({retry: 3}).extend({retry: {methods: ['get']}});
+
+	await t.throwsAsync(extended(server.url).text(), {
+		message: /Request Timeout/,
+	});
+	t.is(requestCount, 4);
+});
+
+test('retry - shorthand expansion does not rewrite nested user data with a `retry` key', async t => {
+	const server = await createHttpTestServer(t);
+	server.post('/', (request, response) => {
+		response.json({body: request.body});
+	});
+
+	// A `retry` key inside the `json` body is user data, not the `retry` option,
+	// so the number-to-`{limit}` shorthand must not touch it.
+	const client = ky.create({json: {retry: 3}}).extend({json: {retry: {foo: 'bar'}}});
+
+	const {body} = await client.post(server.url).json<{body: {retry: unknown}}>();
+	t.deepEqual(body.retry, {foo: 'bar'});
+});
+
 test('doesn\'t retry on 413 with empty statusCodes and methods', async t => {
 	let requestCount = 0;