This repository was archived by the owner on May 10, 2021. It is now read-only.
WIP: Fix Netlify Function compat layer #85
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Test that res.redirect works in API routes. Documentation: https://nextjs.org/docs/api-routes/response-helpers
Copy over the files from the next-aws-lambda package and manually bundle them into our Netlify Functions. This gives us more flexibility to customize the compatibility layer between Netlify Functions and Next.js. For now, no changes have been made to the next-aws-lambda files and they have been copied as-is. next-aws-lambda source: https://github.com/serverless-nextjs/serverless-next.js/tree/master/packages/compat-layers/apigw-lambda-compat
As far as I know, Netlify Functions always support base64 encoding. So we can remove the code for checking if base64 encoding is supported.
Use the promise approach of next-aws-lambda rather than the callback approach, because it makes the code easier to read and puts it in the correct order of execution.
Wrap the logic for rendering the Next.js page into its own function. That keeps the netlifyFunction.js (function handler) minimal and easy to read. It will also make it easier for users to modify the function handler while keeping the render function unchanged (down the line, once we support this feature).
Split the reqResMapper function into two separate files. One for creating the request object and one for creating the response object. This is easier to read and understand.
FinnWoelm
force-pushed
the
fix/compat-layer
branch
from
c99b279 to
ef1e7ae
Compare
|
I'm splitting up this PR into multiple PRs. First one is here: #92 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
UPDATE: I'm closing this PR as I'm splitting it into several smaller PRs for more modularity and reviewability.
WORK-IN-PROGRESS — DO NOT MERGE
This PR will fix a few issues with the compatibility layer between Netlify Functions and Next.js.
First, we copy over files fromSee: Remove dependency: next-aws-lambda #92next-aws-lambdaand remove that dependency.Then, we fixSee: Fix: Add support for res.redirect in API routes #93.writeHeadto return theresobject and enable chaining. That will resolve Add failing test for res.redirect #74 (thanks for the test case, @afzalsayed96! It's the first commit in this PR 🙂)Then, we catch any errors that occur after theres.end()has been called. That will resolve Redirects returned from getServerSideProps are not obeyed 😢 #82. I'm not sure if this is a Next.js bug, because the call ofBuffer.byteLength(null)happens inside the serverless Next.js code. In any case, by catching and logging errors that occur after the response has ended, we can get around this until it is fixed upstream.❓ In my tests just now, it looks like this error is resolved with the changes we made to compat layer in Remove dependency: next-aws-lambda #92. I'm waiting for confirmation in Redirects returned from getServerSideProps are not obeyed 😢 #82. I added a Cypress test, but it currently fails because redirects from GSSP is a Next v10 feature. Next v9.5 supports unstableRedirect from GSSP only. Upgrading to Next 10 crashes all our preview mode tests, becausesetPreviewDataseems broken inserverlessmode:{"errorType":"Runtime.UnhandledPromiseRejection","errorMessage":"TypeError: e[t] is not a function","trace":["Runtime.UnhandledPromiseRejection: TypeError: e[t] is not a function"," at process.<anonymous> (/var/runtime/index.js:35:15)"," at process.emit (events.js:314:20)"," at processPromiseRejections (internal/process/promises.js:209:33)"," at processTicksAndRejections (internal/process/task_queues.js:98:32)"]}(this happens at the line where Next.js loads the jsonwebtoken module). Everything works fine inexperimental-serverless-tracemode, but for that we should bundle our own functions first and reduce dependencies to minimal requirements).Then, we addSee: Expose event and context of Netlify Function in Next.js pages and API routes #119netlifyContextto thereqobject, so thatclientContext(for netlify identity) can be accessed in API routes and pages with SSR. That will resolve Netlify Identity clientContext is not passed to API endpoints and pages #20.