E2E for useServerlessTraceTarget

[danielcondemarin]

This is literally a copy of e2e-tests/next-app with one difference, it sets useServerlessTraceTarget: true.

I think we should aim to set serverless trace as the default build target in future, maybe for 1.19. Hopefully the last few fixes and #779 have made it stable enough.

[codecov]

Codecov Report

Merging #787 (384b3ff) into master (8f978bb) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #787   +/-   ##
=======================================
  Coverage   80.00%   80.00%           
=======================================
  Files          56       56           
  Lines        1835     1835           
  Branches      406      406           
=======================================
  Hits         1468     1468           
  Misses        309      309           
  Partials       58       58           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f978bb...384b3ff. Read the comment docs.

[dphang]

@danielcondemarin LGTM besides that.

I'm not so sure about setting serverless trace target as the default build though, since although it reduces code duplication, it can have cold start perf impact, since it's now having to load a bunch of smaller JS modules instead of a single file for a page. From my experience AWS Lambda code size has very little impact on cold start, it's more the I/O/requires that makes up the majority of the time.

There are ways to optimize code size like minifying the pages JS files or manually removing page JS only used for pre-rendering (e.g we do it automatically if you do not use any API routes). I added postBuildCommands input which can help with this if you do use API routes, but you need to manually specify which JS files are safe to remove. It is also the user's responsibility to ensure they are not importing libraries in a way that prevents proper tree-shaking (material-ui is often a culprit, if you use top-level imports).

[danielcondemarin]

I'm not so sure about setting serverless trace target as the default build though, since although it reduces code duplication, it can have cold start perf impact, since it's now having to load a bunch of smaller JS modules instead of a single file for a page. From my experience AWS Lambda code size has very little impact on cold start, it's more the I/O/requires that makes up the majority of the time.

That's a fair point. I'm mainly proposing the change because the Next team uses this as their build target for Vercel deployments. The other is that I suspect they will be deprecating the serverless target.
Happy to keep serverless-trace as an opt-in for now. I might do a bit more digging on this area and post any findings.

[dphang]

I'm not so sure about setting serverless trace target as the default build though, since although it reduces code duplication, it can have cold start perf impact, since it's now having to load a bunch of smaller JS modules instead of a single file for a page. From my experience AWS Lambda code size has very little impact on cold start, it's more the I/O/requires that makes up the majority of the time.

That's a fair point. I'm mainly proposing the change because the Next team uses this as their build target for Vercel deployments. The other is that I suspect they will be deprecating the serverless target.
Happy to keep serverless-trace as an opt-in for now. I might do a bit more digging on this area and post any findings.

Yep, I did try Vercel's deployment and found it slow for that same reason - a while back I had opened an issue but had no responses. I hope they do not remove the serverless target.

[dphang]

Looks like e2e tests failed here: https://github.com/serverless-nextjs/serverless-next.js/actions/runs/358701235

[dphang]

Woops, overlooked this, I will fix for you @danielcondemarin