Skip to content

Commit 068ce66

Browse files
authored
Fix rewrite query not forwarded and api destination (#767)
* fix issue * add test * changeset * fix unit test
1 parent 9748fcf commit 068ce66

File tree

6 files changed

+41
-1
lines changed

6 files changed

+41
-1
lines changed

.changeset/red-starfishes-wave.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@opennextjs/aws": patch
3+
---
4+
5+
Fix api rewrite destination with i18n and query not forwarded on rewrite

examples/pages-router/next.config.ts

+1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ const nextConfig: NextConfig = {
2626
],
2727
rewrites: async () => [
2828
{ source: "/rewrite", destination: "/", locale: false },
29+
{ source: "/rewriteWithQuery", destination: "/api/query?q=1" },
2930
{
3031
source: "/rewriteUsingQuery",
3132
destination: "/:destination/",
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import type { NextApiRequest, NextApiResponse } from "next";
2+
3+
export default function handler(req: NextApiRequest, res: NextApiResponse) {
4+
res.status(200).json({ query: req.query });
5+
}

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

+14-1
Original file line numberDiff line numberDiff line change
@@ -227,11 +227,23 @@ export function handleRewrites<T extends RewriteDefinition>(
227227
rewrittenHost = unescapeRegex(toDestinationHost(params));
228228
rewrittenQuery = unescapeRegex(toDestinationQuery(params));
229229
}
230+
231+
// We need to strip the locale from the path if it's a local api route
232+
if (NextConfig.i18n && !isExternalRewrite) {
233+
const strippedPathLocale = rewrittenPath.replace(
234+
new RegExp(`^/(${NextConfig.i18n.locales.join("|")})`),
235+
"",
236+
);
237+
if (strippedPathLocale.startsWith("/api/")) {
238+
rewrittenPath = strippedPathLocale;
239+
}
240+
}
241+
230242
rewrittenUrl = isExternalRewrite
231243
? `${protocol}//${rewrittenHost}${rewrittenPath}`
232244
: new URL(rewrittenPath, event.url).href;
233245

234-
// Should we merge the query params or use only the ones from the rewrite?
246+
// We merge query params from the source and the destination
235247
finalQuery = {
236248
...query,
237249
...convertFromQueryString(rewrittenQuery),
@@ -243,6 +255,7 @@ export function handleRewrites<T extends RewriteDefinition>(
243255
return {
244256
internalEvent: {
245257
...event,
258+
query: finalQuery,
246259
rawPath: new URL(rewrittenUrl).pathname,
247260
url: rewrittenUrl,
248261
},

packages/tests-e2e/tests/pagesRouter/rewrite.test.ts

+12
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,15 @@ test("Rewrite to external image", async ({ request }) => {
2323
expect(response.headers()["content-type"]).toBe("image/png");
2424
expect(validateMd5(await response.body(), EXT_PNG_MD5)).toBe(true);
2525
});
26+
27+
test("Rewrite with query in destination", async ({ request }) => {
28+
const response = await request.get("/rewriteWithQuery");
29+
expect(response.status()).toBe(200);
30+
expect(await response.json()).toEqual({ query: { q: "1" } });
31+
});
32+
33+
test("Rewrite with query should merge query params", async ({ request }) => {
34+
const response = await request.get("/rewriteWithQuery?b=2");
35+
expect(response.status()).toBe(200);
36+
expect(await response.json()).toEqual({ query: { q: "1", b: "2" } });
37+
});

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

+4
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,10 @@ describe("handleRewrites", () => {
395395
expect(result).toEqual({
396396
internalEvent: {
397397
...event,
398+
query: {
399+
album: "foo",
400+
song: "bar",
401+
},
398402
rawPath: "/search",
399403
url: "https://external.com/search?album=foo&song=bar",
400404
},

0 commit comments

Comments
 (0)