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

fix(lambda-at-edge): non-dynamic pages (SSG, SSR) should be prioritized over dynamic fallback pages #685

Merged
merged 1 commit into from
Oct 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ describe("Pages Tests", () => {
cy.ensureAllRoutesNotErrored();
});

describe("Dynamic Pages with getStaticPaths() fallback: true", () => {
describe("Dynamic Page at root level with getStaticPaths() fallback: true", () => {
[{ path: "/a" }, { path: "/b" }, { path: "/not-prerendered" }].forEach(
({ path }) => {
it(`serves and caches page ${path}`, () => {
Expand Down Expand Up @@ -36,9 +36,29 @@ describe("Pages Tests", () => {
});
}
);

it("serves non-dynamic SSG page at same level", () => {
const path = "/ssg-page";

cy.request({
url: path
}).then((response) => {
expect(response.body).to.contain("SSG Page");
});
});

it("serves non-dynamic SSR page at same level", () => {
const path = "/ssr-page";

cy.request({
url: path
}).then((response) => {
expect(response.body).to.contain("SSR Page");
});
});
});

describe("Nested Dynamic Pages with getStaticPaths() fallback: false", () => {
describe("Nested Dynamic Page with getStaticPaths() fallback: false", () => {
[{ path: "/nested/a" }, { path: "/nested/b" }].forEach(({ path }) => {
it(`serves and caches page ${path}`, () => {
cy.visit(path);
Expand Down Expand Up @@ -86,9 +106,6 @@ describe("Pages Tests", () => {
failOnStatusCode: false
}).then((response) => {
expect(response.status).to.equal(404);
expect(response.headers["x-cache"]).to.equal(
"Error from cloudfront"
);
});
});
});
Expand Down
2 changes: 1 addition & 1 deletion packages/e2e-tests/next-app-dynamic-routes/pages/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export default function IndexPage(props: IndexPageProps): JSX.Element {
<div>
{`Hello ${props.name}. This is an SSR page using getServerSideProps(). It also has an image.`}
</div>
<img src={"/basepath/app-store-badge.png"} alt={"An image"} />
<img src={"/app-store-badge.png"} alt={"An image"} />
</React.Fragment>
);
}
Expand Down
15 changes: 12 additions & 3 deletions packages/libs/lambda-at-edge/src/default-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ const handleOriginRequest = async ({
const s3Origin = origin.s3 as CloudFrontS3Origin;
const isHTMLPage = isStaticPage || isPrerenderedPage;
const normalisedS3DomainName = normaliseS3OriginDomain(s3Origin);
const hasFallback = hasFallbackForUri(uri, prerenderManifest);
const hasFallback = hasFallbackForUri(uri, prerenderManifest, manifest);
const { now, log } = perfLogger(manifest.logLambdaExecutionTimes);
const previewCookies = getPreviewCookies(request);
const isPreviewRequest =
Expand Down Expand Up @@ -496,7 +496,7 @@ const handleOriginResponse = async ({
res.end(JSON.stringify(renderOpts.pageData));
return await responsePromise;
} else {
const hasFallback = hasFallbackForUri(uri, prerenderManifest);
const hasFallback = hasFallbackForUri(uri, prerenderManifest, manifest);
if (!hasFallback) return response;

// If route has fallback, return that page from S3, otherwise return 404 page
Expand Down Expand Up @@ -550,8 +550,17 @@ const isOriginResponse = (

const hasFallbackForUri = (
uri: string,
prerenderManifest: PrerenderManifestType
prerenderManifest: PrerenderManifestType,
manifest: OriginRequestDefaultHandlerManifest
) => {
const {
pages: { ssr, html }
} = manifest;
// Non dynamic routes are prioritized over dynamic fallbacks, return false to ensure those get rendered instead
if (ssr.nonDynamic[uri] || html.nonDynamic[uri]) {
return false;
}

return Object.values(prerenderManifest.dynamicRoutes).find((routeConfig) => {
const re = new RegExp(routeConfig.routeRegex);
return re.test(uri);
Expand Down