Skip to content
This repository has been archived by the owner on May 10, 2021. It is now read-only.

Remove dependency: next-aws-lambda #92

Merged
merged 5 commits into from
Nov 18, 2020
Merged

Conversation

FinnWoelm
Copy link
Contributor

This PR is the basis for fixing a few issues related to the compatibility layer between Next.js and Netlify Functions.

Until now, we were relying on the next-aws-lambda package (source code:https://github.com/serverless-nextjs/serverless-next.js/blob/master/packages/compat-layers/apigw-lambda-compat/). However, there are some issues that have arisen that we can only fix by modifying the compatibility layer (#74, #82, #20). So this PR copies over next-aws-lambda (it's just two files) and removes the dependency, so that we can modify and customize this compatibility layer in the future and fix the aforementioned issues (PRs coming!).

The PR also refactors the files from next-aws-lambda to remove unnecessary code and make them easier to read. There are were comments in the package code, so unfortunately the files continue to have relatively few comments — at least for now. Our Netlify Function handler should be easier to read now, too.

This PR will also enable us to make progress on opennextjs/opennextjs-netlify#10.

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.
@lindsaylevine lindsaylevine added the type: bug code to address defects in shipped code label Nov 18, 2020
Copy link
Contributor

@lindsaylevine lindsaylevine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YOU ARE A KING

@lindsaylevine lindsaylevine merged commit 6f1a1b3 into main Nov 18, 2020
lindsaylevine added a commit that referenced this pull request Nov 18, 2020
- Fix: don't empty publish/function paths unless they're default ([#94](#94))
- Fix: add support for res.redirect in API routes ([#93](#93))
- Remove next-aws-lambda dependency (now inlined) ([#92](#92))
- Fix: Node.js version in CI ([#91](#91))
@FinnWoelm FinnWoelm deleted the remove-next-aws-lambda branch November 20, 2020 02:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants