fix: query params not appending with the same key (#380)

.changeset/neat-terms-boil.md

@@ -0,0 +1,5 @@
+---
+'@cloudflare/next-on-pages': patch
+---
+
+Fix multiple search params with the same key not being preserved

packages/next-on-pages/templates/_worker.js/utils/http.ts

@@ -45,7 +45,7 @@ export function isUrl(url: string): boolean {
 /**
  * Merges search params from one URLSearchParams object to another.
  *
- * Only updates the a parameter if the target does not contain it, or the source value is not empty.
+ * Only appends the parameter if the target does not contain it, or if the value is different and not undefined.
  *
  * For params prefixed with `nxtP`, it also sets the param without the prefix if it does not exist.
  * The `nxtP` prefix indicates that it is for Next.js dynamic route parameters. In some cases,
@@ -60,13 +60,15 @@ export function applySearchParams(
 	source: URLSearchParams,
 ) {
 	for (const [key, value] of source.entries()) {
-		if (!target.has(key) || !!value) {
+		const paramMatch = /^nxtP(.+)$/.exec(key);
+		if (paramMatch?.[1]) {
 			target.set(key, value);
-
-			const paramMatch = /^nxtP(.+)$/.exec(key);
-			if (paramMatch?.[1] && !target.has(paramMatch[1])) {
-				target.set(paramMatch[1], value);
-			}
+			target.set(paramMatch[1], value);
+		} else if (
+			!target.has(key) ||
+			(!!value && !target.getAll(key).includes(value))
+		) {
+			target.append(key, value);
 		}
 	}
 }

packages/next-on-pages/tests/templates/utils/http.test.ts

@@ -81,6 +81,61 @@ describe('applySearchParams', () => {
 			'http://localhost/page?other=value&foo=bar',
 		);
 	});
+
+	test('allows multiple query params with the same key', () => {
+		const source = new URL('http://localhost/page?foo=bar');
+		const target = new URL(
+			'http://localhost/page?other=value&foo=baz&foo=test',
+		);
+
+		expect([...source.searchParams.entries()].length).toEqual(1);
+		expect([...target.searchParams.entries()].length).toEqual(3);
+
+		applySearchParams(target.searchParams, source.searchParams);
+
+		expect([...source.searchParams.entries()].length).toEqual(1);
+		expect([...target.searchParams.entries()].length).toEqual(4);
+
+		expect(target.toString()).toEqual(
+			'http://localhost/page?other=value&foo=baz&foo=test&foo=bar',
+		);
+	});
+
+	test('multiple query params with the same key must be unique values', () => {
+		const source = new URL('http://localhost/page?foo=bar&foo=baz&foo=baz');
+		const target = new URL('http://localhost/page?other=value&foo=baz');
+
+		expect([...source.searchParams.entries()].length).toEqual(3);
+		expect([...target.searchParams.entries()].length).toEqual(2);
+
+		applySearchParams(target.searchParams, source.searchParams);
+
+		expect([...source.searchParams.entries()].length).toEqual(3);
+		expect([...target.searchParams.entries()].length).toEqual(3);
+
+		expect(target.toString()).toEqual(
+			'http://localhost/page?other=value&foo=baz&foo=bar',
+		);
+	});
+
+	test('Next.js page params (nxtP) always override', () => {
+		const source = new URL('http://localhost/page?nxtPfoo=bar');
+		const target = new URL(
+			'http://localhost/page?other=value&foo=baz&foo=test',
+		);
+
+		expect([...source.searchParams.entries()].length).toEqual(1);
+		expect([...target.searchParams.entries()].length).toEqual(3);
+
+		applySearchParams(target.searchParams, source.searchParams);
+
+		expect([...source.searchParams.entries()].length).toEqual(1);
+		expect([...target.searchParams.entries()].length).toEqual(3);
+
+		expect(target.toString()).toEqual(
+			'http://localhost/page?other=value&foo=bar&nxtPfoo=bar',
+		);
+	});
 });
 
 describe('createRouteRequest', () => {