Discussion: inlining next-aws-lambda vs fixing upstream and other alternatives

[lindsaylevine]

stakeholders: @FinnWoelm @ehmicky

This issue is to publicly discuss an internal discussion around inlining next-aws-lambda.

Original issues with depending on next-aws-lambda:

1. plugin dependency issue (Remove npm install next-on-netlify opennextjs/opennextjs-netlify#10) (this is being addressed internally)
2. miscellaneous user-reported issues

To move forward and relieve some tension with these open issues, we've temporarily inlined next-aws-lambda with Finn's awesome PR. However, @ehmicky brought up concerns around future maintenance issues with inlining.

Options:

1. Inline the package (most fine-grained control but needing to repeatedly update a semi-active project)
2. Use patch-package
3. Fork the project (monorepo though)
4. Open PRs upstream (this could benefit both the greater Next.js community and other Netlify plugins)

While we all agree inlining is a great first step and improvement from what we had for the original issues, let's try to align on the best path forward.

[ehmicky]

Thanks for opening this @lindsaylevine!
For information, this is the link to patch-package.
Also, this is a possible list of things to fix with next-aws-lambda.

I think opening PRs upstream to solve those problems in next-aws-lambda would be better since it could benefit the greater Next.js community as you mention. Also, the current inlining would require any update of the two inlined files in next-aws-lambda to be ported to this repository manually. Finally, we would benefit from additional code review from next-aws-lambda maintainers. And we would benefit from their automated tests.

[FinnWoelm]

Hmmm! Good points! I see the strong benefits of opening PRs upstream!

How about the following proposal: We continue with inlined next-aws-lambda for now, which is already 95% done and allows us to fix the two remaining compatibility-layer-related issues #82 and #20 asap. As we go on, @lindsaylevine and I will periodically check for updates made to next-aws-lambda (source code and changelog). If, say in a couple months time, we realize that this inlining approach is creating a headache because we have to manually copy over a bunch of patches all the time, let's reevaluate then and perhaps invite Daniel Conde (the creator and maintainer of the package) for a Zoom chat to discuss how we can best integrate any Netlify-specific needs. How does that sound?

PS: If we need to justify the inlining for now, we could use the following three arguments:

1. According to the changelog and git blame, next-aws-lambda has received only four or five minor patches over the last 15 months (most version changes are just version bumps to keep it aligned with the other packages in that monorepo)
2. The large majority of the userbase of next-aws-lambda are our own users: next-aws-lambda has 8.5k weekly downloads, 6.5k of which come from NoN users
   
   
3. The maintainers of next-aws-lambda are currently focused on using AWS Lambda@edge infrastructure rather than AWS lambda (see Could you still support deploy nextjs 9 using lambda not lambda edge? serverless-nextjs/serverless-next.js#296). next-aws-lambda is only used in their deprecated next-serverless-plugin. Although, this thread sounds like that may change in the future: RFC: state of Serverless Next.js and next steps serverless-nextjs/serverless-next.js#777 — a good reason to keep a close eye on developments and reevaluate our decision once we have more information!

[FinnWoelm]

PPS: I am personally very interested to learn more about the patch-package approach (even though I think for the sake of next-on-netlify, we should continue with inlining next-aws-lambda for now and then reevaluate in a couple months based on how our experience goes).

I've used and suggested patch-package to users before (see #20), but the workflow was never really smooth for the end user as they had to manually copy patches (copy -r ./node_modules/next-on-netlify/patches ./patches). patch-package is not built to support libraries/modules supplying their own patches to avoid situations where one of your dependencies gets patched by one module and then is incompatible with another module, as explained here and here. If you know a way around that, I would be very keen to learn 😊

[ehmicky]

This is a very pragmatic approach @FinnWoelm, love it! ❤️

[FinnWoelm]

Ok 😊 PS: Your work and improvements to next-on-netlify and the Next.js plugin are AMAZING 🎉 🙌 🙌

[lindsaylevine]

hey all, closing this since the discussion is over :). per the resolution we reached, if/when we run into issues manually updating what's inlined, we can re-address then!