-
Notifications
You must be signed in to change notification settings - Fork 73
Fix for next 15.2 and renderHTML should not be called in minimal mode #441
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
commit: |
packages/cloudflare/src/cli/build/patches/plugins/next-minimal.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @conico974 for this fix.
I think the workerd error is expected - but we should fix this by not running next code at top level. It will also help with other issues (access process.env
and getCloudflareContext()
at top level). I hope to get to this next week. But it should not be a reason not to merge this PR.
I have added a few questions/comments after a quick review.
packages/cloudflare/src/cli/build/patches/plugins/next-minimal.ts
Outdated
Show resolved
Hide resolved
packages/cloudflare/src/cli/build/patches/plugins/next-minimal.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome PR, it will be well appreciated by the community 🎉
I have added a few minor comments
Should fix #432 #373 and #333
Next 15.2 introduced a change that create an
AbortController
https://github.com/vercel/next.js/pull/73975/files as a const outside of the request context.This
AbortController
cause an issue while requiring app router pages that makes all these require return{}
instead of the actual module in workerd. (see the comment for more detail)It also replace the define of
process.env.NEXT_MINIMAL
inside of esbuild by a custom plugin that keep therenderHTML
that can be used as a fallback by Next in case of error.It also bump next version to 15.2.0 for the e2e examples
It's a draft for now, if workerd is not expected to throw for this case we could remove almost everything in this PR and just bump wrangler once fixed and merged