Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(lambda-at-edge): fix fallback (non-prerendered) SSG requests not … #800

Merged
merged 3 commits into from
Nov 15, 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 @@ -328,14 +328,27 @@ describe("Pages Tests", () => {

describe("Optional catch-all SSG Page with fallback: true", () => {
[
{ path: "/optional-catch-all-ssg-with-fallback", param: "" },
{ path: "/optional-catch-all-ssg-with-fallback/a", param: "a" },
{ path: "/optional-catch-all-ssg-with-fallback/b", param: "b" },
{
path: "/optional-catch-all-ssg-with-fallback",
param: "",
prerendered: true
},
{
path: "/optional-catch-all-ssg-with-fallback/a",
param: "a",
prerendered: true
},
{
path: "/optional-catch-all-ssg-with-fallback/b",
param: "b",
prerendered: true
},
{
path: "/optional-catch-all-ssg-with-fallback/not-found",
param: ""
param: "",
prerendered: false
}
].forEach(({ path, param }) => {
].forEach(({ path, param, prerendered }) => {
it(`serves and caches page ${path}`, () => {
cy.visit(path);
cy.contains("optional-catch-all-ssg-with-fallback");
Expand All @@ -349,7 +362,9 @@ describe("Pages Tests", () => {
expect(response.body).to.contain(
"optional-catch-all-ssg-with-fallback"
);
expect(response.body).to.contain(`<p data-cy="catch">${param}</p>`);
if (prerendered) {
expect(response.body).to.contain(`<p data-cy="catch">${param}</p>`);
}
});
});

Expand Down Expand Up @@ -386,6 +401,22 @@ describe("Pages Tests", () => {
__N_SSG: true
});
});

// If not prerendered, check that the SSG data request gets cached on the 2nd time requested
if (!prerendered) {
cy.request({ url: fullPath, method: "GET" }).then((response) => {
expect(response.status).to.equal(200);
expect(response.body).to.deep.equal({
pageProps: {
name: "serverless-next.js",
catch: dataRequestParam
},
__N_SSG: true
});

cy.verifyResponseCacheStatus(response, true);
});
}
});
});
});
Expand Down
15 changes: 11 additions & 4 deletions packages/libs/lambda-at-edge/src/default-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -360,15 +360,21 @@ const handleOriginRequest = async ({
request.uri = pagePath.replace("pages", "");
} else if (
pagePath === "pages/_error.js" ||
!prerenderManifest.routes[normalisedDataRequestUri]
(!prerenderManifest.routes[normalisedDataRequestUri] &&
!hasFallbackForUri(
normalisedDataRequestUri,
prerenderManifest,
manifest
))
) {
// Break to continue to SSR render in two cases:
// 1. URI routes to _error.js
// 2. URI is not unmatched, but it's not in prerendered routes, i.e this is an SSR data request, we need to SSR render the JSON
// 2. URI is not unmatched, but it's not in prerendered routes nor is for an SSG fallback, i.e this is an SSR data request, we need to SSR render the JSON
break S3Check;
}

// Otherwise, this is an SSG data request, so continue to get the JSON from S3
// Otherwise, this is an SSG data request, so continue to get to try to get the JSON from S3.
// For fallback SSG, this will fail the first time but the origin response handler will render and store in S3.
}

addS3HostHeader(request, normalisedS3DomainName);
Expand Down Expand Up @@ -500,7 +506,8 @@ const handleOriginResponse = async ({
""
)}`,
Body: JSON.stringify(renderOpts.pageData),
ContentType: "application/json"
ContentType: "application/json",
CacheControl: "public, max-age=0, s-maxage=2678400, must-revalidate"
};
const s3HtmlParams = {
Bucket: bucketName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
"/customers/:catchAll*": {
"file": "pages/customers/[...catchAll].js",
"regex": "^/customers(?:/((?:[^/#?]+?)(?:/(?:[^/#?]+?))*))?[/#?]?$"
},
"/tests/prerender-manifest-fallback/:fallback": {
"file": "pages/tests/prerender-manifest-fallback/[fallback].js",
"regex": "^/tests/prerender-manifest-fallback/([^/]+?)(?:/)?$"
}
},
"nonDynamic": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
"/customers/:catchAll*": {
"file": "pages/customers/[...catchAll].js",
"regex": "^/customers(?:/((?:[^/#?]+?)(?:/(?:[^/#?]+?))*))?[/#?]?$"
},
"/tests/prerender-manifest-fallback/:fallback": {
"file": "pages/tests/prerender-manifest-fallback/[fallback].js",
"regex": "^/tests/prerender-manifest-fallback/([^/]+?)(?:/)?$"
}
},
"nonDynamic": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,8 @@ describe("Lambda@Edge origin response", () => {
Body: JSON.stringify({
page: "pages/fallback/[slug].js"
}),
ContentType: "application/json"
ContentType: "application/json",
CacheControl: "public, max-age=0, s-maxage=2678400, must-revalidate"
});
expect(s3Client.send).toHaveBeenNthCalledWith(2, {
Command: "PutObjectCommand",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -426,9 +426,10 @@ describe("Lambda@Edge", () => {
);

it.each`
path | expectedPage
${"/_next/data/build-id"} | ${"pages/index.js"}
${"/_next/data/build-id/index.json"} | ${"pages/index.js"}
path | expectedPage
${"/_next/data/build-id"} | ${"pages/index.js"}
${"/_next/data/build-id/index.json"} | ${"pages/index.js"}
${"/_next/data/build-id/tests/prerender-manifest-fallback/not-yet-built.json"} | ${"pages/tests/prerender-manifest-fallback/not-yet-built.json"}
`(
"serves json data via S3 for SSG path $path",
async ({ path, expectedPage }) => {
Expand Down