update to use initialPath when needed

opennextjs · Dec 27, 2024 · f6a29a9 · f6a29a9
1 parent 67a6102
commit f6a29a9
Show file tree

Hide file tree

Showing 6 changed files with 89 additions and 44 deletions.
diff --git a/packages/open-next/src/adapters/cache.ts b/packages/open-next/src/adapters/cache.ts
@@ -447,9 +447,15 @@ export default class Cache {
         if (uniquePaths.length > 0) {
           await globalThis.cdnInvalidationHandler.invalidatePaths(
             uniquePaths.map((path) => ({
-              path,
-              // Here we can be sure that the path is from app router
-              isAppRouter: true,
+              initialPath: path,
+              rawPath: path,
+              resolvedRoutes: [
+                {
+                  route: path,
+                  // TODO: ideally here we should check if it's an app router page or route
+                  type: "app",
+                },
+              ],
             })),
           );
         }

diff --git a/packages/open-next/src/core/requestHandler.ts b/packages/open-next/src/core/requestHandler.ts
@@ -118,7 +118,14 @@ export async function openNextHandler(
         // response is used only in the streaming case
         if (responseStreaming) {
           const response = createServerResponse(
-            internalEvent,
+            {
+              internalEvent,
+              isExternalRewrite: false,
+              isISR: false,
+              resolvedRoutes: [],
+              origin: false,
+              initialPath: internalEvent.rawPath,
+            },
             headers,
             responseStreaming,
           );
@@ -162,7 +169,7 @@ export async function openNextHandler(
 
       const req = new IncomingMessage(reqProps);
       const res = createServerResponse(
-        preprocessedEvent,
+        routingResult,
         overwrittenResponseHeaders,
         responseStreaming,
       );

diff --git a/packages/open-next/src/core/routing/util.ts b/packages/open-next/src/core/routing/util.ts
@@ -8,8 +8,8 @@ import { OpenNextNodeResponse } from "http/openNextResponse.js";
 import { parseHeaders } from "http/util.js";
 import type { MiddlewareManifest } from "types/next-types";
 import type {
-  InternalEvent,
   InternalResult,
+  RoutingResult,
   StreamCreator,
 } from "types/open-next.js";
 
@@ -401,10 +401,11 @@ export function fixISRHeaders(headers: OutgoingHttpHeaders) {
  * @__PURE__
  */
 export function createServerResponse(
-  internalEvent: InternalEvent,
+  routingResult: RoutingResult,
   headers: Record<string, string | string[] | undefined>,
   responseStream?: StreamCreator,
 ) {
+  const internalEvent = routingResult.internalEvent;
   return new OpenNextNodeResponse(
     (_headers) => {
       fixCacheHeaderForHtmlPages(internalEvent.rawPath, _headers);
@@ -418,34 +419,29 @@ export function createServerResponse(
         internalEvent.rawPath,
         _headers,
       );
-      await invalidateCDNOnRequest({
-        rawPath: internalEvent.rawPath,
-        isIsrRevalidation: internalEvent.headers["x-isr"] === "1",
-        headers: _headers,
-      });
+      await invalidateCDNOnRequest(routingResult, _headers);
     },
     responseStream,
     headers,
   );
 }
 
 // This function is used only for `res.revalidate()`
-export async function invalidateCDNOnRequest(params: {
-  //TODO: use the initialPath instead of rawPath, a rewrite could have happened and would make cdn invalidation fail
-  rawPath: string;
-  isIsrRevalidation?: boolean;
-  headers: OutgoingHttpHeaders;
-}) {
-  const { rawPath, isIsrRevalidation, headers } = params;
+export async function invalidateCDNOnRequest(
+  params: RoutingResult,
+  headers: OutgoingHttpHeaders,
+) {
+  const { internalEvent, initialPath, resolvedRoutes } = params;
+  const isIsrRevalidation = internalEvent.headers["x-isr"] === "1";
   if (
     !isIsrRevalidation &&
     headers[CommonHeaders.NEXT_CACHE] === "REVALIDATED"
   ) {
     await globalThis.cdnInvalidationHandler.invalidatePaths([
       {
-        path: rawPath,
-        //TODO: Here we assume that the path is for page router, this might not be the case
-        isAppRouter: false,
+        initialPath,
+        rawPath: internalEvent.rawPath,
+        resolvedRoutes,
       },
     ]);
   }

diff --git a/packages/open-next/src/overrides/cdnInvalidation/cloudfront.ts b/packages/open-next/src/overrides/cdnInvalidation/cloudfront.ts
@@ -8,14 +8,20 @@ const cloudfront = new CloudFrontClient({});
 export default {
   name: "cloudfront",
   invalidatePaths: async (paths) => {
-    //TODO: test the constructed paths
-    const constructedPaths = paths.flatMap(({ path, isAppRouter }) =>
-      isAppRouter
-        ? [`${path}`, `${path}?_rsc=*`]
-        : [
-            `${path}`,
-            `/_next/data/${process.env.NEXT_BUILD_ID}${path === "/" ? "/index" : path}.json*`,
-          ],
+    const constructedPaths = paths.flatMap(
+      ({ initialPath, resolvedRoutes }) => {
+        const isAppRouter = resolvedRoutes.some(
+          (route) => route.type === "app",
+        );
+        // revalidateTag doesn't have any leading slash, remove it just to be sure
+        const path = initialPath.replace(/^\//, "");
+        return isAppRouter
+          ? [`/${path}`, `/${path}?_rsc=*`]
+          : [
+              `/${path}`,
+              `/_next/data/${process.env.NEXT_BUILD_ID}${path === "/" ? "/index" : `/${path}`}.json*`,
+            ];
+      },
     );
     await cloudfront.send(
       new CreateInvalidationCommand({

diff --git a/packages/open-next/src/types/overrides.ts b/packages/open-next/src/types/overrides.ts
@@ -8,6 +8,7 @@ import type {
   InternalEvent,
   InternalResult,
   Origin,
+  ResolvedRoute,
   StreamCreator,
 } from "./open-next";
 
@@ -141,8 +142,9 @@ export type ProxyExternalRequest = BaseOverride & {
 };
 
 type CDNPath = {
-  path: string;
-  isAppRouter: boolean;
+  initialPath: string;
+  rawPath: string;
+  resolvedRoutes: ResolvedRoute[];
 };
 
 export type CDNInvalidationHandler = BaseOverride & {

diff --git a/packages/tests-unit/tests/core/routing/util.test.ts b/packages/tests-unit/tests/core/routing/util.test.ts
@@ -687,10 +687,14 @@ describe("invalidateCDNOnRequest", () => {
     const headers: Record<string, string> = {
       "x-nextjs-cache": "HIT",
     };
-    await invalidateCDNOnRequest({
-      rawPath: "/path",
+    await invalidateCDNOnRequest(
+      {
+        internalEvent: {
+          headers: {},
+        },
+      },
       headers,
-    });
+    );
 
     expect(
       globalThis.cdnInvalidationHandler.invalidatePaths,
@@ -701,11 +705,17 @@ describe("invalidateCDNOnRequest", () => {
     const headers: Record<string, string> = {
       "x-nextjs-cache": "REVALIDATED",
     };
-    await invalidateCDNOnRequest({
-      rawPath: "/path",
+    await invalidateCDNOnRequest(
+      {
+        internalEvent: {
+          rawPath: "/path",
+          headers: {
+            "x-isr": "1",
+          },
+        },
+      },
       headers,
-      isIsrRevalidation: true,
-    });
+    );
 
     expect(
       globalThis.cdnInvalidationHandler.invalidatePaths,
@@ -716,17 +726,35 @@ describe("invalidateCDNOnRequest", () => {
     const headers: Record<string, string> = {
       "x-nextjs-cache": "REVALIDATED",
     };
-    await invalidateCDNOnRequest({
-      rawPath: "/path",
+    await invalidateCDNOnRequest(
+      {
+        initialPath: "/path",
+        internalEvent: {
+          rawPath: "/path",
+          headers: {},
+        },
+        resolvedRoutes: [
+          {
+            type: "app",
+            route: "/path",
+          },
+        ],
+      },
       headers,
-    });
+    );
 
     expect(
       globalThis.cdnInvalidationHandler.invalidatePaths,
     ).toHaveBeenCalledWith([
       {
-        path: "/path",
-        isAppRouter: false,
+        initialPath: "/path",
+        rawPath: "/path",
+        resolvedRoutes: [
+          {
+            type: "app",
+            route: "/path",
+          },
+        ],
       },
     ]);
   });