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

Commit e667e76

Browse files
authored
fix(lambda-at-edge): fix dynamic route precedence conflicting with fallback pages (#714)
1 parent 0b91309 commit e667e76

File tree

5 files changed

+173
-2
lines changed

5 files changed

+173
-2
lines changed

packages/e2e-tests/next-app-dynamic-routes/cypress/integration/pages.test.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,10 @@ describe("Pages Tests", () => {
133133

134134
cy.visit(path);
135135
cy.location("pathname").should("eq", path);
136+
137+
cy.request(path).then((response) => {
138+
expect(response.body).to.contain("catch-all-ssr");
139+
});
136140
});
137141

138142
["HEAD", "DELETE", "POST", "GET", "OPTIONS", "PUT", "PATCH"].forEach(
@@ -146,4 +150,70 @@ describe("Pages Tests", () => {
146150
);
147151
});
148152
});
153+
154+
describe("Dynamic SSR page", () => {
155+
[{ path: "/another/1234" }].forEach(({ path }) => {
156+
it(`serves and caches page ${path}`, () => {
157+
cy.ensureRouteNotCached(path);
158+
159+
cy.visit(path);
160+
cy.location("pathname").should("eq", path);
161+
162+
cy.request(path).then((response) => {
163+
expect(response.body).to.contain("dynamic-ssr");
164+
});
165+
});
166+
167+
["HEAD", "DELETE", "POST", "GET", "OPTIONS", "PUT", "PATCH"].forEach(
168+
(method) => {
169+
it(`allows HTTP method for path ${path}: ${method}`, () => {
170+
cy.request({ url: path, method: method }).then((response) => {
171+
expect(response.status).to.equal(200);
172+
});
173+
});
174+
}
175+
);
176+
});
177+
178+
[{ path: "/another/ssg-prioritized-over-dynamic-ssr" }].forEach(
179+
({ path }) => {
180+
it(`serves and caches page ${path}`, () => {
181+
cy.visit(path);
182+
183+
cy.ensureRouteCached(path);
184+
185+
cy.visit(path);
186+
cy.location("pathname").should("eq", path);
187+
188+
cy.request(path).then((response) => {
189+
expect(response.body).to.contain(
190+
"ssg-prioritized-over-dynamic-ssr"
191+
);
192+
});
193+
});
194+
195+
["HEAD", "GET"].forEach((method) => {
196+
it(`allows HTTP method for path ${path}: ${method}`, () => {
197+
cy.request({ url: path, method: method }).then((response) => {
198+
expect(response.status).to.equal(200);
199+
});
200+
});
201+
});
202+
203+
["DELETE", "POST", "OPTIONS", "PUT", "PATCH"].forEach((method) => {
204+
it(`disallows HTTP method for path ${path} with 4xx error: ${method}`, () => {
205+
cy.request({
206+
url: path,
207+
method: method,
208+
failOnStatusCode: false
209+
}).then((response) => {
210+
expect(response.status).to.not.equal(404);
211+
expect(response.status).to.be.at.least(400);
212+
expect(response.status).to.be.lessThan(500);
213+
});
214+
});
215+
});
216+
}
217+
);
218+
});
149219
});

packages/e2e-tests/next-app-dynamic-routes/pages/[...catch].tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export default function CatchAllPage(props: CatchAllPageProps): JSX.Element {
99
return (
1010
<React.Fragment>
1111
<div>
12-
{`Hello ${props.name}. This is a catch-all SSR page using getServerSideProps(). It also has an image.`}
12+
{`Hello ${props.name}. This is catch-all-ssr, a catch-all SSR page using getServerSideProps(). It also has an image.`}
1313
</div>
1414
<img src={"/app-store-badge.png"} alt={"An image"} />
1515
</React.Fragment>
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import React from "react";
2+
import { NextPageContext } from "next";
3+
4+
type DynamicSSRPageProps = {
5+
name: string;
6+
};
7+
8+
export default function DynamicSSRPage(
9+
props: DynamicSSRPageProps
10+
): JSX.Element {
11+
return (
12+
<React.Fragment>
13+
<div>{`Hello ${props.name}. This is dynamic-ssr, a dynamic SSR page.`}</div>
14+
</React.Fragment>
15+
);
16+
}
17+
18+
export async function getServerSideProps(
19+
ctx: NextPageContext
20+
): Promise<{ props: DynamicSSRPageProps }> {
21+
return {
22+
props: { name: "serverless-next.js" }
23+
};
24+
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import React from "react";
2+
import { NextPageContext } from "next";
3+
4+
type SSGPrioritizedOverDynamicSSRPageProps = {
5+
name: string;
6+
};
7+
8+
export default function SSGPrioritizedOverDynamicSSRPage(
9+
props: SSGPrioritizedOverDynamicSSRPageProps
10+
): JSX.Element {
11+
return (
12+
<React.Fragment>
13+
{`Hello ${props.name}! This is ssg-prioritized-over-dynamic-ssr, to test that predefined SSG page is prioritized over dynamic SSR page.`}
14+
</React.Fragment>
15+
);
16+
}
17+
18+
export async function getStaticProps(
19+
ctx: NextPageContext
20+
): Promise<{ props: SSGPrioritizedOverDynamicSSRPageProps }> {
21+
return {
22+
props: { name: "serverless-next.js" }
23+
};
24+
}

packages/libs/lambda-at-edge/src/default-handler.ts

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -576,11 +576,64 @@ const hasFallbackForUri = (
576576
const {
577577
pages: { ssr, html }
578578
} = manifest;
579-
// Non dynamic routes are prioritized over dynamic fallbacks, return false to ensure those get rendered instead
579+
// Non-dynamic routes are prioritized over dynamic fallbacks, return false to ensure those get rendered instead
580580
if (ssr.nonDynamic[uri] || html.nonDynamic[uri]) {
581581
return false;
582582
}
583583

584+
let foundFallback:
585+
| {
586+
routeRegex: string;
587+
fallback: string | false;
588+
dataRoute: string;
589+
dataRouteRegex: string;
590+
}
591+
| undefined = undefined; // for later use to reduce duplicate work
592+
593+
// Dynamic routes that does not have fallback are prioritized over dynamic fallback
594+
const isNonFallbackDynamicRoute = Object.values({
595+
...ssr.dynamic,
596+
...html.dynamic
597+
}).find((dynamicRoute) => {
598+
if (foundFallback) {
599+
return false;
600+
}
601+
602+
const re = new RegExp(dynamicRoute.regex);
603+
const matchesRegex = re.test(uri);
604+
605+
// If any dynamic route matches, check that this isn't one of the fallback routes in prerender manifest
606+
if (matchesRegex) {
607+
const matchesFallbackRoute = Object.keys(
608+
prerenderManifest.dynamicRoutes
609+
).find((prerenderManifestRoute) => {
610+
const fileMatchesPrerenderRoute =
611+
dynamicRoute.file === `pages${prerenderManifestRoute}.js`;
612+
613+
if (fileMatchesPrerenderRoute) {
614+
foundFallback =
615+
prerenderManifest.dynamicRoutes[prerenderManifestRoute];
616+
}
617+
618+
return fileMatchesPrerenderRoute;
619+
});
620+
621+
return !matchesFallbackRoute;
622+
} else {
623+
return false;
624+
}
625+
});
626+
627+
if (isNonFallbackDynamicRoute) {
628+
return false;
629+
}
630+
631+
// If fallback previously found, return it to prevent additional regex matching
632+
if (foundFallback) {
633+
return foundFallback;
634+
}
635+
636+
// Otherwise, try to match fallback against dynamic routes in prerender manifest
584637
return Object.values(prerenderManifest.dynamicRoutes).find((routeConfig) => {
585638
const re = new RegExp(routeConfig.routeRegex);
586639
return re.test(uri);

0 commit comments

Comments
 (0)