Skip to content
This repository was archived by the owner on Jan 28, 2025. It is now read-only.

Commit 27d1943

Browse files
authored
fix(core): support no-op rewrites (#1329)
1 parent df2b703 commit 27d1943

File tree

5 files changed

+68
-6
lines changed

5 files changed

+68
-6
lines changed

packages/e2e-tests/next-app/cypress/integration/rewrites.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,11 @@ describe("Rewrites Tests", () => {
7070
path: "/rewrite-dest-with-query?a=b",
7171
expectedRewrite: "/ssr-page?a=b&foo=bar",
7272
expectedStatus: 200
73+
},
74+
{
75+
path: "/no-op-rewrite",
76+
expectedRewrite: "/ssr-page",
77+
expectedStatus: 200
7378
}
7479
].forEach(({ path, expectedRewrite, expectedStatus }) => {
7580
it(`rewrites path ${path} to ${expectedRewrite}`, () => {

packages/e2e-tests/next-app/next.config.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,14 @@ module.exports = {
146146
source: "/api/external-rewrite-issues-with-query",
147147
destination:
148148
"https://api.github.com/repos/serverless-nextjs/serverless-next.js/issues?a=b"
149+
},
150+
{
151+
source: "/no-op-rewrite",
152+
destination: "/no-op-rewrite"
153+
},
154+
{
155+
source: "/no-op-rewrite",
156+
destination: "/ssr-page"
149157
}
150158
];
151159
},

packages/libs/core/src/route/page.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ export const handlePageReq = (
6464
};
6565
}
6666

67-
const rewrite = !isRewrite && getRewritePath(uri, routesManifest);
67+
const rewrite = !isRewrite && getRewritePath(uri, routesManifest, manifest);
6868
if (rewrite) {
6969
const [path, querystring] = rewrite.split("?");
7070
if (isExternalRewrite(path)) {

packages/libs/core/src/route/rewrite.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,18 @@
11
import { addDefaultLocaleToPath } from "./locale";
22
import { compileDestination, matchPath } from "../match";
3-
import { RewriteData, RoutesManifest } from "../types";
3+
import { PageManifest, RewriteData, RoutesManifest } from "../types";
4+
import { handlePageReq } from "../route/page";
45

56
/**
67
* Get the rewrite of the given path, if it exists.
7-
* @param path
8+
* @param uri
9+
* @param pageManifest
810
* @param routesManifest
911
*/
1012
export function getRewritePath(
1113
uri: string,
12-
routesManifest: RoutesManifest
14+
routesManifest: RoutesManifest,
15+
pageManifest?: PageManifest
1316
): string | undefined {
1417
const path = addDefaultLocaleToPath(uri, routesManifest);
1518

@@ -27,6 +30,21 @@ export function getRewritePath(
2730
return;
2831
}
2932

33+
// No-op rewrite support for pages: skip to next rewrite if path does not map to existing non-dynamic and dynamic routes
34+
if (pageManifest && path === destination) {
35+
const url = handlePageReq(
36+
destination,
37+
pageManifest,
38+
routesManifest,
39+
false,
40+
true
41+
);
42+
43+
if (url.statusCode === 404) {
44+
continue;
45+
}
46+
}
47+
3048
// Pass unused params to destination
3149
// except nextInternalLocale param since it's already in path prefix
3250
const querystring = Object.keys(params)

packages/libs/core/tests/route/rewrite.test.ts

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import { getRewritePath, isExternalRewrite } from "../../src/route/rewrite";
2-
import { RoutesManifest } from "../../src/types";
2+
import { PageManifest, RoutesManifest } from "../../src/types";
33

44
describe("Rewriter Tests", () => {
55
describe("getRewritePath()", () => {
66
let routesManifest: RoutesManifest;
7+
let pageManifest: PageManifest;
78

89
beforeAll(() => {
910
routesManifest = {
@@ -47,10 +48,39 @@ describe("Rewriter Tests", () => {
4748
{
4849
source: "/multi-query/:path*",
4950
destination: "/target"
51+
},
52+
{
53+
source: "/no-op-rewrite",
54+
destination: "/no-op-rewrite"
55+
},
56+
{
57+
source: "/no-op-rewrite",
58+
destination: "/actual-no-op-rewrite-destination"
5059
}
5160
],
5261
redirects: []
5362
};
63+
64+
pageManifest = {
65+
publicFiles: {},
66+
trailingSlash: false,
67+
buildId: "test-build-id",
68+
pages: {
69+
dynamic: [],
70+
html: {
71+
dynamic: {},
72+
nonDynamic: {}
73+
},
74+
ssg: {
75+
dynamic: {},
76+
nonDynamic: {}
77+
},
78+
ssr: {
79+
dynamic: {},
80+
nonDynamic: {}
81+
}
82+
}
83+
};
5484
});
5585

5686
it.each`
@@ -68,10 +98,11 @@ describe("Rewriter Tests", () => {
6898
${"/query/foo"} | ${"/target?a=b&path=foo"}
6999
${"/manual-query/foo"} | ${"/target?key=foo"}
70100
${"/multi-query/foo/bar"} | ${"/target?path=foo&path=bar"}
101+
${"/no-op-rewrite"} | ${"/actual-no-op-rewrite-destination"}
71102
`(
72103
"rewrites path $path to $expectedRewrite",
73104
({ path, expectedRewrite }) => {
74-
const rewrite = getRewritePath(path, routesManifest);
105+
const rewrite = getRewritePath(path, routesManifest, pageManifest);
75106

76107
if (expectedRewrite) {
77108
expect(rewrite).toEqual(expectedRewrite);

0 commit comments

Comments
 (0)