From c008911032c3a5ef0a371de1347ba2d5e6b2c428 Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Wed, 11 Dec 2024 17:58:12 +0100 Subject: [PATCH 1/8] add additional metadata to RoutingResult --- .../open-next/src/adapters/config/index.ts | 3 +- .../open-next/src/adapters/config/util.ts | 11 +- packages/open-next/src/adapters/middleware.ts | 12 +- packages/open-next/src/core/requestHandler.ts | 13 ++ packages/open-next/src/core/routingHandler.ts | 116 +++++++++++++----- packages/open-next/src/plugins/edge.ts | 3 + packages/open-next/src/types/open-next.ts | 11 ++ 7 files changed, 128 insertions(+), 41 deletions(-) diff --git a/packages/open-next/src/adapters/config/index.ts b/packages/open-next/src/adapters/config/index.ts index 6bea482d2..0a392bfab 100644 --- a/packages/open-next/src/adapters/config/index.ts +++ b/packages/open-next/src/adapters/config/index.ts @@ -2,6 +2,7 @@ import path from "node:path"; import { debug } from "../logger"; import { + loadAppPathsManifest, loadAppPathsManifestKeys, loadBuildId, loadConfig, @@ -9,7 +10,6 @@ import { loadHtmlPages, loadMiddlewareManifest, loadPrerenderManifest, - // loadPublicAssets, loadRoutesManifest, } from "./util.js"; @@ -31,3 +31,4 @@ export const AppPathsManifestKeys = /* @__PURE__ */ loadAppPathsManifestKeys(NEXT_DIR); export const MiddlewareManifest = /* @__PURE__ */ loadMiddlewareManifest(NEXT_DIR); +export const AppPathsManifest = loadAppPathsManifest(NEXT_DIR); diff --git a/packages/open-next/src/adapters/config/util.ts b/packages/open-next/src/adapters/config/util.ts index f1543df5a..43aa07f07 100644 --- a/packages/open-next/src/adapters/config/util.ts +++ b/packages/open-next/src/adapters/config/util.ts @@ -77,7 +77,7 @@ export function loadPrerenderManifest(nextDir: string) { return JSON.parse(json) as PrerenderManifest; } -export function loadAppPathsManifestKeys(nextDir: string) { +export function loadAppPathsManifest(nextDir: string) { const appPathsManifestPath = path.join( nextDir, "server", @@ -86,10 +86,11 @@ export function loadAppPathsManifestKeys(nextDir: string) { const appPathsManifestJson = fs.existsSync(appPathsManifestPath) ? fs.readFileSync(appPathsManifestPath, "utf-8") : "{}"; - const appPathsManifest = JSON.parse(appPathsManifestJson) as Record< - string, - string - >; + return JSON.parse(appPathsManifestJson) as Record; +} + +export function loadAppPathsManifestKeys(nextDir: string) { + const appPathsManifest = loadAppPathsManifest(nextDir); return Object.keys(appPathsManifest).map((key) => { // Remove parallel route let cleanedKey = key.replace(/\/@[^\/]+/g, ""); diff --git a/packages/open-next/src/adapters/middleware.ts b/packages/open-next/src/adapters/middleware.ts index bb9b7c2da..90fb54257 100644 --- a/packages/open-next/src/adapters/middleware.ts +++ b/packages/open-next/src/adapters/middleware.ts @@ -57,10 +57,19 @@ const defaultHandler = async ( ); return { type: "middleware", - internalEvent: result.internalEvent, + internalEvent: { + ...result.internalEvent, + headers: { + ...result.internalEvent.headers, + "x-opennext-initial-path": internalEvent.rawPath, + "x-opennext-resolved-route": result.resolvedRoute ?? "", + "x-opennext-route-type": result.routeType ?? "", + }, + }, isExternalRewrite: result.isExternalRewrite, origin, isISR: result.isISR, + initialPath: result.initialPath, }; } try { @@ -79,6 +88,7 @@ const defaultHandler = async ( isExternalRewrite: false, origin: false, isISR: result.isISR, + initialPath: result.internalEvent.rawPath, }; } } diff --git a/packages/open-next/src/core/requestHandler.ts b/packages/open-next/src/core/requestHandler.ts index 8d1c03944..268f16610 100644 --- a/packages/open-next/src/core/requestHandler.ts +++ b/packages/open-next/src/core/requestHandler.ts @@ -5,6 +5,7 @@ import { IncomingMessage } from "http/index.js"; import type { InternalEvent, InternalResult, + RouteType, RoutingResult, StreamCreator, } from "types/open-next"; @@ -14,6 +15,7 @@ import { debug, error, warn } from "../adapters/logger"; import { patchAsyncStorage } from "./patchAsyncStorage"; import { convertRes, createServerResponse } from "./routing/util"; import routingHandler, { + INTERNAL_HEADER_PREFIX, MIDDLEWARE_HEADER_PREFIX, MIDDLEWARE_HEADER_PREFIX_LEN, } from "./routingHandler"; @@ -42,6 +44,15 @@ export async function openNextHandler( isExternalRewrite: false, origin: false, isISR: false, + // These 3 will get overwritten by the routing handler if not using an external middleware + initialPath: + internalEvent.headers[`${INTERNAL_HEADER_PREFIX}-initial-path`] ?? + internalEvent.rawPath, + resolvedRoute: + internalEvent.headers[`${INTERNAL_HEADER_PREFIX}-resolved-route`], + routeType: internalEvent.headers[ + `${INTERNAL_HEADER_PREFIX}-route-type` + ] as RouteType | undefined, }; //#override withRouting @@ -94,6 +105,8 @@ export async function openNextHandler( isExternalRewrite: false, isISR: false, origin: false, + initialPath: internalEvent.rawPath, + resolvedRoute: "/500", }; } } diff --git a/packages/open-next/src/core/routingHandler.ts b/packages/open-next/src/core/routingHandler.ts index e7aec3f2e..c2321ddf1 100644 --- a/packages/open-next/src/core/routingHandler.ts +++ b/packages/open-next/src/core/routingHandler.ts @@ -1,12 +1,15 @@ import { + AppPathsManifest, BuildId, ConfigHeaders, PrerenderManifest, RoutesManifest, } from "config/index"; +import type { RouteDefinition } from "types/next-types"; import type { InternalEvent, InternalResult, + RouteType, RoutingResult, } from "types/open-next"; @@ -22,6 +25,7 @@ import { import { handleMiddleware } from "./routing/middleware"; export const MIDDLEWARE_HEADER_PREFIX = "x-middleware-response-"; +export const INTERNAL_HEADER_PREFIX = "x-opennext-"; export const MIDDLEWARE_HEADER_PREFIX_LEN = MIDDLEWARE_HEADER_PREFIX.length; // Add the locale prefix to the regex so we correctly match the rawPath @@ -39,23 +43,49 @@ const apiPrefix = RoutesManifest.basePath ? `${RoutesManifest.basePath}/api` : "/api"; -const staticRegexp = RoutesManifest.routes.static.map( - (route) => - new RegExp( - route.regex - .replace("^/", optionalLocalePrefixRegex) - .replace("^/", optionalBasepathPrefixRegex), - ), -); - -const dynamicRegexp = RoutesManifest.routes.dynamic.map( - (route) => - new RegExp( - route.regex - .replace("^/", optionalLocalePrefixRegex) - .replace("^/", optionalBasepathPrefixRegex), - ), -); +export function routeMatcher(routeDefinitions: RouteDefinition[]) { + const regexp = routeDefinitions.map((route) => { + return { + page: route.page, + regexp: new RegExp( + route.regex + .replace("^/", optionalLocalePrefixRegex) + .replace("^/", optionalBasepathPrefixRegex), + ), + }; + }); + const appPathsSet = new Set( + Object.keys(AppPathsManifest) + .filter((key) => key.endsWith("/page")) + // Remove the /page suffix + .map((key) => key.replace(/\/page$/, "")), + ); + const routePathsSet = new Set( + Object.keys(AppPathsManifest) + .filter((key) => key.endsWith("/route")) + // Remove the /route suffix + .map((key) => key.replace(/\/route$/, "")), + ); + return function matchRoute(path: string) { + const foundRoute = regexp.find((route) => route.regexp.test(path)); + let routeType: RouteType | undefined = undefined; + if (foundRoute) { + routeType = appPathsSet.has(foundRoute.page) + ? "app" + : routePathsSet.has(foundRoute.page) + ? "route" + : "page"; + return { + page: foundRoute.page, + routeType, + }; + } + return false; + }; +} + +const staticRouteMatcher = routeMatcher(RoutesManifest.routes.static); +const dynamicRouteMatcher = routeMatcher(RoutesManifest.routes.dynamic); // Geolocation headers starting from Nextjs 15 // See https://github.com/vercel/vercel/blob/7714b1c/packages/functions/src/headers.ts @@ -95,6 +125,17 @@ export default async function routingHandler( } } + // First we remove internal headers + // We don't want to allow users to set these headers + for (const key of Object.keys(event.headers)) { + if ( + key.startsWith(INTERNAL_HEADER_PREFIX) || + key.startsWith(MIDDLEWARE_HEADER_PREFIX) + ) { + delete event.headers[key]; + } + } + const nextHeaders = getNextConfigHeaders(event, ConfigHeaders); let internalEvent = fixDataPage(event, BuildId); @@ -127,12 +168,8 @@ export default async function routingHandler( internalEvent = beforeRewrites.internalEvent; isExternalRewrite = beforeRewrites.isExternalRewrite; } - - const isStaticRoute = - !isExternalRewrite && - staticRegexp.some((route) => - route.test((internalEvent as InternalEvent).rawPath), - ); + const foundStaticRoute = staticRouteMatcher(internalEvent.rawPath); + const isStaticRoute = !isExternalRewrite && Boolean(foundStaticRoute); if (!isStaticRoute && !isExternalRewrite) { // Second rewrite to be applied @@ -151,11 +188,9 @@ export default async function routingHandler( ); internalEvent = fallbackEvent; - const isDynamicRoute = - !isExternalRewrite && - dynamicRegexp.some((route) => - route.test((internalEvent as InternalEvent).rawPath), - ); + const foundDynamicRoute = dynamicRouteMatcher(internalEvent.rawPath); + const isDynamicRoute = !isExternalRewrite && Boolean(foundDynamicRoute); + if (!isDynamicRoute && !isStaticRoute && !isExternalRewrite) { // Fallback rewrite to be applied const fallbackRewrites = handleRewrites( @@ -185,12 +220,8 @@ export default async function routingHandler( !isApiRoute && !isNextImageRoute && // We need to check again once all rewrites have been applied - !staticRegexp.some((route) => - route.test((internalEvent as InternalEvent).rawPath), - ) && - !dynamicRegexp.some((route) => - route.test((internalEvent as InternalEvent).rawPath), - ) + !staticRouteMatcher(internalEvent.rawPath) && + !dynamicRouteMatcher(internalEvent.rawPath) ) { internalEvent = { ...internalEvent, @@ -229,10 +260,27 @@ export default async function routingHandler( ...nextHeaders, }); + const resolvedRoute = foundStaticRoute + ? foundStaticRoute.page + : foundDynamicRoute + ? foundDynamicRoute.page + : undefined; + + const routeType = foundStaticRoute + ? foundStaticRoute.routeType + : foundDynamicRoute + ? foundDynamicRoute.routeType + : // For /api paths we assume that they're route types + internalEvent.rawPath.startsWith("/api") + ? "route" + : undefined; return { internalEvent, isExternalRewrite, origin: false, isISR, + initialPath: event.rawPath, + resolvedRoute, + routeType, }; } diff --git a/packages/open-next/src/plugins/edge.ts b/packages/open-next/src/plugins/edge.ts index 83c71c18c..73d033885 100644 --- a/packages/open-next/src/plugins/edge.ts +++ b/packages/open-next/src/plugins/edge.ts @@ -6,6 +6,7 @@ import type { Plugin } from "esbuild"; import type { MiddlewareInfo } from "types/next-types.js"; import { + loadAppPathsManifest, loadAppPathsManifestKeys, loadBuildId, loadConfig, @@ -167,6 +168,7 @@ ${contents} const PrerenderManifest = loadPrerenderManifest(nextDir); const AppPathsManifestKeys = loadAppPathsManifestKeys(nextDir); const MiddlewareManifest = loadMiddlewareManifest(nextDir); + const AppPathsManifest = loadAppPathsManifest(nextDir); const contents = ` import path from "node:path"; @@ -188,6 +190,7 @@ ${contents} export const PrerenderManifest = ${JSON.stringify(PrerenderManifest)}; export const AppPathsManifestKeys = ${JSON.stringify(AppPathsManifestKeys)}; export const MiddlewareManifest = ${JSON.stringify(MiddlewareManifest)}; + export const AppPathsManifest = ${JSON.stringify(AppPathsManifest)}; process.env.NEXT_BUILD_ID = BuildId; `; diff --git a/packages/open-next/src/types/open-next.ts b/packages/open-next/src/types/open-next.ts index f24c318ac..3283ee066 100644 --- a/packages/open-next/src/types/open-next.ts +++ b/packages/open-next/src/types/open-next.ts @@ -107,11 +107,22 @@ export type IncludedConverter = | "sqs-revalidate" | "dummy"; +export type RouteType = "route" | "page" | "app"; + export interface RoutingResult { internalEvent: InternalEvent; + // If the request is an external rewrite, if used with an external middleware will be false on every server function isExternalRewrite: boolean; + // Origin is only used in external middleware, will be false on every server function origin: Origin | false; + // If the request is for an ISR route, will be false on every server function. Only used in external middleware isISR: boolean; + // The initial rawPath of the request before applying rewrites, if used with an external middleware will be defined in x-opennext-initial-path header + initialPath: string; + // The resolved route after applying rewrites, if used with an external middleware will be defined in x-opennext-resolved-route header + resolvedRoute?: string; + // The type of the resolved route, if used with an external middleware will be defined in x-opennext-route-type header + routeType?: RouteType; } export interface MiddlewareResult From ea9d3974a9714ba860aa1645d07b252f9504b804 Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Sat, 14 Dec 2024 17:12:57 +0100 Subject: [PATCH 2/8] review fix --- packages/open-next/src/adapters/middleware.ts | 12 +++-- packages/open-next/src/core/requestHandler.ts | 25 ++++++----- packages/open-next/src/core/routingHandler.ts | 45 ++++++++++--------- 3 files changed, 48 insertions(+), 34 deletions(-) diff --git a/packages/open-next/src/adapters/middleware.ts b/packages/open-next/src/adapters/middleware.ts index 90fb54257..f4578a345 100644 --- a/packages/open-next/src/adapters/middleware.ts +++ b/packages/open-next/src/adapters/middleware.ts @@ -14,7 +14,11 @@ import { resolveQueue, resolveTagCache, } from "../core/resolve"; -import routingHandler from "../core/routingHandler"; +import routingHandler, { + INTERNAL_HEADER_INITIAL_PATH, + INTERNAL_HEADER_RESOLVED_ROUTE, + INTERNAL_HEADER_ROUTE_TYPE, +} from "../core/routingHandler"; globalThis.internalFetch = fetch; globalThis.__openNextAls = new AsyncLocalStorage(); @@ -61,9 +65,9 @@ const defaultHandler = async ( ...result.internalEvent, headers: { ...result.internalEvent.headers, - "x-opennext-initial-path": internalEvent.rawPath, - "x-opennext-resolved-route": result.resolvedRoute ?? "", - "x-opennext-route-type": result.routeType ?? "", + [INTERNAL_HEADER_INITIAL_PATH]: internalEvent.rawPath, + [INTERNAL_HEADER_RESOLVED_ROUTE]: result.resolvedRoute ?? "", + [INTERNAL_HEADER_ROUTE_TYPE]: result.routeType ?? "", }, }, isExternalRewrite: result.isExternalRewrite, diff --git a/packages/open-next/src/core/requestHandler.ts b/packages/open-next/src/core/requestHandler.ts index 268f16610..28f35e621 100644 --- a/packages/open-next/src/core/requestHandler.ts +++ b/packages/open-next/src/core/requestHandler.ts @@ -15,7 +15,9 @@ import { debug, error, warn } from "../adapters/logger"; import { patchAsyncStorage } from "./patchAsyncStorage"; import { convertRes, createServerResponse } from "./routing/util"; import routingHandler, { - INTERNAL_HEADER_PREFIX, + INTERNAL_HEADER_INITIAL_PATH, + INTERNAL_HEADER_RESOLVED_ROUTE, + INTERNAL_HEADER_ROUTE_TYPE, MIDDLEWARE_HEADER_PREFIX, MIDDLEWARE_HEADER_PREFIX_LEN, } from "./routingHandler"; @@ -39,20 +41,23 @@ export async function openNextHandler( } debug("internalEvent", internalEvent); + // These 3 will get overwritten by the routing handler if not using an external middleware + const internalHeaders = { + initialPath: + internalEvent.headers[INTERNAL_HEADER_INITIAL_PATH] ?? + internalEvent.rawPath, + resolvedRoute: internalEvent.headers[INTERNAL_HEADER_RESOLVED_ROUTE], + routeType: internalEvent.headers[INTERNAL_HEADER_ROUTE_TYPE] as + | RouteType + | undefined, + }; + let routingResult: InternalResult | RoutingResult = { internalEvent, isExternalRewrite: false, origin: false, isISR: false, - // These 3 will get overwritten by the routing handler if not using an external middleware - initialPath: - internalEvent.headers[`${INTERNAL_HEADER_PREFIX}-initial-path`] ?? - internalEvent.rawPath, - resolvedRoute: - internalEvent.headers[`${INTERNAL_HEADER_PREFIX}-resolved-route`], - routeType: internalEvent.headers[ - `${INTERNAL_HEADER_PREFIX}-route-type` - ] as RouteType | undefined, + ...internalHeaders, }; //#override withRouting diff --git a/packages/open-next/src/core/routingHandler.ts b/packages/open-next/src/core/routingHandler.ts index c2321ddf1..a83bc505d 100644 --- a/packages/open-next/src/core/routingHandler.ts +++ b/packages/open-next/src/core/routingHandler.ts @@ -26,6 +26,9 @@ import { handleMiddleware } from "./routing/middleware"; export const MIDDLEWARE_HEADER_PREFIX = "x-middleware-response-"; export const INTERNAL_HEADER_PREFIX = "x-opennext-"; +export const INTERNAL_HEADER_INITIAL_PATH = `${INTERNAL_HEADER_PREFIX}initial-path`; +export const INTERNAL_HEADER_RESOLVED_ROUTE = `${INTERNAL_HEADER_PREFIX}resolved-route`; +export const INTERNAL_HEADER_ROUTE_TYPE = `${INTERNAL_HEADER_PREFIX}route-type`; export const MIDDLEWARE_HEADER_PREFIX_LEN = MIDDLEWARE_HEADER_PREFIX.length; // Add the locale prefix to the regex so we correctly match the rawPath @@ -191,7 +194,7 @@ export default async function routingHandler( const foundDynamicRoute = dynamicRouteMatcher(internalEvent.rawPath); const isDynamicRoute = !isExternalRewrite && Boolean(foundDynamicRoute); - if (!isDynamicRoute && !isStaticRoute && !isExternalRewrite) { + if (!(isDynamicRoute || isStaticRoute || isExternalRewrite)) { // Fallback rewrite to be applied const fallbackRewrites = handleRewrites( internalEvent, @@ -216,12 +219,14 @@ export default async function routingHandler( // If we still haven't found a route, we show the 404 page // We need to ensure that rewrites are applied before showing the 404 page if ( - !isRouteFoundBeforeAllRewrites && - !isApiRoute && - !isNextImageRoute && - // We need to check again once all rewrites have been applied - !staticRouteMatcher(internalEvent.rawPath) && - !dynamicRouteMatcher(internalEvent.rawPath) + !( + isRouteFoundBeforeAllRewrites || + isApiRoute || + isNextImageRoute || + // We need to check again once all rewrites have been applied + staticRouteMatcher(internalEvent.rawPath) || + dynamicRouteMatcher(internalEvent.rawPath) + ) ) { internalEvent = { ...internalEvent, @@ -260,20 +265,20 @@ export default async function routingHandler( ...nextHeaders, }); - const resolvedRoute = foundStaticRoute - ? foundStaticRoute.page - : foundDynamicRoute - ? foundDynamicRoute.page - : undefined; + let resolvedRoute: string | undefined; + let routeType: RouteType | undefined; + + if (foundStaticRoute) { + resolvedRoute = foundStaticRoute.page; + routeType = foundStaticRoute.routeType; + } else if (foundDynamicRoute) { + resolvedRoute = foundDynamicRoute.page; + routeType = foundDynamicRoute.routeType; + } else if (isApiRoute) { + // For /api paths we assume that they're route types + routeType = "route"; + } - const routeType = foundStaticRoute - ? foundStaticRoute.routeType - : foundDynamicRoute - ? foundDynamicRoute.routeType - : // For /api paths we assume that they're route types - internalEvent.rawPath.startsWith("/api") - ? "route" - : undefined; return { internalEvent, isExternalRewrite, From 0a6ee8d0ab7218a0cf0e3ba6044fe73196d22899 Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Tue, 17 Dec 2024 12:45:21 +0100 Subject: [PATCH 3/8] handle multiple route --- .../open-next/src/adapters/config/index.ts | 5 +- .../open-next/src/adapters/config/util.ts | 11 +++ packages/open-next/src/adapters/middleware.ts | 9 +-- packages/open-next/src/core/requestHandler.ts | 16 ++--- packages/open-next/src/core/routingHandler.ts | 70 +++++++++---------- packages/open-next/src/plugins/edge.ts | 4 ++ packages/open-next/src/types/open-next.ts | 12 ++-- 7 files changed, 72 insertions(+), 55 deletions(-) diff --git a/packages/open-next/src/adapters/config/index.ts b/packages/open-next/src/adapters/config/index.ts index 0a392bfab..12adfb939 100644 --- a/packages/open-next/src/adapters/config/index.ts +++ b/packages/open-next/src/adapters/config/index.ts @@ -2,6 +2,7 @@ import path from "node:path"; import { debug } from "../logger"; import { + loadAppPathRoutesManifest, loadAppPathsManifest, loadAppPathsManifestKeys, loadBuildId, @@ -31,4 +32,6 @@ export const AppPathsManifestKeys = /* @__PURE__ */ loadAppPathsManifestKeys(NEXT_DIR); export const MiddlewareManifest = /* @__PURE__ */ loadMiddlewareManifest(NEXT_DIR); -export const AppPathsManifest = loadAppPathsManifest(NEXT_DIR); +export const AppPathsManifest = /* @__PURE__ */ loadAppPathsManifest(NEXT_DIR); +export const AppPathRoutesManifest = + /* @__PURE__ */ loadAppPathRoutesManifest(NEXT_DIR); diff --git a/packages/open-next/src/adapters/config/util.ts b/packages/open-next/src/adapters/config/util.ts index 43aa07f07..095d37ae7 100644 --- a/packages/open-next/src/adapters/config/util.ts +++ b/packages/open-next/src/adapters/config/util.ts @@ -89,6 +89,17 @@ export function loadAppPathsManifest(nextDir: string) { return JSON.parse(appPathsManifestJson) as Record; } +export function loadAppPathRoutesManifest(nextDir: 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; +} + export function loadAppPathsManifestKeys(nextDir: string) { const appPathsManifest = loadAppPathsManifest(nextDir); return Object.keys(appPathsManifest).map((key) => { diff --git a/packages/open-next/src/adapters/middleware.ts b/packages/open-next/src/adapters/middleware.ts index f4578a345..363ce4b21 100644 --- a/packages/open-next/src/adapters/middleware.ts +++ b/packages/open-next/src/adapters/middleware.ts @@ -16,8 +16,7 @@ import { } from "../core/resolve"; import routingHandler, { INTERNAL_HEADER_INITIAL_PATH, - INTERNAL_HEADER_RESOLVED_ROUTE, - INTERNAL_HEADER_ROUTE_TYPE, + INTERNAL_HEADER_RESOLVED_ROUTES, } from "../core/routingHandler"; globalThis.internalFetch = fetch; @@ -66,14 +65,15 @@ const defaultHandler = async ( headers: { ...result.internalEvent.headers, [INTERNAL_HEADER_INITIAL_PATH]: internalEvent.rawPath, - [INTERNAL_HEADER_RESOLVED_ROUTE]: result.resolvedRoute ?? "", - [INTERNAL_HEADER_ROUTE_TYPE]: result.routeType ?? "", + [INTERNAL_HEADER_RESOLVED_ROUTES]: + JSON.stringify(result.resolvedRoutes) ?? "[]", }, }, isExternalRewrite: result.isExternalRewrite, origin, isISR: result.isISR, initialPath: result.initialPath, + resolvedRoutes: result.resolvedRoutes, }; } try { @@ -93,6 +93,7 @@ const defaultHandler = async ( origin: false, isISR: result.isISR, initialPath: result.internalEvent.rawPath, + resolvedRoutes: [{ route: "/500", type: "page" }], }; } } diff --git a/packages/open-next/src/core/requestHandler.ts b/packages/open-next/src/core/requestHandler.ts index 28f35e621..57aad21ad 100644 --- a/packages/open-next/src/core/requestHandler.ts +++ b/packages/open-next/src/core/requestHandler.ts @@ -5,7 +5,7 @@ import { IncomingMessage } from "http/index.js"; import type { InternalEvent, InternalResult, - RouteType, + ResolvedRoute, RoutingResult, StreamCreator, } from "types/open-next"; @@ -16,8 +16,7 @@ import { patchAsyncStorage } from "./patchAsyncStorage"; import { convertRes, createServerResponse } from "./routing/util"; import routingHandler, { INTERNAL_HEADER_INITIAL_PATH, - INTERNAL_HEADER_RESOLVED_ROUTE, - INTERNAL_HEADER_ROUTE_TYPE, + INTERNAL_HEADER_RESOLVED_ROUTES, MIDDLEWARE_HEADER_PREFIX, MIDDLEWARE_HEADER_PREFIX_LEN, } from "./routingHandler"; @@ -41,15 +40,14 @@ export async function openNextHandler( } debug("internalEvent", internalEvent); - // These 3 will get overwritten by the routing handler if not using an external middleware + // These 2 will get overwritten by the routing handler if not using an external middleware const internalHeaders = { initialPath: internalEvent.headers[INTERNAL_HEADER_INITIAL_PATH] ?? internalEvent.rawPath, - resolvedRoute: internalEvent.headers[INTERNAL_HEADER_RESOLVED_ROUTE], - routeType: internalEvent.headers[INTERNAL_HEADER_ROUTE_TYPE] as - | RouteType - | undefined, + resolvedRoutes: internalEvent.headers[INTERNAL_HEADER_RESOLVED_ROUTES] + ? JSON.parse(internalEvent.headers[INTERNAL_HEADER_RESOLVED_ROUTES]) + : ([] as ResolvedRoute[]), }; let routingResult: InternalResult | RoutingResult = { @@ -111,7 +109,7 @@ export async function openNextHandler( isISR: false, origin: false, initialPath: internalEvent.rawPath, - resolvedRoute: "/500", + resolvedRoutes: [{ route: "/500", type: "page" }], }; } } diff --git a/packages/open-next/src/core/routingHandler.ts b/packages/open-next/src/core/routingHandler.ts index a83bc505d..410f868f2 100644 --- a/packages/open-next/src/core/routingHandler.ts +++ b/packages/open-next/src/core/routingHandler.ts @@ -1,5 +1,5 @@ import { - AppPathsManifest, + AppPathRoutesManifest, BuildId, ConfigHeaders, PrerenderManifest, @@ -9,6 +9,7 @@ import type { RouteDefinition } from "types/next-types"; import type { InternalEvent, InternalResult, + ResolvedRoute, RouteType, RoutingResult, } from "types/open-next"; @@ -27,8 +28,7 @@ import { handleMiddleware } from "./routing/middleware"; export const MIDDLEWARE_HEADER_PREFIX = "x-middleware-response-"; export const INTERNAL_HEADER_PREFIX = "x-opennext-"; export const INTERNAL_HEADER_INITIAL_PATH = `${INTERNAL_HEADER_PREFIX}initial-path`; -export const INTERNAL_HEADER_RESOLVED_ROUTE = `${INTERNAL_HEADER_PREFIX}resolved-route`; -export const INTERNAL_HEADER_ROUTE_TYPE = `${INTERNAL_HEADER_PREFIX}route-type`; +export const INTERNAL_HEADER_RESOLVED_ROUTES = `${INTERNAL_HEADER_PREFIX}resolved-routes`; export const MIDDLEWARE_HEADER_PREFIX_LEN = MIDDLEWARE_HEADER_PREFIX.length; // Add the locale prefix to the regex so we correctly match the rawPath @@ -57,31 +57,35 @@ export function routeMatcher(routeDefinitions: RouteDefinition[]) { ), }; }); + + // We need to use AppPathRoutesManifest here const appPathsSet = new Set( - Object.keys(AppPathsManifest) - .filter((key) => key.endsWith("/page")) - // Remove the /page suffix - .map((key) => key.replace(/\/page$/, "")), + Object.entries(AppPathRoutesManifest) + .filter(([key, _]) => key.endsWith("page")) + .map(([_, value]) => value), ); const routePathsSet = new Set( - Object.keys(AppPathsManifest) - .filter((key) => key.endsWith("/route")) - // Remove the /route suffix - .map((key) => key.replace(/\/route$/, "")), + Object.entries(AppPathRoutesManifest) + .filter(([key, _]) => key.endsWith("route")) + .map(([_, value]) => value), ); return function matchRoute(path: string) { - const foundRoute = regexp.find((route) => route.regexp.test(path)); - let routeType: RouteType | undefined = undefined; - if (foundRoute) { - routeType = appPathsSet.has(foundRoute.page) - ? "app" - : routePathsSet.has(foundRoute.page) - ? "route" - : "page"; - return { - page: foundRoute.page, - routeType, - }; + 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; }; @@ -265,19 +269,12 @@ export default async function routingHandler( ...nextHeaders, }); - let resolvedRoute: string | undefined; - let routeType: RouteType | undefined; + const resolvedRoutes: ResolvedRoute[] = [ + ...(Array.isArray(foundStaticRoute) ? foundStaticRoute : []), + ...(Array.isArray(foundDynamicRoute) ? foundDynamicRoute : []), + ]; - if (foundStaticRoute) { - resolvedRoute = foundStaticRoute.page; - routeType = foundStaticRoute.routeType; - } else if (foundDynamicRoute) { - resolvedRoute = foundDynamicRoute.page; - routeType = foundDynamicRoute.routeType; - } else if (isApiRoute) { - // For /api paths we assume that they're route types - routeType = "route"; - } + console.log("resolvedRoutes", resolvedRoutes); return { internalEvent, @@ -285,7 +282,6 @@ export default async function routingHandler( origin: false, isISR, initialPath: event.rawPath, - resolvedRoute, - routeType, + resolvedRoutes, }; } diff --git a/packages/open-next/src/plugins/edge.ts b/packages/open-next/src/plugins/edge.ts index 73d033885..325c5f896 100644 --- a/packages/open-next/src/plugins/edge.ts +++ b/packages/open-next/src/plugins/edge.ts @@ -6,6 +6,7 @@ import type { Plugin } from "esbuild"; import type { MiddlewareInfo } from "types/next-types.js"; import { + loadAppPathRoutesManifest, loadAppPathsManifest, loadAppPathsManifestKeys, loadBuildId, @@ -169,6 +170,7 @@ ${contents} const AppPathsManifestKeys = loadAppPathsManifestKeys(nextDir); const MiddlewareManifest = loadMiddlewareManifest(nextDir); const AppPathsManifest = loadAppPathsManifest(nextDir); + const AppPathRoutesManifest = loadAppPathRoutesManifest(nextDir); const contents = ` import path from "node:path"; @@ -191,6 +193,8 @@ ${contents} export const AppPathsManifestKeys = ${JSON.stringify(AppPathsManifestKeys)}; export const MiddlewareManifest = ${JSON.stringify(MiddlewareManifest)}; export const AppPathsManifest = ${JSON.stringify(AppPathsManifest)}; + export const AppPathRoutesManifest = ${JSON.stringify(AppPathRoutesManifest)}; + process.env.NEXT_BUILD_ID = BuildId; `; diff --git a/packages/open-next/src/types/open-next.ts b/packages/open-next/src/types/open-next.ts index 3283ee066..74a6572b7 100644 --- a/packages/open-next/src/types/open-next.ts +++ b/packages/open-next/src/types/open-next.ts @@ -109,6 +109,11 @@ export type IncludedConverter = export type RouteType = "route" | "page" | "app"; +export interface ResolvedRoute { + route: string; + type: RouteType; +} + export interface RoutingResult { internalEvent: InternalEvent; // If the request is an external rewrite, if used with an external middleware will be false on every server function @@ -119,10 +124,9 @@ export interface RoutingResult { isISR: boolean; // The initial rawPath of the request before applying rewrites, if used with an external middleware will be defined in x-opennext-initial-path header initialPath: string; - // The resolved route after applying rewrites, if used with an external middleware will be defined in x-opennext-resolved-route header - resolvedRoute?: string; - // The type of the resolved route, if used with an external middleware will be defined in x-opennext-route-type header - routeType?: RouteType; + + // The resolved route after applying rewrites, if used with an external middleware will be defined in x-opennext-resolved-routes header as a json encoded array + resolvedRoutes: ResolvedRoute[]; } export interface MiddlewareResult From 189a87e1c39a23b6f2016abcf1af004143bd6401 Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Tue, 17 Dec 2024 13:26:07 +0100 Subject: [PATCH 4/8] add unit test --- .../src/core/routing/routeMatcher.ts | 66 ++++++++ packages/open-next/src/core/routingHandler.ts | 71 +------- .../tests/core/routing/routeMatcher.test.ts | 152 ++++++++++++++++++ 3 files changed, 223 insertions(+), 66 deletions(-) create mode 100644 packages/open-next/src/core/routing/routeMatcher.ts create mode 100644 packages/tests-unit/tests/core/routing/routeMatcher.test.ts diff --git a/packages/open-next/src/core/routing/routeMatcher.ts b/packages/open-next/src/core/routing/routeMatcher.ts new file mode 100644 index 000000000..fb1986d55 --- /dev/null +++ b/packages/open-next/src/core/routing/routeMatcher.ts @@ -0,0 +1,66 @@ +import { AppPathRoutesManifest, RoutesManifest } from "config/index"; +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("|")})?` + : "^/"; + +// 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"; + +function routeMatcher(routeDefinitions: RouteDefinition[]) { + const regexp = routeDefinitions.map((route) => { + return { + page: route.page, + regexp: new RegExp( + route.regex + .replace("^/", optionalLocalePrefixRegex) + .replace("^/", optionalBasepathPrefixRegex), + ), + }; + }); + + // 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), + ); + 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; + }; +} + +export const staticRouteMatcher = routeMatcher(RoutesManifest.routes.static); +export const dynamicRouteMatcher = routeMatcher(RoutesManifest.routes.dynamic); diff --git a/packages/open-next/src/core/routingHandler.ts b/packages/open-next/src/core/routingHandler.ts index 410f868f2..2325b02fc 100644 --- a/packages/open-next/src/core/routingHandler.ts +++ b/packages/open-next/src/core/routingHandler.ts @@ -1,16 +1,13 @@ import { - AppPathRoutesManifest, BuildId, ConfigHeaders, PrerenderManifest, RoutesManifest, } from "config/index"; -import type { RouteDefinition } from "types/next-types"; import type { InternalEvent, InternalResult, ResolvedRoute, - RouteType, RoutingResult, } from "types/open-next"; @@ -24,6 +21,11 @@ import { handleRewrites, } from "./routing/matcher"; import { handleMiddleware } from "./routing/middleware"; +import { + apiPrefix, + dynamicRouteMatcher, + staticRouteMatcher, +} from "./routing/routeMatcher"; export const MIDDLEWARE_HEADER_PREFIX = "x-middleware-response-"; export const INTERNAL_HEADER_PREFIX = "x-opennext-"; @@ -31,69 +33,6 @@ export const INTERNAL_HEADER_INITIAL_PATH = `${INTERNAL_HEADER_PREFIX}initial-pa export const INTERNAL_HEADER_RESOLVED_ROUTES = `${INTERNAL_HEADER_PREFIX}resolved-routes`; export const MIDDLEWARE_HEADER_PREFIX_LEN = MIDDLEWARE_HEADER_PREFIX.length; -// Add the locale prefix to the regex so we correctly match the rawPath -const optionalLocalePrefixRegex = RoutesManifest.locales.length - ? `^/(?:${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 -const apiPrefix = RoutesManifest.basePath - ? `${RoutesManifest.basePath}/api` - : "/api"; - -export function routeMatcher(routeDefinitions: RouteDefinition[]) { - const regexp = routeDefinitions.map((route) => { - return { - page: route.page, - regexp: new RegExp( - route.regex - .replace("^/", optionalLocalePrefixRegex) - .replace("^/", optionalBasepathPrefixRegex), - ), - }; - }); - - // 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), - ); - 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; - }; -} - -const staticRouteMatcher = routeMatcher(RoutesManifest.routes.static); -const dynamicRouteMatcher = routeMatcher(RoutesManifest.routes.dynamic); - // Geolocation headers starting from Nextjs 15 // See https://github.com/vercel/vercel/blob/7714b1c/packages/functions/src/headers.ts const geoHeaderToNextHeader = { diff --git a/packages/tests-unit/tests/core/routing/routeMatcher.test.ts b/packages/tests-unit/tests/core/routing/routeMatcher.test.ts new file mode 100644 index 000000000..640781536 --- /dev/null +++ b/packages/tests-unit/tests/core/routing/routeMatcher.test.ts @@ -0,0 +1,152 @@ +import { + staticRouteMatcher, + dynamicRouteMatcher, +} from "@opennextjs/aws/core/routing/routeMatcher.js"; +import { vi } from "vitest"; + +vi.mock("@opennextjs/aws/adapters/config/index.js", () => ({ + NextConfig: {}, + AppPathRoutesManifest: { + "/api/app/route": "/api/app", + "/app/page": "/app", + "/catchAll/[...slug]/page": "/catchAll/[...slug]", + }, + RoutesManifest: { + version: 3, + pages404: true, + caseSensitive: false, + basePath: "", + locales: [], + redirects: [], + headers: [], + routes: { + dynamic: [ + { + page: "/catchAll/[...slug]", + regex: "^/catchAll/(.+?)(?:/)?$", + routeKeys: { + nxtPslug: "nxtPslug", + }, + namedRegex: "^/catchAll/(?.+?)(?:/)?$", + }, + { + page: "/page/catchAll/[...slug]", + regex: "^/page/catchAll/(.+?)(?:/)?$", + routeKeys: { + nxtPslug: "nxtPslug", + }, + namedRegex: "^/page/catchAll/(?.+?)(?:/)?$", + }, + ], + static: [ + { + page: "/app", + regex: "^/app(?:/)?$", + routeKeys: {}, + namedRegex: "^/app(?:/)?$", + }, + { + page: "/api/app", + regex: "^/api/app(?:/)?$", + routeKeys: {}, + namedRegex: "^/api/app(?:/)?$", + }, + { + page: "/page", + regex: "^/page(?:/)?$", + routeKeys: {}, + namedRegex: "^/page(?:/)?$", + }, + { + page: "/page/catchAll/static", + regex: "^/page/catchAll/static(?:/)?$", + routeKeys: {}, + namedRegex: "^/page/catchAll/static(?:/)?$", + }, + ], + }, + }, +})); + +describe("routeMatcher", () => { + beforeEach(() => { + vi.resetAllMocks(); + }); + + describe("staticRouteMatcher", () => { + it("should match static app route", () => { + const route = staticRouteMatcher("/app"); + expect(route).toEqual([ + { + route: "/app", + type: "app", + }, + ]); + }); + + it("should match static api route", () => { + const route = staticRouteMatcher("/api/app"); + expect(route).toEqual([ + { + route: "/api/app", + type: "route", + }, + ]); + }); + + it("should not match app dynamic route", () => { + const route = staticRouteMatcher("/catchAll/slug"); + expect(route).toEqual(false); + }); + + it("should not match page dynamic route", () => { + const route = staticRouteMatcher("/page/catchAll/slug"); + expect(route).toEqual(false); + }); + + it("should not match random route", () => { + const route = staticRouteMatcher("/random"); + expect(route).toEqual(false); + }); + }); + + describe("dynamicRouteMatcher", () => { + it("should match dynamic app page", () => { + const route = dynamicRouteMatcher("/catchAll/slug/b"); + expect(route).toEqual([ + { + route: "/catchAll/[...slug]", + type: "app", + }, + ]); + }); + + it("should match dynamic page router page", () => { + const route = dynamicRouteMatcher("/page/catchAll/slug/b"); + expect(route).toEqual([ + { + route: "/page/catchAll/[...slug]", + type: "page", + }, + ]); + }); + + 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", + }, + ]); + expect(staticRoutes).toEqual([ + { + route: "/page/catchAll/static", + type: "page", + }, + ]); + }); + }); +}); From c737e11b771d7d1810aa09850e30730a783a47a1 Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Tue, 17 Dec 2024 13:31:10 +0100 Subject: [PATCH 5/8] review fix --- packages/open-next/src/core/requestHandler.ts | 14 +++++++------- .../open-next/src/core/routing/routeMatcher.ts | 18 ++++++++---------- packages/open-next/src/core/routingHandler.ts | 2 +- 3 files changed, 16 insertions(+), 18 deletions(-) diff --git a/packages/open-next/src/core/requestHandler.ts b/packages/open-next/src/core/requestHandler.ts index 57aad21ad..d2dc2c6f8 100644 --- a/packages/open-next/src/core/requestHandler.ts +++ b/packages/open-next/src/core/requestHandler.ts @@ -31,22 +31,22 @@ export async function openNextHandler( internalEvent: InternalEvent, responseStreaming?: StreamCreator, ): Promise { + const initialHeaders = internalEvent.headers; // We run everything in the async local storage context so that it is available in the middleware as well as in NextServer return runWithOpenNextRequestContext( - { isISRRevalidation: internalEvent.headers["x-isr"] === "1" }, + { isISRRevalidation: initialHeaders["x-isr"] === "1" }, async () => { - if (internalEvent.headers["x-forwarded-host"]) { - internalEvent.headers.host = internalEvent.headers["x-forwarded-host"]; + if (initialHeaders["x-forwarded-host"]) { + initialHeaders.host = initialHeaders["x-forwarded-host"]; } debug("internalEvent", internalEvent); // These 2 will get overwritten by the routing handler if not using an external middleware const internalHeaders = { initialPath: - internalEvent.headers[INTERNAL_HEADER_INITIAL_PATH] ?? - internalEvent.rawPath, - resolvedRoutes: internalEvent.headers[INTERNAL_HEADER_RESOLVED_ROUTES] - ? JSON.parse(internalEvent.headers[INTERNAL_HEADER_RESOLVED_ROUTES]) + initialHeaders[INTERNAL_HEADER_INITIAL_PATH] ?? internalEvent.rawPath, + resolvedRoutes: initialHeaders[INTERNAL_HEADER_RESOLVED_ROUTES] + ? JSON.parse(initialHeaders[INTERNAL_HEADER_RESOLVED_ROUTES]) : ([] as ResolvedRoute[]), }; diff --git a/packages/open-next/src/core/routing/routeMatcher.ts b/packages/open-next/src/core/routing/routeMatcher.ts index fb1986d55..d709c42f7 100644 --- a/packages/open-next/src/core/routing/routeMatcher.ts +++ b/packages/open-next/src/core/routing/routeMatcher.ts @@ -18,16 +18,14 @@ export const apiPrefix = RoutesManifest.basePath : "/api"; function routeMatcher(routeDefinitions: RouteDefinition[]) { - const regexp = routeDefinitions.map((route) => { - return { - page: route.page, - regexp: new RegExp( - route.regex - .replace("^/", optionalLocalePrefixRegex) - .replace("^/", optionalBasepathPrefixRegex), - ), - }; - }); + const regexp = routeDefinitions.map((route) => ({ + page: route.page, + regexp: new RegExp( + route.regex + .replace("^/", optionalLocalePrefixRegex) + .replace("^/", optionalBasepathPrefixRegex), + ), + })); // We need to use AppPathRoutesManifest here const appPathsSet = new Set( diff --git a/packages/open-next/src/core/routingHandler.ts b/packages/open-next/src/core/routingHandler.ts index 2325b02fc..8330880bc 100644 --- a/packages/open-next/src/core/routingHandler.ts +++ b/packages/open-next/src/core/routingHandler.ts @@ -28,10 +28,10 @@ import { } from "./routing/routeMatcher"; export const MIDDLEWARE_HEADER_PREFIX = "x-middleware-response-"; +export const MIDDLEWARE_HEADER_PREFIX_LEN = MIDDLEWARE_HEADER_PREFIX.length; export const INTERNAL_HEADER_PREFIX = "x-opennext-"; export const INTERNAL_HEADER_INITIAL_PATH = `${INTERNAL_HEADER_PREFIX}initial-path`; export const INTERNAL_HEADER_RESOLVED_ROUTES = `${INTERNAL_HEADER_PREFIX}resolved-routes`; -export const MIDDLEWARE_HEADER_PREFIX_LEN = MIDDLEWARE_HEADER_PREFIX.length; // Geolocation headers starting from Nextjs 15 // See https://github.com/vercel/vercel/blob/7714b1c/packages/functions/src/headers.ts From 4f46657587c89b1dfacbdf751d3b63f988e7335f Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Tue, 17 Dec 2024 13:39:53 +0100 Subject: [PATCH 6/8] changeset --- .changeset/spotty-chairs-pretend.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/spotty-chairs-pretend.md diff --git a/.changeset/spotty-chairs-pretend.md b/.changeset/spotty-chairs-pretend.md new file mode 100644 index 000000000..d22e1b751 --- /dev/null +++ b/.changeset/spotty-chairs-pretend.md @@ -0,0 +1,5 @@ +--- +"@opennextjs/aws": patch +--- + +Add additional metadata to RoutingResult From 1b0121072b8c49ead10585985a91c7af53cb5969 Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Tue, 17 Dec 2024 13:41:49 +0100 Subject: [PATCH 7/8] fix lint --- packages/tests-unit/tests/core/routing/routeMatcher.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/tests-unit/tests/core/routing/routeMatcher.test.ts b/packages/tests-unit/tests/core/routing/routeMatcher.test.ts index 640781536..5dcfa9e0b 100644 --- a/packages/tests-unit/tests/core/routing/routeMatcher.test.ts +++ b/packages/tests-unit/tests/core/routing/routeMatcher.test.ts @@ -1,6 +1,6 @@ import { - staticRouteMatcher, dynamicRouteMatcher, + staticRouteMatcher, } from "@opennextjs/aws/core/routing/routeMatcher.js"; import { vi } from "vitest"; From 0be846158a628f4981465c5476ac51e6eb05edc8 Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Tue, 17 Dec 2024 15:14:16 +0100 Subject: [PATCH 8/8] review fix --- .changeset/spotty-chairs-pretend.md | 3 + .../open-next/src/adapters/config/util.ts | 12 ++-- .../src/core/routing/routeMatcher.ts | 67 +++++++++---------- packages/open-next/src/core/routingHandler.ts | 16 ++--- .../tests/core/routing/routeMatcher.test.ts | 31 ++++----- 5 files changed, 64 insertions(+), 65 deletions(-) diff --git a/.changeset/spotty-chairs-pretend.md b/.changeset/spotty-chairs-pretend.md index d22e1b751..3613e4ed3 100644 --- a/.changeset/spotty-chairs-pretend.md +++ b/.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` \ No newline at end of file diff --git a/packages/open-next/src/adapters/config/util.ts b/packages/open-next/src/adapters/config/util.ts index 095d37ae7..501a76501 100644 --- a/packages/open-next/src/adapters/config/util.ts +++ b/packages/open-next/src/adapters/config/util.ts @@ -89,15 +89,17 @@ export function loadAppPathsManifest(nextDir: string) { return JSON.parse(appPathsManifestJson) as Record; } -export function loadAppPathRoutesManifest(nextDir: string) { +export function loadAppPathRoutesManifest( + nextDir: string, +): Record { 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; + if (fs.existsSync(appPathRoutesManifestPath)) { + return JSON.parse(fs.readFileSync(appPathRoutesManifestPath, "utf-8")); + } + return {}; } export function loadAppPathsManifestKeys(nextDir: string) { diff --git a/packages/open-next/src/core/routing/routeMatcher.ts b/packages/open-next/src/core/routing/routeMatcher.ts index d709c42f7..c9568ada2 100644 --- a/packages/open-next/src/core/routing/routeMatcher.ts +++ b/packages/open-next/src/core/routing/routeMatcher.ts @@ -3,9 +3,7 @@ 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 @@ -13,50 +11,45 @@ const optionalBasepathPrefixRegex = 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, + }; + }); }; } diff --git a/packages/open-next/src/core/routingHandler.ts b/packages/open-next/src/core/routingHandler.ts index 8330880bc..b08966b63 100644 --- a/packages/open-next/src/core/routingHandler.ts +++ b/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, diff --git a/packages/tests-unit/tests/core/routing/routeMatcher.test.ts b/packages/tests-unit/tests/core/routing/routeMatcher.test.ts index 5dcfa9e0b..e47e66d25 100644 --- a/packages/tests-unit/tests/core/routing/routeMatcher.test.ts +++ b/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",