Support Preview Mode #562

thchia · 2020-08-26T11:13:28Z

#355

Changes

lambda-at-edge-compat

NextJS internally calls res.writeHead(...).end(), so we have to return the res in writeHead.

lambda-at-edge

For all HTML/JSON data requests (including potential fallbacks), only forward the response to the origin if it is not a preview request.
When we reach the SSR portion, we are potentially still handling a JSON data request (due to changes in the previous point). Check and render/return the appropriate content.

Caveat

This is actually nothing to do with this library. It's more about the behaviour of NextJS.

Handling a client side transition in preview mode is not really supported beyond the first transition. After this, NextJS caches the JSON response in memory and does not even issue a new request if you navigate back to that page in the same session (without a refresh). Consider the following steps:

Enter preview mode for mysite.com/blog/1 (an SSG page)
mysite.com/blog/1 is SSR'd because we are in preview mode
Navigate to mysite.com/blog/2 using next/link
Navigate back to mysite.com/blog/1 using next/link
JSON data for Blog 1 is generated at request time
Repeat steps 3-4
JSON data is not regenerated, because we already have it in memory and NextJS does not issue a new request

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

thchia · 2020-08-30T10:25:39Z

@danielcondemarin marking this as draft for now.

I have added rollup and am bundling the default handler. As a result of this, the build step for this package is getting fragmented, with one thing for default lambda and another for the api lambda and the build function (also, the bundler needs a different tsconfig).

Should we bundle the api lambda too? For now I am just telling the TS process for the existing build to ignore the default lambda file, and letting it get handled by rollup. We can have 2 sub-builds, one for the lambdas and one for the build function, with both going into the dist folder.

danielcondemarin · 2020-08-30T12:38:10Z

Should we bundle the api lambda too?

If it isn't much extra work go for it. There is refactor that could be made once the handler is bundled like splitting logic into other files etc.

For now I am just telling the TS process for the existing build to ignore the default lambda file, and letting it get handled by rollup. We can have 2 sub-builds, one for the lambdas and one for the build function, with both going into the dist folder.

Sounds good to me 👍

thchia · 2020-08-31T11:15:00Z

@danielcondemarin this is good for review. I will squash the commits before we merge and close this though.

For some reason, rollup didn't like using a tsconfig that extends another tsconfig. Hence why tsconfig.bundle.json is standalone 🤷‍♀️
If the preview token verification fails, it's a "hard fail" rather than sending back a nice 403 page (like pages/_error.js). I'm neither here nor there on this, but I don't think it's necessary to go through the SSR for this.

- Fixed expected uri in origin response. - Added util to rollup externals as it was breaking.

- Fixed some erroneous logic in the router, since it's now being used in the response handler - Mocked console errors in tests

dphang · 2020-09-01T01:35:55Z

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

@@ -272,7 +313,17 @@ const handleOriginRequest = async ({
    }


I think this can be moved into the else block below just before the await page.render(req, res), so we don't have to check whether it is error page every time

Does that mean we know for a fact that data requests will not result in a possible error page?

Ah, I guess unmatched data request can still go to pages/_error.js, maybe still need this at the start

dphang · 2020-09-01T01:40:48Z

packages/libs/lambda-at-edge/tests/default-handler/default-handler.test.ts

+
+      it("handles preview mode", async () => {
+        const event = createCloudFrontEvent({
+          uri: "/preview",


I think you would need to append trailing slash if trailingSlash is true to the URI, this test runs with both trailingSlash true and false

dphang · 2020-09-01T01:43:35Z

packages/libs/lambda-at-edge/tests/default-handler/default-handler.test.ts

+          }
+        });
+
+        if (trailingSlash) {


You shouldn't need the trailingSlash block here, I added a redirect test where you can add /preview as one of the tests, it will test both kinds of redirects

You mean changing the URI in createCloudFrontEvent to be either /preview or /preview/ depending on whether trailingSlash is true, and then just testing for the expected response?

Yep, basically if trailingSlash is true then you can pass /preview/ as the URI, otherwise it is /preview.

Then you can add the same path /preview as a test case in redirect test, it will auto append trailing slash and test the redirects

Also i think you can add /preview to redirect test (around line 158).

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

dphang · 2020-09-01T01:56:03Z

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

+        prerenderManifest.preview.previewModeSigningKey
+      );
+    } catch (e) {
+      return {


Maybe some logging (at info level?) is useful here, I added it for 500 page as well, not sure if it's needed for this though

I added a console.error, similar to the logging for the 500 response.

dphang

Seems fine to me besides minor comments, probably @danielcondemarin should approve since it's a bigger change

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

danielcondemarin

🚀

danielcondemarin · 2020-09-02T09:29:36Z

I'll do some testing and get an alpha published tonight 👍

danielcondemarin reviewed Aug 26, 2020

View reviewed changes

packages/libs/lambda-at-edge/src/default-handler.ts Outdated Show resolved Hide resolved

danielcondemarin reviewed Aug 26, 2020

View reviewed changes

packages/libs/lambda-at-edge/src/default-handler.ts Outdated Show resolved Hide resolved

danielcondemarin reviewed Aug 26, 2020

View reviewed changes

packages/libs/lambda-at-edge/src/default-handler.ts Outdated Show resolved Hide resolved

thchia force-pushed the feature/preview-mode branch from 13fb79b to e54d915 Compare August 27, 2020 12:02

thchia changed the title ~~Support Preview Mode~~ 🚧 Support Preview Mode Aug 27, 2020

thchia force-pushed the feature/preview-mode branch 2 times, most recently from bd82c46 to 5f28647 Compare August 30, 2020 10:20

thchia marked this pull request as draft August 30, 2020 10:21

thchia force-pushed the feature/preview-mode branch 2 times, most recently from d40579b to 2ce1d38 Compare August 31, 2020 10:53

thchia changed the title ~~🚧 Support Preview Mode~~ Support Preview Mode Sep 1, 2020

thchia added 8 commits September 1, 2020 09:25

Added preview mode support.

b4cf02b

Rebased and updated tests.

b0d997d

- Added rollup and bundle default handler

aadbc82

- Fixed expected uri in origin response. - Added util to rollup externals as it was breaking.

Rebased and updated tests

59af91f

Updated another test

9f68430

- Add API Lambda to rollup

c8b12a8

- Fixed some erroneous logic in the router, since it's now being used in the response handler - Mocked console errors in tests

Revert some changes which were superseded by @dphang's changes.

938e21d

Fixed integration test

6b95135

thchia force-pushed the feature/preview-mode branch from c60d647 to 6b95135 Compare September 1, 2020 01:26

thchia marked this pull request as ready for review September 1, 2020 01:26

dphang reviewed Sep 1, 2020

View reviewed changes

packages/libs/lambda-at-edge/src/default-handler.ts Outdated Show resolved Hide resolved

dphang reviewed Sep 1, 2020

View reviewed changes

thchia added 4 commits September 1, 2020 09:58

Updated tests

12efe58

Added explicit return to getPreviewCookies

0cb6377

Added error logging to preview token verification catch block.

c5d662c

Added preview path test case to redirect

131913c

danielcondemarin reviewed Sep 2, 2020

View reviewed changes

packages/libs/lambda-at-edge/src/default-handler.ts Outdated Show resolved Hide resolved

Optimised getPreviewCookies function.

e477354

danielcondemarin approved these changes Sep 2, 2020

View reviewed changes

danielcondemarin merged commit 5e1ea38 into serverless-nextjs:master Sep 2, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Preview Mode #562

Support Preview Mode #562

thchia commented Aug 26, 2020

thchia commented Aug 30, 2020

danielcondemarin commented Aug 30, 2020 •

edited

Loading

thchia commented Aug 31, 2020

dphang Sep 1, 2020 •

edited

Loading

thchia Sep 1, 2020

dphang Sep 1, 2020 •

edited

Loading

dphang Sep 1, 2020 •

edited

Loading

dphang Sep 1, 2020

thchia Sep 1, 2020

dphang Sep 1, 2020

thchia Sep 1, 2020

dphang Sep 1, 2020

thchia Sep 1, 2020

dphang Sep 1, 2020 •

edited

Loading

thchia Sep 1, 2020

dphang left a comment

danielcondemarin left a comment

danielcondemarin commented Sep 2, 2020

Support Preview Mode #562

Support Preview Mode #562

Conversation

thchia commented Aug 26, 2020

Changes

lambda-at-edge-compat

lambda-at-edge

Caveat

thchia commented Aug 30, 2020

danielcondemarin commented Aug 30, 2020 • edited Loading

thchia commented Aug 31, 2020

dphang Sep 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dphang Sep 1, 2020 • edited Loading

Choose a reason for hiding this comment

dphang Sep 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dphang Sep 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dphang left a comment

Choose a reason for hiding this comment

danielcondemarin left a comment

Choose a reason for hiding this comment

danielcondemarin commented Sep 2, 2020

danielcondemarin commented Aug 30, 2020 •

edited

Loading

dphang Sep 1, 2020 •

edited

Loading

dphang Sep 1, 2020 •

edited

Loading

dphang Sep 1, 2020 •

edited

Loading

dphang Sep 1, 2020 •

edited

Loading