Fix replacing the HTTP cache

source/core/index.ts

@@ -23,6 +23,7 @@ import proxyEvents from './utils/proxy-events';
 import timedOut, {Delays, TimeoutError as TimedOutTimeoutError} from './utils/timed-out';
 import urlToOptions from './utils/url-to-options';
 import optionsToUrl, {URLOptions} from './utils/options-to-url';
+import WeakableMap from './utils/weakable-map';
 
 type HttpRequestFunction = typeof httpRequest;
 type Error = NodeJS.ErrnoException;
@@ -107,6 +108,11 @@ export type RequestFunction = (url: URL, options: RequestOptions, callback?: (re
 
 export type Headers = Record<string, string | string[] | undefined>;
 
+type CacheableRequestFn = (
+	opts: string | URL | RequestOptions,
+	cb?: (response: ServerResponse | ResponseLike) => void
+) => CacheableRequest.Emitter;
+
 export interface Options extends URLOptions, SecureContextOptions {
 	request?: RequestFunction;
 	agent?: Agents | false;
@@ -160,7 +166,6 @@ export interface NormalizedOptions extends Options {
 	cache?: string | CacheableRequest.StorageAdapter;
 	throwHttpErrors: boolean;
 	dnsCache?: CacheableLookup;
-	cacheableRequest?: (options: string | URL | http.RequestOptions, callback?: (response: http.ServerResponse | ResponseLike) => void) => CacheableRequest.Emitter;
 	http2: boolean;
 	allowGetBody: boolean;
 	rejectUnauthorized: boolean;
@@ -247,15 +252,17 @@ function isClientRequest(clientRequest: unknown): clientRequest is ClientRequest
 	return is.object(clientRequest) && !('statusCode' in clientRequest);
 }
 
+const cacheableStore = new WeakableMap<string | CacheableRequest.StorageAdapter, CacheableRequestFn>();
+
 const cacheFn = async (url: URL, options: RequestOptions): Promise<ClientRequest | ResponseLike> => new Promise<ClientRequest | ResponseLike>((resolve, reject) => {
 	// TODO: Remove `utils/url-to-options.ts` when `cacheable-request` is fixed
 	Object.assign(options, urlToOptions(url));
 
 	// `http-cache-semantics` checks this
 	delete (options as unknown as NormalizedOptions).url;
 
-	// TODO: `cacheable-request` is incorrectly typed
-	const cacheRequest = (options as Pick<NormalizedOptions, 'cacheableRequest'>).cacheableRequest!(options, resolve as any);
+	// This is ugly
+	const cacheRequest = cacheableStore.get((options as any).cache)!(options, resolve as any);
 
 	// Restore options
 	(options as unknown as NormalizedOptions).url = url;
@@ -762,12 +769,14 @@ export default class Request extends Duplex implements RequestEvents<Request> {
 		}
 
 		// `options.cache`
-		if (options.cache && !(options as NormalizedOptions).cacheableRequest) {
-			// Better memory management, so we don't have to generate a new object every time
-			(options as NormalizedOptions).cacheableRequest = new CacheableRequest(
-				((requestOptions: RequestOptions, handler?: (response: IncomingMessage) => void): ClientRequest => (requestOptions as Pick<NormalizedOptions, typeof kRequest>)[kRequest](requestOptions, handler)) as HttpRequestFunction,
-				options.cache
-			);
+		let {cache} = options;
+		if (cache) {
+			if (!cacheableStore.has(cache)) {
+				cacheableStore.set(cache, new CacheableRequest(
+					((requestOptions: RequestOptions, handler?: (response: IncomingMessage) => void): ClientRequest => (requestOptions as Pick<NormalizedOptions, typeof kRequest>)[kRequest](requestOptions, handler)) as HttpRequestFunction,
+					cache as CacheableRequest.StorageAdapter
+				));
+			}
 		}
 
 		// `options.dnsCache`
@@ -1257,7 +1266,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
 		}
 
 		const realFn = options.request ?? fallbackFn;
-		const fn = options.cacheableRequest ? cacheFn : realFn;
+		const fn = options.cache ? cacheFn : realFn;
 
 		if (agent && !options.http2) {
 			(options as unknown as RequestOptions).agent = agent[isHttps ? 'https' : 'http'];

source/core/utils/weakable-map.ts

@@ -0,0 +1,33 @@
+export default class WeakableMap<K, V> {
+	weakMap: WeakMap<object, V>;
+	map: Map<K, V>;
+
+	constructor() {
+		this.weakMap = new WeakMap();
+		this.map = new Map();
+	}
+
+	set(key: K, value: V): void {
+		if (typeof key === 'object') {
+			this.weakMap.set(key as unknown as object, value);
+		} else {
+			this.map.set(key, value);
+		}
+	}
+
+	get(key: K): V | undefined {
+		if (typeof key === 'object') {
+			return this.weakMap.get(key as unknown as object);
+		} else {
+			return this.map.get(key);
+		}
+	}
+
+	has(key: K): boolean {
+		if (typeof key === 'object') {
+			return this.weakMap.has(key as unknown as object);
+		} else {
+			return this.map.has(key);
+		}
+	}
+}

test/cache.ts

@@ -242,7 +242,7 @@ test('decompresses cached responses', withServer, async (t, server, got) => {
 			response.end();
 		} else {
 			response.setHeader('content-encoding', 'gzip');
-			response.setHeader('cache-control', 'public, max-age: 60');
+			response.setHeader('cache-control', 'public, max-age=60');
 			response.setHeader('etag', 'foobar');
 			response.end(compressed);
 		}
@@ -259,4 +259,29 @@ test('decompresses cached responses', withServer, async (t, server, got) => {
 			retry: 2
 		}));
 	}
+
+	t.is(cache.size, 1);
+});
+
+test('can replace the instance\'s HTTP cache', withServer, async (t, server, got) => {
+	server.get('/', cacheEndpoint);
+
+	const cache = new Map();
+	const secondCache = new Map();
+
+	const instance = got.extend({
+		mutableDefaults: true,
+		cache
+	});
+
+	await t.notThrowsAsync(instance(''));
+	await t.notThrowsAsync(instance(''));
+
+	instance.defaults.options.cache = secondCache;
+
+	await t.notThrowsAsync(instance(''));
+	await t.notThrowsAsync(instance(''));
+
+	t.is(cache.size, 1);
+	t.is(secondCache.size, 1);
 });