From 268c1ae250054f12dba66ddba500188bcd251127 Mon Sep 17 00:00:00 2001 From: Daniel Phang Date: Sat, 14 Nov 2020 23:32:23 -0800 Subject: [PATCH 1/3] fix(lambda-at-edge): fix fallback (non-prerendered) SSG requests not generating data in S3 --- .../cypress/integration/pages.test.ts | 39 ++++++++++++++++--- .../lambda-at-edge/src/default-handler.ts | 15 +++++-- ...lt-build-manifest-with-trailing-slash.json | 4 ++ .../default-build-manifest.json | 4 ++ .../default-handler-origin-response.test.ts | 3 +- .../default-handler/default-handler.test.ts | 9 +++-- 6 files changed, 61 insertions(+), 13 deletions(-) diff --git a/packages/e2e-tests/next-app-dynamic-routes/cypress/integration/pages.test.ts b/packages/e2e-tests/next-app-dynamic-routes/cypress/integration/pages.test.ts index 4c0987676d..87b57e648f 100644 --- a/packages/e2e-tests/next-app-dynamic-routes/cypress/integration/pages.test.ts +++ b/packages/e2e-tests/next-app-dynamic-routes/cypress/integration/pages.test.ts @@ -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"); @@ -386,6 +399,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); + }); + } }); }); }); diff --git a/packages/libs/lambda-at-edge/src/default-handler.ts b/packages/libs/lambda-at-edge/src/default-handler.ts index 90d25913e3..99136533ce 100644 --- a/packages/libs/lambda-at-edge/src/default-handler.ts +++ b/packages/libs/lambda-at-edge/src/default-handler.ts @@ -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); @@ -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, diff --git a/packages/libs/lambda-at-edge/tests/default-handler/default-build-manifest-with-trailing-slash.json b/packages/libs/lambda-at-edge/tests/default-handler/default-build-manifest-with-trailing-slash.json index 402ad6b81f..7154d258e7 100644 --- a/packages/libs/lambda-at-edge/tests/default-handler/default-build-manifest-with-trailing-slash.json +++ b/packages/libs/lambda-at-edge/tests/default-handler/default-build-manifest-with-trailing-slash.json @@ -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": { diff --git a/packages/libs/lambda-at-edge/tests/default-handler/default-build-manifest.json b/packages/libs/lambda-at-edge/tests/default-handler/default-build-manifest.json index d28ab00afb..770934392e 100644 --- a/packages/libs/lambda-at-edge/tests/default-handler/default-build-manifest.json +++ b/packages/libs/lambda-at-edge/tests/default-handler/default-build-manifest.json @@ -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": { diff --git a/packages/libs/lambda-at-edge/tests/default-handler/default-handler-origin-response.test.ts b/packages/libs/lambda-at-edge/tests/default-handler/default-handler-origin-response.test.ts index eb616f438c..60c33a462f 100644 --- a/packages/libs/lambda-at-edge/tests/default-handler/default-handler-origin-response.test.ts +++ b/packages/libs/lambda-at-edge/tests/default-handler/default-handler-origin-response.test.ts @@ -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", diff --git a/packages/libs/lambda-at-edge/tests/default-handler/default-handler.test.ts b/packages/libs/lambda-at-edge/tests/default-handler/default-handler.test.ts index a9e0928b05..c230968ae1 100644 --- a/packages/libs/lambda-at-edge/tests/default-handler/default-handler.test.ts +++ b/packages/libs/lambda-at-edge/tests/default-handler/default-handler.test.ts @@ -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 }) => { @@ -444,6 +445,8 @@ describe("Lambda@Edge", () => { const request = result as CloudFrontRequest; + console.log(JSON.stringify(request)); + expect(request.origin).toEqual({ s3: { authMethod: "origin-access-identity", From ecdf11ea6d4db90531fdd6e0e5ee068ac0dbe5ad Mon Sep 17 00:00:00 2001 From: Daniel Phang Date: Sat, 14 Nov 2020 23:34:26 -0800 Subject: [PATCH 2/3] remove log statement --- .../tests/default-handler/default-handler.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/libs/lambda-at-edge/tests/default-handler/default-handler.test.ts b/packages/libs/lambda-at-edge/tests/default-handler/default-handler.test.ts index c230968ae1..b73d1a8f35 100644 --- a/packages/libs/lambda-at-edge/tests/default-handler/default-handler.test.ts +++ b/packages/libs/lambda-at-edge/tests/default-handler/default-handler.test.ts @@ -445,8 +445,6 @@ describe("Lambda@Edge", () => { const request = result as CloudFrontRequest; - console.log(JSON.stringify(request)); - expect(request.origin).toEqual({ s3: { authMethod: "origin-access-identity", From ac7b2a54113b9699149775c2e0a04413e4001292 Mon Sep 17 00:00:00 2001 From: Daniel Phang Date: Sat, 14 Nov 2020 23:50:18 -0800 Subject: [PATCH 3/3] fix e2e test --- .../next-app-dynamic-routes/cypress/integration/pages.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/e2e-tests/next-app-dynamic-routes/cypress/integration/pages.test.ts b/packages/e2e-tests/next-app-dynamic-routes/cypress/integration/pages.test.ts index 87b57e648f..fbba6abc36 100644 --- a/packages/e2e-tests/next-app-dynamic-routes/cypress/integration/pages.test.ts +++ b/packages/e2e-tests/next-app-dynamic-routes/cypress/integration/pages.test.ts @@ -362,7 +362,9 @@ describe("Pages Tests", () => { expect(response.body).to.contain( "optional-catch-all-ssg-with-fallback" ); - expect(response.body).to.contain(`

${param}

`); + if (prerendered) { + expect(response.body).to.contain(`

${param}

`); + } }); });