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
apollographql · Oct 18, 2023 · 870ba80 · 870ba80
1 parent aed01be
commit 870ba80
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 23 deletions.
diff --git a/.changeset/perfect-experts-sell.md 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
@@ -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,15 +49,15 @@ export class HTTPCache<CO extends CacheOptions = CacheOptions> {
 
   async fetch(
     url: URL,
-    requestOpts: FetcherRequestInit = {},
+    requestOpts: RequestOptions<CO> = {},
     cache?: {
       cacheKey?: string;
       cacheOptions?:
         | CO
         | ((
             url: string,
             response: FetcherResponse,
-            request: RequestOptions,
+            request: RequestOptions<CO>,
           ) => ValueOrPromise<CO | undefined>);
       httpCacheSemanticsCachePolicyOptions?: HttpCacheSemanticsOptions;
     },
@@ -148,7 +144,7 @@ export class HTTPCache<CO extends CacheOptions = CacheOptions> {
       const revalidationHeaders = policy.revalidationHeaders(
         policyRequestFrom(urlString, requestOpts),
       );
-      const revalidationRequest: RequestOptions = {
+      const revalidationRequest = {
         ...requestOpts,
         headers: cachePolicyHeadersToFetcherHeadersInit(revalidationHeaders),
       };
@@ -185,15 +181,15 @@ export class HTTPCache<CO extends CacheOptions = CacheOptions> {
   private async storeResponseAndReturnClone(
     url: string,
     response: FetcherResponse,
-    request: RequestOptions,
+    request: RequestOptions<CO>,
     policy: SneakyCachePolicy,
     cacheKey: string,
     cacheOptions?:
       | CO
       | ((
           url: string,
           response: FetcherResponse,
-          request: RequestOptions,
+          request: RequestOptions<CO>,
         ) => ValueOrPromise<CO | undefined>),
   ): Promise<ResponseWithCacheWritePromise> {
     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<CO extends CacheOptions = CacheOptions>(
+  url: string,
+  request: RequestOptions<CO>,
+) {
   return {
     url,
     method: request.method ?? 'GET',

diff --git a/src/RESTDataSource.ts b/src/RESTDataSource.ts
@@ -43,8 +43,8 @@ export type RequestOptions<CO extends CacheOptions = CacheOptions> =
       | ((
           url: string,
           response: FetcherResponse,
-          request: RequestOptions,
-        ) => Promise<CO | undefined>);
+          request: RequestOptions<CO>,
+        ) => ValueOrPromise<CO | undefined>);
     /**
      * 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<CO extends CacheOptions = CacheOptions> {
   // 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<CO>): string {
     return request.cacheKey ?? `${request.method ?? 'GET'} ${url}`;
   }
 
@@ -227,7 +227,7 @@ export abstract class RESTDataSource<CO extends CacheOptions = CacheOptions> {
    */
   protected requestDeduplicationPolicyFor(
     url: URL,
-    request: RequestOptions,
+    request: RequestOptions<CO>,
   ): 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<CO extends CacheOptions = CacheOptions> {
 
   protected willSendRequest?(
     path: string,
-    requestOpts: AugmentedRequest,
+    requestOpts: AugmentedRequest<CO>,
   ): ValueOrPromise<void>;
 
   protected resolveURL(
     path: string,
-    _request: AugmentedRequest,
+    _request: AugmentedRequest<CO>,
   ): ValueOrPromise<URL> {
     return new URL(path, this.baseURL);
   }
@@ -276,7 +276,7 @@ export abstract class RESTDataSource<CO extends CacheOptions = CacheOptions> {
 
   protected didEncounterError(
     _error: Error,
-    _request: RequestOptions,
+    _request: RequestOptions<CO>,
     // TODO(v7): this shouldn't be optional in a future major version
     _url?: URL,
   ) {
@@ -331,7 +331,9 @@ export abstract class RESTDataSource<CO extends CacheOptions = CacheOptions> {
     return cloneDeep(parsedBody);
   }
 
-  protected shouldJSONSerializeBody(body: RequestWithBody['body']): boolean {
+  protected shouldJSONSerializeBody(
+    body: RequestWithBody<CO>['body'],
+  ): boolean {
     return !!(
       // We accept arbitrary objects and arrays as body and serialize them as JSON.
       (
@@ -352,7 +354,7 @@ export abstract class RESTDataSource<CO extends CacheOptions = CacheOptions> {
 
   protected async throwIfResponseIsError(options: {
     url: URL;
-    request: RequestOptions;
+    request: RequestOptions<CO>;
     response: FetcherResponse;
     parsedBody: unknown;
   }) {
@@ -367,7 +369,7 @@ export abstract class RESTDataSource<CO extends CacheOptions = CacheOptions> {
     parsedBody,
   }: {
     url?: URL;
-    request?: RequestOptions;
+    request?: RequestOptions<CO>;
     response: FetcherResponse;
     parsedBody: unknown;
   }) {
@@ -483,7 +485,7 @@ export abstract class RESTDataSource<CO extends CacheOptions = CacheOptions> {
       });
     }
 
-    const augmentedRequest: AugmentedRequest = {
+    const augmentedRequest: AugmentedRequest<CO> = {
       ...incomingRequest,
       // guarantee params and headers objects before calling `willSendRequest` for convenience
       params:
@@ -632,7 +634,7 @@ export abstract class RESTDataSource<CO extends CacheOptions = CacheOptions> {
 
   protected async trace<TResult>(
     url: URL,
-    request: RequestOptions,
+    request: RequestOptions<CO>,
     fn: () => Promise<TResult>,
   ): Promise<TResult> {
     if (NODE_ENV === 'development') {