change querystring format for multi value parameters (#319) (#320)

* change querystring format vor multi value parameters (#319)

Signed-off-by: Thomas Altmann <[email protected]>

* fix: move query string ternary to util function

Signed-off-by: Thomas Altmann <[email protected]>

* fix: unify query for internalEvent

Signed-off-by: Thomas Altmann <[email protected]>

---------

Signed-off-by: Thomas Altmann <[email protected]>

Author: thoaltmann

examples/app-router/app/search-query/page.tsx

@@ -3,14 +3,26 @@ import { headers } from "next/headers";
 export default function SearchQuery({
   searchParams: propsSearchParams,
 }: {
-  searchParams: Record<string, string>;
+  searchParams: Record<string, string | string[]>;
 }) {
   const mwSearchParams = headers().get("search-params");
+  const multiValueParams = propsSearchParams["multi"];
+  const multiValueArray = Array.isArray(multiValueParams)
+    ? multiValueParams
+    : [multiValueParams];
   return (
     <>
       <h1>Search Query</h1>
       <div>Search Params via Props: {propsSearchParams.searchParams}</div>
       <div>Search Params via Middleware: {mwSearchParams}</div>
+      {multiValueParams && (
+        <>
+          <div>Multi-value Params (key: multi): {multiValueArray.length}</div>
+          {multiValueArray.map((value) => (
+            <div>{value}</div>
+          ))}
+        </>
+      )}
     </>
   );
 }

packages/open-next/src/adapters/event-mapper.ts

@@ -9,6 +9,7 @@ import type {
 } from "aws-lambda";
 
 import { debug } from "./logger.js";
+import { convertToQuery } from "./routing/util.js";
 import { parseCookies } from "./util.js";
 
 export type InternalEvent = {
@@ -123,7 +124,7 @@ function convertFromAPIGatewayProxyEventV2(
     body: normalizeAPIGatewayProxyEventV2Body(event),
     headers: normalizeAPIGatewayProxyEventV2Headers(event),
     remoteAddress: requestContext.http.sourceIp,
-    query: removeUndefinedFromQuery(event.queryStringParameters ?? {}),
+    query: removeUndefinedFromQuery(convertToQuery(rawQueryString)),
     cookies:
       event.cookies?.reduce((acc, cur) => {
         const [key, value] = cur.split("=");
@@ -148,13 +149,7 @@ function convertFromCloudFrontRequestEvent(
     ),
     headers: normalizeCloudFrontRequestEventHeaders(headers),
     remoteAddress: clientIp,
-    query: querystring.split("&").reduce(
-      (acc, cur) => ({
-        ...acc,
-        [cur.split("=")[0]]: cur.split("=")[1],
-      }),
-      {},
-    ),
+    query: convertToQuery(querystring),
     cookies:
       headers.cookie?.reduce((acc, cur) => {
         const { key, value } = cur;

packages/open-next/src/adapters/routing/matcher.ts

@@ -11,7 +11,7 @@ import {
   RouteHas,
 } from "../types/next-types";
 import { escapeRegex, unescapeRegex } from "../util";
-import { convertQuery, getUrlParts, isExternal } from "./util";
+import { convertToQueryString, getUrlParts, isExternal } from "./util";
 
 const routeHasMatcher =
   (
@@ -127,7 +127,6 @@ export function handleRewrites<T extends RewriteDefinition>(
       checkHas(matcher, route.missing, true),
   );
 
-  const urlQueryString = new URLSearchParams(convertQuery(query)).toString();
   let rewrittenUrl = rawPath;
   const isExternalRewrite = isExternal(rewrite?.destination);
   debug("isExternalRewrite", isExternalRewrite);
@@ -154,13 +153,11 @@ export function handleRewrites<T extends RewriteDefinition>(
     }
   }
 
-  const queryString = urlQueryString ? `?${urlQueryString}` : "";
-
   return {
     internalEvent: {
       ...event,
       rawPath: rewrittenUrl,
-      url: `${rewrittenUrl}${queryString}`,
+      url: `${rewrittenUrl}${convertToQueryString(query)}`,
     },
     __rewrite: rewrite,
     isExternalRewrite,
@@ -233,18 +230,12 @@ export function fixDataPage(internalEvent: InternalEvent, buildId: string) {
     let newPath = rawPath.replace(dataPattern, "").replace(/\.json$/, "");
     newPath = newPath === "/index" ? "/" : newPath;
     query.__nextDataReq = "1";
-    const urlQuery: Record<string, string> = {};
-    Object.keys(query).forEach((k) => {
-      const v = query[k];
-      urlQuery[k] = Array.isArray(v) ? v.join(",") : v;
-    });
+
     return {
       ...internalEvent,
       rawPath: newPath,
       query,
-      url: `${newPath}${
-        query ? `?${new URLSearchParams(urlQuery).toString()}` : ""
-      }`,
+      url: `${newPath}${convertToQueryString(query)}`,
     };
   }
   return internalEvent;

packages/open-next/src/adapters/routing/middleware.ts

@@ -6,6 +6,7 @@ import { IncomingMessage } from "../http/request.js";
 import { ServerlessResponse } from "../http/response.js";
 import {
   convertRes,
+  convertToQueryString,
   getMiddlewareMatch,
   isExternal,
   loadMiddlewareManifest,
@@ -61,17 +62,11 @@ export async function handleMiddleware(
     path.join(NEXT_DIR, file),
   );
 
-  const urlQuery: Record<string, string> = {};
-  Object.keys(query).forEach((k) => {
-    const v = query[k];
-    urlQuery[k] = Array.isArray(v) ? v.join(",") : v;
-  });
-
   const host = req.headers.host
     ? `https://${req.headers.host}`
     : "http://localhost:3000";
   const initialUrl = new URL(rawPath, host);
-  initialUrl.search = new URLSearchParams(urlQuery).toString();
+  initialUrl.search = convertToQueryString(query);
   const url = initialUrl.toString();
 
   const result: MiddlewareResult = await run({

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

@@ -54,13 +54,38 @@ export function convertRes(res: ServerlessResponse) {
   };
 }
 
-export function convertQuery(query: Record<string, string | string[]>) {
-  const urlQuery: Record<string, string> = {};
-  Object.keys(query).forEach((k) => {
-    const v = query[k];
-    urlQuery[k] = Array.isArray(v) ? v.join(",") : v;
+/**
+ * Make sure that multi-value query parameters are transformed to
+ * ?key=value1&key=value2&... so that Next converts those parameters
+ * to an array when reading the query parameters
+ */
+export function convertToQueryString(query: Record<string, string | string[]>) {
+  const urlQuery = new URLSearchParams();
+  Object.entries(query).forEach(([key, value]) => {
+    if (Array.isArray(value)) {
+      value.forEach((entry) => urlQuery.append(key, entry));
+    } else {
+      urlQuery.append(key, value);
+    }
   });
-  return urlQuery;
+  const queryString = urlQuery.toString();
+
+  return queryString ? `?${queryString}` : "";
+}
+
+/**
+ * Given a raw query string, returns a record with key value-array pairs
+ * similar to how multiValueQueryStringParameters are structured
+ */
+export function convertToQuery(querystring: string) {
+  const query = new URLSearchParams(querystring);
+  const queryObject: Record<string, string[]> = {};
+
+  for (const key of query.keys()) {
+    queryObject[key] = query.getAll(key);
+  }
+
+  return queryObject;
 }
 
 export function getMiddlewareMatch(middlewareManifest: MiddlewareManifest) {

packages/tests-e2e/tests/appRouter/query.test.ts

@@ -4,10 +4,16 @@ import { expect, test } from "@playwright/test";
  * Tests that query params are available in middleware and RSC
  */
 test("SearchQuery", async ({ page }) => {
-  await page.goto("/search-query?searchParams=e2etest");
+  await page.goto("/search-query?searchParams=e2etest&multi=one&multi=two");
 
   let propsEl = page.getByText(`Search Params via Props: e2etest`);
   let mwEl = page.getByText(`Search Params via Middleware: mw/e2etest`);
+  let multiEl = page.getByText(`Multi-value Params (key: multi): 2`);
+  let multiOne = page.getByText(`one`);
+  let multiTwo = page.getByText(`two`);
   await expect(propsEl).toBeVisible();
   await expect(mwEl).toBeVisible();
+  await expect(multiEl).toBeVisible();
+  await expect(multiOne).toBeVisible();
+  await expect(multiTwo).toBeVisible();
 });

packages/tests-unit/tests/adapter.utils.test.ts

@@ -1,3 +1,7 @@
+import {
+  convertToQuery,
+  convertToQueryString,
+} from "../../open-next/src/adapters/routing/util";
 import { parseCookies } from "../../open-next/src/adapters/util";
 
 describe("adapter utils", () => {
@@ -46,4 +50,68 @@ describe("adapter utils", () => {
       ]);
     });
   });
+
+  describe("convertToQueryString", () => {
+    it("returns an empty string for no queries", () => {
+      const query = {};
+      expect(convertToQueryString(query)).toBe("");
+    });
+
+    it("converts a single entry to one querystring parameter", () => {
+      const query = { key: "value" };
+      expect(convertToQueryString(query)).toBe("?key=value");
+    });
+
+    it("converts multiple distinct entries to a querystring parameter each", () => {
+      const query = { key: "value", another: "value2" };
+      expect(convertToQueryString(query)).toBe("?key=value&another=value2");
+    });
+
+    it("converts multi-value parameters to multiple key value pairs", () => {
+      const query = { key: ["value1", "value2"] };
+      expect(convertToQueryString(query)).toBe("?key=value1&key=value2");
+    });
+
+    it("converts mixed multi-value and single value parameters", () => {
+      const query = { key: ["value1", "value2"], another: "value3" };
+      expect(convertToQueryString(query)).toBe(
+        "?key=value1&key=value2&another=value3",
+      );
+    });
+  });
+
+  describe("convertToQuery", () => {
+    it("returns an empty object for empty string", () => {
+      const querystring = "";
+      expect(convertToQuery(querystring)).toEqual({});
+    });
+
+    it("converts a single querystring parameter to one query entry", () => {
+      const querystring = "key=value";
+      expect(convertToQuery(querystring)).toEqual({ key: ["value"] });
+    });
+
+    it("converts multiple distinct entries to an entry in the query", () => {
+      const querystring = "key=value&another=value2";
+      expect(convertToQuery(querystring)).toEqual({
+        key: ["value"],
+        another: ["value2"],
+      });
+    });
+
+    it("converts multi-value parameters to an array in the query", () => {
+      const querystring = "key=value1&key=value2";
+      expect(convertToQuery(querystring)).toEqual({
+        key: ["value1", "value2"],
+      });
+    });
+
+    it("converts mixed multi-value and single value parameters", () => {
+      const querystring = "key=value1&key=value2&another=value3";
+      expect(convertToQuery(querystring)).toEqual({
+        key: ["value1", "value2"],
+        another: ["value3"],
+      });
+    });
+  });
 });

packages/tests-unit/tests/cache.test.ts

@@ -2,7 +2,11 @@ import { hasCacheExtension } from "../../open-next/src/adapters/cache";
 
 describe("hasCacheExtension", () => {
   it("Should returns true if has an extension and it is a CacheExtension", () => {
-    expect(hasCacheExtension("hello.json")).toBeTruthy();
+    expect(hasCacheExtension("hello.cache")).toBeTruthy();
+  });
+
+  it("Should returns false if has an extension and it is not a CacheExtension", () => {
+    expect(hasCacheExtension("hello.json")).toBeFalsy();
   });
 
   it("Should return false if does not have any extension", () => {