From 98ee918adf2695ce7109c67ff35ee5d9a290c746 Mon Sep 17 00:00:00 2001 From: Brian Perry Date: Fri, 8 Nov 2024 14:35:21 -0600 Subject: [PATCH 1/4] Add cache option to fetch Fixes #809 Add support for the `cache` option in the Next Drupal client. * Add `cache` option to `JsonApiWithNextFetchOptions` type in `packages/next-drupal/src/types/options.ts`. * Update `fetch` method in `packages/next-drupal/src/next-drupal.ts` to pass the `cache` option as part of the `init` object if provided. * Add tests for the `cache` option in `fetch` and `getResource` methods in `packages/next-drupal/tests/NextDrupalBase/fetch-methods.test.ts` and `packages/next-drupal/tests/NextDrupal/resource-methods.test.ts`. --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/chapter-three/next-drupal/issues/809?shareId=XXXX-XXXX-XXXX-XXXX). --- packages/next-drupal/src/next-drupal.ts | 13 +++++++++++ packages/next-drupal/src/types/options.ts | 1 + .../tests/NextDrupal/resource-methods.test.ts | 23 +++++++++++++++++++ .../NextDrupalBase/fetch-methods.test.ts | 21 +++++++++++++++++ 4 files changed, 58 insertions(+) diff --git a/packages/next-drupal/src/next-drupal.ts b/packages/next-drupal/src/next-drupal.ts index 1c837c7c..7bf2f894 100644 --- a/packages/next-drupal/src/next-drupal.ts +++ b/packages/next-drupal/src/next-drupal.ts @@ -115,6 +115,7 @@ export class NextDrupal extends NextDrupalBase { method: "POST", body: JSON.stringify(body), withAuth: options.withAuth, + cache: options.cache, }) await this.throwIfJsonErrors(response, "Error while creating resource: ") @@ -158,6 +159,7 @@ export class NextDrupal extends NextDrupalBase { }, body: body.data.attributes.file, withAuth: options.withAuth, + cache: options.cache, }) await this.throwIfJsonErrors( @@ -202,6 +204,7 @@ export class NextDrupal extends NextDrupalBase { method: "PATCH", body: JSON.stringify(body), withAuth: options.withAuth, + cache: options.cache, }) await this.throwIfJsonErrors(response, "Error while updating resource: ") @@ -239,6 +242,7 @@ export class NextDrupal extends NextDrupalBase { const response = await this.fetch(endpoint, { method: "DELETE", withAuth: options.withAuth, + cache: options.cache, }) await this.throwIfJsonErrors(response, "Error while deleting resource: ") @@ -287,6 +291,7 @@ export class NextDrupal extends NextDrupalBase { const response = await this.fetch(endpoint, { withAuth: options.withAuth, next: options.next, + cache: options.cache, }) await this.throwIfJsonErrors(response, "Error while fetching resource: ") @@ -376,6 +381,7 @@ export class NextDrupal extends NextDrupalBase { body: JSON.stringify(payload), withAuth: options.withAuth, next: options.next, + cache: options.cache, }) const errorMessagePrefix = "Error while fetching resource by path:" @@ -435,6 +441,7 @@ export class NextDrupal extends NextDrupalBase { const response = await this.fetch(endpoint, { withAuth: options.withAuth, next: options.next, + cache: options.cache, }) await this.throwIfJsonErrors( @@ -500,6 +507,7 @@ export class NextDrupal extends NextDrupalBase { params, withAuth: options.withAuth, next: options.next, + cache: options.cache, } if (locale) { opts = { @@ -573,6 +581,7 @@ export class NextDrupal extends NextDrupalBase { const response = await this.fetch(endpoint, { withAuth: options.withAuth, next: options.next, + cache: options.cache, }) if (response.status === 404) { @@ -600,6 +609,7 @@ export class NextDrupal extends NextDrupalBase { // As per https://www.drupal.org/node/2984034 /jsonapi is public. withAuth: false, next: options?.next, + cache: options?.cache, }) await this.throwIfJsonErrors( @@ -710,6 +720,7 @@ export class NextDrupal extends NextDrupalBase { const response = await this.fetch(endpoint, { withAuth: options.withAuth, next: options.next, + cache: options.cache, }) await this.throwIfJsonErrors(response, "Error while fetching menu items: ") @@ -760,6 +771,7 @@ export class NextDrupal extends NextDrupalBase { const response = await this.fetch(endpoint, { withAuth: options.withAuth, next: options.next, + cache: options.cache, }) await this.throwIfJsonErrors(response, "Error while fetching view: ") @@ -798,6 +810,7 @@ export class NextDrupal extends NextDrupalBase { const response = await this.fetch(endpoint, { withAuth: options.withAuth, next: options.next, + cache: options.cache, }) await this.throwIfJsonErrors( diff --git a/packages/next-drupal/src/types/options.ts b/packages/next-drupal/src/types/options.ts index fc9c6aaf..eb09e6c3 100644 --- a/packages/next-drupal/src/types/options.ts +++ b/packages/next-drupal/src/types/options.ts @@ -36,6 +36,7 @@ export type JsonApiWithCacheOptions = { export type JsonApiWithNextFetchOptions = { next?: NextFetchRequestConfig + cache?: RequestCache } // TODO: Properly type this. /* eslint-disable @typescript-eslint/no-explicit-any */ diff --git a/packages/next-drupal/tests/NextDrupal/resource-methods.test.ts b/packages/next-drupal/tests/NextDrupal/resource-methods.test.ts index 35389f2c..ecdf251f 100644 --- a/packages/next-drupal/tests/NextDrupal/resource-methods.test.ts +++ b/packages/next-drupal/tests/NextDrupal/resource-methods.test.ts @@ -512,6 +512,29 @@ describe("getResource()", () => { }) ) }) + + test("makes request with cache option", async () => { + const drupal = new NextDrupal(BASE_URL) + const mockInit = { + cache: "no-store", + } as FetchOptions + + const fetchSpy = spyOnFetch() + + await drupal.getResource( + "node--recipe", + "71e04ead-4cc7-416c-b9ca-60b635fdc50f", + mockInit + ) + + expect(fetchSpy).toBeCalledTimes(1) + expect(fetchSpy).toBeCalledWith( + expect.anything(), + expect.objectContaining({ + ...mockInit, + }) + ) + }) }) describe("getResourceByPath()", () => { diff --git a/packages/next-drupal/tests/NextDrupalBase/fetch-methods.test.ts b/packages/next-drupal/tests/NextDrupalBase/fetch-methods.test.ts index cc08554d..f94a7e31 100644 --- a/packages/next-drupal/tests/NextDrupalBase/fetch-methods.test.ts +++ b/packages/next-drupal/tests/NextDrupalBase/fetch-methods.test.ts @@ -200,6 +200,27 @@ describe("fetch()", () => { }) ) }) + + test("optionally adds cache option", async () => { + const drupal = new NextDrupalBase(BASE_URL) + const mockUrl = "/mock-url" + const mockInit = { + cache: "no-store", + } as FetchOptions + + const fetchSpy = spyOnFetch() + + await drupal.fetch(mockUrl, mockInit) + + expect(fetchSpy).toBeCalledTimes(1) + expect(fetchSpy).toBeCalledWith( + `${BASE_URL}${mockUrl}`, + expect.objectContaining({ + ...defaultInit, + ...mockInit, + }) + ) + }) }) describe("getAccessToken()", () => { From 864453a360a4bda1afd8a6e15e0c389c21d4bebb Mon Sep 17 00:00:00 2001 From: Brian Perry Date: Mon, 18 Nov 2024 16:49:34 -0600 Subject: [PATCH 2/4] fix: add JsonApiWithNextFetchOptions type to instances of options object --- packages/next-drupal/src/next-drupal.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/next-drupal/src/next-drupal.ts b/packages/next-drupal/src/next-drupal.ts index 7bf2f894..11b90468 100644 --- a/packages/next-drupal/src/next-drupal.ts +++ b/packages/next-drupal/src/next-drupal.ts @@ -89,7 +89,7 @@ export class NextDrupal extends NextDrupalBase { async createResource( type: string, body: JsonApiCreateResourceBody, - options?: JsonApiOptions + options?: JsonApiOptions & JsonApiWithNextFetchOptions ): Promise { options = { deserialize: true, @@ -130,7 +130,7 @@ export class NextDrupal extends NextDrupalBase { async createFileResource( type: string, body: JsonApiCreateFileResourceBody, - options?: JsonApiOptions + options?: JsonApiOptions & JsonApiWithNextFetchOptions ): Promise { options = { deserialize: true, @@ -176,7 +176,7 @@ export class NextDrupal extends NextDrupalBase { type: string, uuid: string, body: JsonApiUpdateResourceBody, - options?: JsonApiOptions + options?: JsonApiOptions & JsonApiWithNextFetchOptions ): Promise { options = { deserialize: true, @@ -219,7 +219,7 @@ export class NextDrupal extends NextDrupalBase { async deleteResource( type: string, uuid: string, - options?: JsonApiOptions + options?: JsonApiOptions & JsonApiWithNextFetchOptions ): Promise { options = { withAuth: true, From 864686c371e0dd8c412898b65b1fe9355fc98adc Mon Sep 17 00:00:00 2001 From: Brian Perry Date: Tue, 19 Nov 2024 09:56:22 -0600 Subject: [PATCH 3/4] feat: additional test coverage for resource methods --- .../tests/NextDrupal/resource-methods.test.ts | 142 ++++++++++++++++++ 1 file changed, 142 insertions(+) diff --git a/packages/next-drupal/tests/NextDrupal/resource-methods.test.ts b/packages/next-drupal/tests/NextDrupal/resource-methods.test.ts index ecdf251f..7d1867b2 100644 --- a/packages/next-drupal/tests/NextDrupal/resource-methods.test.ts +++ b/packages/next-drupal/tests/NextDrupal/resource-methods.test.ts @@ -281,6 +281,25 @@ describe("getMenu()", () => { }) ) }) + + test("makes request with cache option", async () => { + const drupal = new NextDrupal(BASE_URL) + const mockInit = { + cache: "no-store", + } as FetchOptions + + const fetchSpy = spyOnFetch() + + await drupal.getMenu("main", mockInit) + + expect(fetchSpy).toBeCalledTimes(1) + expect(fetchSpy).toBeCalledWith( + expect.anything(), + expect.objectContaining({ + ...mockInit, + }) + ) + }) }) describe("getResource()", () => { @@ -853,6 +872,32 @@ describe("getResourceByPath()", () => { }) ) }) + + test("makes request with cache option", async () => { + const drupal = new NextDrupal(BASE_URL) + const mockInit = { + cache: "no-store", + } as FetchOptions + + const fetchSpy = spyOnFetch({ + status: 207, + statusText: "Multi-Status", + responseBody: mocks.resources.subRequests.ok, + }) + + await drupal.getResourceByPath( + "/recipes/deep-mediterranean-quiche", + mockInit + ) + + expect(fetchSpy).toBeCalledTimes(1) + expect(fetchSpy).toBeCalledWith( + expect.anything(), + expect.objectContaining({ + ...mockInit, + }) + ) + }) }) describe("getResourceCollection()", () => { @@ -980,6 +1025,25 @@ describe("getResourceCollection()", () => { }) ) }) + + test("makes request with cache option", async () => { + const drupal = new NextDrupal(BASE_URL) + const mockInit = { + cache: "no-store", + } as FetchOptions + + const fetchSpy = spyOnFetch() + + await drupal.getResourceCollection("node--recipe", mockInit) + + expect(fetchSpy).toBeCalledTimes(1) + expect(fetchSpy).toBeCalledWith( + expect.anything(), + expect.objectContaining({ + ...mockInit, + }) + ) + }) }) describe("getResourceCollectionPathSegments()", () => { @@ -1139,6 +1203,27 @@ describe("getResourceCollectionPathSegments()", () => { }) ) }) + + test("makes request with cache option", async () => { + const drupal = new NextDrupal(BASE_URL) + const mockInit = { + cache: "no-store", + } as FetchOptions + + const fetchSpy = spyOnFetch({ + responseBody: { data: [] }, + }) + + await drupal.getResourceCollectionPathSegments("node--article", mockInit) + + expect(fetchSpy).toBeCalledTimes(1) + expect(fetchSpy).toBeCalledWith( + expect.anything(), + expect.objectContaining({ + ...mockInit, + }) + ) + }) }) describe("getSearchIndex()", () => { @@ -1269,6 +1354,25 @@ describe("getSearchIndex()", () => { }) ) }) + + test("makes request with cache option", async () => { + const drupal = new NextDrupal(BASE_URL) + const mockInit = { + cache: "no-store", + } as FetchOptions + + const fetchSpy = spyOnFetch() + + await drupal.getSearchIndex("recipes", mockInit) + + expect(fetchSpy).toBeCalledTimes(1) + expect(fetchSpy).toBeCalledWith( + expect.anything(), + expect.objectContaining({ + ...mockInit, + }) + ) + }) }) describe("getView()", () => { @@ -1392,6 +1496,25 @@ describe("getView()", () => { }) ) }) + + test("makes request with cache option", async () => { + const drupal = new NextDrupal(BASE_URL) + const mockInit = { + cache: "no-store", + } as FetchOptions + + const fetchSpy = spyOnFetch() + + await drupal.getView("featured_articles--page_1", mockInit) + + expect(fetchSpy).toBeCalledTimes(1) + expect(fetchSpy).toBeCalledWith( + expect.anything(), + expect.objectContaining({ + ...mockInit, + }) + ) + }) }) describe("translatePath()", () => { @@ -1501,4 +1624,23 @@ describe("translatePath()", () => { }) ) }) + + test("makes request with cache option", async () => { + const drupal = new NextDrupal(BASE_URL) + const mockInit = { + cache: "no-store", + } as FetchOptions + + const fetchSpy = spyOnFetch() + + await drupal.translatePath("recipes/deep-mediterranean-quiche", mockInit) + + expect(fetchSpy).toBeCalledTimes(1) + expect(fetchSpy).toBeCalledWith( + expect.anything(), + expect.objectContaining({ + ...mockInit, + }) + ) + }) }) From c41da4f1b740099ea63ad2c7e8ff0297a4b1f6a3 Mon Sep 17 00:00:00 2001 From: Brian Perry Date: Tue, 19 Nov 2024 10:05:59 -0600 Subject: [PATCH 4/4] feat: explicitly add force-cache option to getResource in basic starter --- starters/basic-starter/app/[...slug]/page.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/starters/basic-starter/app/[...slug]/page.tsx b/starters/basic-starter/app/[...slug]/page.tsx index 52b0189a..91a7e64e 100644 --- a/starters/basic-starter/app/[...slug]/page.tsx +++ b/starters/basic-starter/app/[...slug]/page.tsx @@ -35,6 +35,7 @@ async function getNode(slug: string[]) { const resource = await drupal.getResource(type, uuid, { params, + cache: "force-cache", next: { revalidate: 3600, // Replace `revalidate` with `tags` if using tag based revalidation.