From 870ba800da56dd31ba4274cc33de4400286e4507 Mon Sep 17 00:00:00 2001 From: Hisham Ali Date: Thu, 19 Oct 2023 01:17:26 +0800 Subject: [PATCH] fix: [GH-267] Fix CacheOptions Related Types (#268) * Fix RequestOptions.cacheOptions function return type to also return a non-promise value. * Fix propagation of the cache options generic type RequestOptions and AugmentedRequest. Fixes #267 --- .changeset/perfect-experts-sell.md | 6 ++++++ src/HTTPCache.ts | 21 ++++++++++----------- src/RESTDataSource.ts | 26 ++++++++++++++------------ 3 files changed, 30 insertions(+), 23 deletions(-) create mode 100644 .changeset/perfect-experts-sell.md diff --git a/.changeset/perfect-experts-sell.md b/.changeset/perfect-experts-sell.md new file mode 100644 index 0000000..c3f341c --- /dev/null +++ b/.changeset/perfect-experts-sell.md @@ -0,0 +1,6 @@ +--- +'@apollo/datasource-rest': patch +--- + +* Fix RequestOptions.cacheOptions function return type to also return a non-promise value. +* Fix propagation of the cache options generic type `RequestOptions` and `AugmentedRequest`. diff --git a/src/HTTPCache.ts b/src/HTTPCache.ts index 42cef2d..0f65f51 100644 --- a/src/HTTPCache.ts +++ b/src/HTTPCache.ts @@ -5,11 +5,7 @@ import nodeFetch, { } from 'node-fetch'; import CachePolicy from 'http-cache-semantics'; import type { Options as HttpCacheSemanticsOptions } from 'http-cache-semantics'; -import type { - Fetcher, - FetcherResponse, - FetcherRequestInit, -} from '@apollo/utils.fetcher'; +import type { Fetcher, FetcherResponse } from '@apollo/utils.fetcher'; import { type KeyValueCache, InMemoryLRUCache, @@ -53,7 +49,7 @@ export class HTTPCache { async fetch( url: URL, - requestOpts: FetcherRequestInit = {}, + requestOpts: RequestOptions = {}, cache?: { cacheKey?: string; cacheOptions?: @@ -61,7 +57,7 @@ export class HTTPCache { | (( url: string, response: FetcherResponse, - request: RequestOptions, + request: RequestOptions, ) => ValueOrPromise); httpCacheSemanticsCachePolicyOptions?: HttpCacheSemanticsOptions; }, @@ -148,7 +144,7 @@ export class HTTPCache { const revalidationHeaders = policy.revalidationHeaders( policyRequestFrom(urlString, requestOpts), ); - const revalidationRequest: RequestOptions = { + const revalidationRequest = { ...requestOpts, headers: cachePolicyHeadersToFetcherHeadersInit(revalidationHeaders), }; @@ -185,7 +181,7 @@ export class HTTPCache { private async storeResponseAndReturnClone( url: string, response: FetcherResponse, - request: RequestOptions, + request: RequestOptions, policy: SneakyCachePolicy, cacheKey: string, cacheOptions?: @@ -193,7 +189,7 @@ export class HTTPCache { | (( url: string, response: FetcherResponse, - request: RequestOptions, + request: RequestOptions, ) => ValueOrPromise), ): Promise { if (typeof cacheOptions === 'function') { @@ -288,7 +284,10 @@ function canBeRevalidated(response: FetcherResponse): boolean { return response.headers.has('ETag') || response.headers.has('Last-Modified'); } -function policyRequestFrom(url: string, request: RequestOptions) { +function policyRequestFrom( + url: string, + request: RequestOptions, +) { return { url, method: request.method ?? 'GET', diff --git a/src/RESTDataSource.ts b/src/RESTDataSource.ts index 264448f..7f44790 100644 --- a/src/RESTDataSource.ts +++ b/src/RESTDataSource.ts @@ -43,8 +43,8 @@ export type RequestOptions = | (( url: string, response: FetcherResponse, - request: RequestOptions, - ) => Promise); + request: RequestOptions, + ) => ValueOrPromise); /** * If provided, this is passed through as the third argument to `new * CachePolicy()` from the `http-cache-semantics` npm package as part of the @@ -205,7 +205,7 @@ export abstract class RESTDataSource { // won't return a cache entry whose Vary-ed header field doesn't match, new // responses can overwrite old ones with different Vary-ed header fields if // you don't take the header into account in the cache key. - protected cacheKeyFor(url: URL, request: RequestOptions): string { + protected cacheKeyFor(url: URL, request: RequestOptions): string { return request.cacheKey ?? `${request.method ?? 'GET'} ${url}`; } @@ -227,7 +227,7 @@ export abstract class RESTDataSource { */ protected requestDeduplicationPolicyFor( url: URL, - request: RequestOptions, + request: RequestOptions, ): RequestDeduplicationPolicy { const method = request.method ?? 'GET'; // Start with the cache key that is used for the shared header-sensitive @@ -258,12 +258,12 @@ export abstract class RESTDataSource { protected willSendRequest?( path: string, - requestOpts: AugmentedRequest, + requestOpts: AugmentedRequest, ): ValueOrPromise; protected resolveURL( path: string, - _request: AugmentedRequest, + _request: AugmentedRequest, ): ValueOrPromise { return new URL(path, this.baseURL); } @@ -276,7 +276,7 @@ export abstract class RESTDataSource { protected didEncounterError( _error: Error, - _request: RequestOptions, + _request: RequestOptions, // TODO(v7): this shouldn't be optional in a future major version _url?: URL, ) { @@ -331,7 +331,9 @@ export abstract class RESTDataSource { return cloneDeep(parsedBody); } - protected shouldJSONSerializeBody(body: RequestWithBody['body']): boolean { + protected shouldJSONSerializeBody( + body: RequestWithBody['body'], + ): boolean { return !!( // We accept arbitrary objects and arrays as body and serialize them as JSON. ( @@ -352,7 +354,7 @@ export abstract class RESTDataSource { protected async throwIfResponseIsError(options: { url: URL; - request: RequestOptions; + request: RequestOptions; response: FetcherResponse; parsedBody: unknown; }) { @@ -367,7 +369,7 @@ export abstract class RESTDataSource { parsedBody, }: { url?: URL; - request?: RequestOptions; + request?: RequestOptions; response: FetcherResponse; parsedBody: unknown; }) { @@ -483,7 +485,7 @@ export abstract class RESTDataSource { }); } - const augmentedRequest: AugmentedRequest = { + const augmentedRequest: AugmentedRequest = { ...incomingRequest, // guarantee params and headers objects before calling `willSendRequest` for convenience params: @@ -632,7 +634,7 @@ export abstract class RESTDataSource { protected async trace( url: URL, - request: RequestOptions, + request: RequestOptions, fn: () => Promise, ): Promise { if (NODE_ENV === 'development') {