review fix

Author: conico974

.changeset/spotty-chairs-pretend.md

@@ -3,3 +3,6 @@
 ---
 
 Add additional metadata to RoutingResult
+
+For some future features [#658](https://github.com/opennextjs/opennextjs-aws/issues/658) (and bug fix [#677](https://github.com/opennextjs/opennextjs-aws/issues/677)) we need to add some additional metadata to the RoutingResult. 
+This PR adds 2 new fields to the RoutingResult: `initialPath` and `resolvedRoutes`

packages/open-next/src/adapters/config/util.ts

@@ -89,15 +89,17 @@ export function loadAppPathsManifest(nextDir: string) {
   return JSON.parse(appPathsManifestJson) as Record<string, string>;
 }
 
-export function loadAppPathRoutesManifest(nextDir: string) {
+export function loadAppPathRoutesManifest(
+  nextDir: string,
+): Record<string, string> {
   const appPathRoutesManifestPath = path.join(
     nextDir,
     "app-path-routes-manifest.json",
   );
-  const appPathRoutesManifestJson = fs.existsSync(appPathRoutesManifestPath)
-    ? fs.readFileSync(appPathRoutesManifestPath, "utf-8")
-    : "{}";
-  return JSON.parse(appPathRoutesManifestJson) as Record<string, string>;
+  if (fs.existsSync(appPathRoutesManifestPath)) {
+    return JSON.parse(fs.readFileSync(appPathRoutesManifestPath, "utf-8"));
+  }
+  return {};
 }
 
 export function loadAppPathsManifestKeys(nextDir: string) {

packages/open-next/src/core/routing/routeMatcher.ts

@@ -3,60 +3,53 @@ import type { RouteDefinition } from "types/next-types";
 import type { RouteType } from "types/open-next";
 
 // Add the locale prefix to the regex so we correctly match the rawPath
-const optionalLocalePrefixRegex = RoutesManifest.locales.length
-  ? `^/(?:${RoutesManifest.locales.map((locale) => `${locale}/?`).join("|")})?`
-  : "^/";
+const optionalLocalePrefixRegex = `^/(?:${RoutesManifest.locales.map((locale) => `${locale}/?`).join("|")})?`;
 
 // Add the basepath prefix to the regex so we correctly match the rawPath
 const optionalBasepathPrefixRegex = RoutesManifest.basePath
   ? `^${RoutesManifest.basePath}/?`
   : "^/";
 
 // Add the basePath prefix to the api routes
-export const apiPrefix = RoutesManifest.basePath
-  ? `${RoutesManifest.basePath}/api`
-  : "/api";
+export const apiPrefix = `${RoutesManifest.basePath ?? ""}/api`;
+
+const optionalPrefix = optionalLocalePrefixRegex.replace(
+  "^/",
+  optionalBasepathPrefixRegex,
+);
 
 function routeMatcher(routeDefinitions: RouteDefinition[]) {
   const regexp = routeDefinitions.map((route) => ({
     page: route.page,
-    regexp: new RegExp(
-      route.regex
-        .replace("^/", optionalLocalePrefixRegex)
-        .replace("^/", optionalBasepathPrefixRegex),
-    ),
+    regexp: new RegExp(route.regex.replace("^/", optionalPrefix)),
   }));
 
+  const appPathsSet = new Set();
+  const routePathsSet = new Set();
   // We need to use AppPathRoutesManifest here
-  const appPathsSet = new Set(
-    Object.entries(AppPathRoutesManifest)
-      .filter(([key, _]) => key.endsWith("page"))
-      .map(([_, value]) => value),
-  );
-  const routePathsSet = new Set(
-    Object.entries(AppPathRoutesManifest)
-      .filter(([key, _]) => key.endsWith("route"))
-      .map(([_, value]) => value),
-  );
+  for (const [k, v] of Object.entries(AppPathRoutesManifest)) {
+    if (k.endsWith("page")) {
+      appPathsSet.add(v);
+    } else if (k.endsWith("route")) {
+      routePathsSet.add(v);
+    }
+  }
+
   return function matchRoute(path: string) {
     const foundRoutes = regexp.filter((route) => route.regexp.test(path));
 
-    if (foundRoutes.length > 0) {
-      return foundRoutes.map((foundRoute) => {
-        const routeType: RouteType | undefined = appPathsSet.has(
-          foundRoute.page,
-        )
-          ? "app"
-          : routePathsSet.has(foundRoute.page)
-            ? "route"
-            : "page";
-        return {
-          route: foundRoute.page,
-          type: routeType,
-        };
-      });
-    }
-    return false;
+    return foundRoutes.map((foundRoute) => {
+      let routeType: RouteType = "page";
+      if (appPathsSet.has(foundRoute.page)) {
+        routeType = "app";
+      } else if (routePathsSet.has(foundRoute.page)) {
+        routeType = "route";
+      }
+      return {
+        route: foundRoute.page,
+        type: routeType,
+      };
+    });
   };
 }
 

packages/open-next/src/core/routingHandler.ts

@@ -115,9 +115,9 @@ export default async function routingHandler(
     isExternalRewrite = beforeRewrites.isExternalRewrite;
   }
   const foundStaticRoute = staticRouteMatcher(internalEvent.rawPath);
-  const isStaticRoute = !isExternalRewrite && Boolean(foundStaticRoute);
+  const isStaticRoute = !isExternalRewrite && foundStaticRoute.length > 0;
 
-  if (!isStaticRoute && !isExternalRewrite) {
+  if (!(isStaticRoute || isExternalRewrite)) {
     // Second rewrite to be applied
     const afterRewrites = handleRewrites(
       internalEvent,
@@ -135,7 +135,7 @@ export default async function routingHandler(
   internalEvent = fallbackEvent;
 
   const foundDynamicRoute = dynamicRouteMatcher(internalEvent.rawPath);
-  const isDynamicRoute = !isExternalRewrite && Boolean(foundDynamicRoute);
+  const isDynamicRoute = !isExternalRewrite && foundDynamicRoute.length > 0;
 
   if (!(isDynamicRoute || isStaticRoute || isExternalRewrite)) {
     // Fallback rewrite to be applied
@@ -167,8 +167,8 @@ export default async function routingHandler(
       isApiRoute ||
       isNextImageRoute ||
       // We need to check again once all rewrites have been applied
-      staticRouteMatcher(internalEvent.rawPath) ||
-      dynamicRouteMatcher(internalEvent.rawPath)
+      staticRouteMatcher(internalEvent.rawPath).length > 0 ||
+      dynamicRouteMatcher(internalEvent.rawPath).length > 0
     )
   ) {
     internalEvent = {
@@ -209,11 +209,11 @@ export default async function routingHandler(
   });
 
   const resolvedRoutes: ResolvedRoute[] = [
-    ...(Array.isArray(foundStaticRoute) ? foundStaticRoute : []),
-    ...(Array.isArray(foundDynamicRoute) ? foundDynamicRoute : []),
+    ...foundStaticRoute,
+    ...foundDynamicRoute,
   ];
 
-  console.log("resolvedRoutes", resolvedRoutes);
+  debug("resolvedRoutes", resolvedRoutes);
 
   return {
     internalEvent,

packages/tests-unit/tests/core/routing/routeMatcher.test.ts

@@ -75,8 +75,8 @@ describe("routeMatcher", () => {
 
   describe("staticRouteMatcher", () => {
     it("should match static app route", () => {
-      const route = staticRouteMatcher("/app");
-      expect(route).toEqual([
+      const routes = staticRouteMatcher("/app");
+      expect(routes).toEqual([
         {
           route: "/app",
           type: "app",
@@ -85,8 +85,8 @@ describe("routeMatcher", () => {
     });
 
     it("should match static api route", () => {
-      const route = staticRouteMatcher("/api/app");
-      expect(route).toEqual([
+      const routes = staticRouteMatcher("/api/app");
+      expect(routes).toEqual([
         {
           route: "/api/app",
           type: "route",
@@ -95,25 +95,25 @@ describe("routeMatcher", () => {
     });
 
     it("should not match app dynamic route", () => {
-      const route = staticRouteMatcher("/catchAll/slug");
-      expect(route).toEqual(false);
+      const routes = staticRouteMatcher("/catchAll/slug");
+      expect(routes).toEqual([]);
     });
 
     it("should not match page dynamic route", () => {
-      const route = staticRouteMatcher("/page/catchAll/slug");
-      expect(route).toEqual(false);
+      const routes = staticRouteMatcher("/page/catchAll/slug");
+      expect(routes).toEqual([]);
     });
 
     it("should not match random route", () => {
-      const route = staticRouteMatcher("/random");
-      expect(route).toEqual(false);
+      const routes = staticRouteMatcher("/random");
+      expect(routes).toEqual([]);
     });
   });
 
   describe("dynamicRouteMatcher", () => {
     it("should match dynamic app page", () => {
-      const route = dynamicRouteMatcher("/catchAll/slug/b");
-      expect(route).toEqual([
+      const routes = dynamicRouteMatcher("/catchAll/slug/b");
+      expect(routes).toEqual([
         {
           route: "/catchAll/[...slug]",
           type: "app",
@@ -122,8 +122,8 @@ describe("routeMatcher", () => {
     });
 
     it("should match dynamic page router page", () => {
-      const route = dynamicRouteMatcher("/page/catchAll/slug/b");
-      expect(route).toEqual([
+      const routes = dynamicRouteMatcher("/page/catchAll/slug/b");
+      expect(routes).toEqual([
         {
           route: "/page/catchAll/[...slug]",
           type: "page",
@@ -134,13 +134,14 @@ describe("routeMatcher", () => {
     it("should match both the static and dynamic page", () => {
       const pathToMatch = "/page/catchAll/static";
       const dynamicRoutes = dynamicRouteMatcher(pathToMatch);
-      const staticRoutes = staticRouteMatcher(pathToMatch);
       expect(dynamicRoutes).toEqual([
         {
           route: "/page/catchAll/[...slug]",
           type: "page",
         },
       ]);
+
+      const staticRoutes = staticRouteMatcher(pathToMatch);
       expect(staticRoutes).toEqual([
         {
           route: "/page/catchAll/static",