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
fix(nextjs): Fix broken server component wrapping because of interrupted promise chain #7456
Conversation
|
||
// It is very important that we return the original promise here, because Next.js attaches various properties | ||
// to that promise and will throw if they are not on the returned value. | ||
return maybePromiseResult; |
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.
omg I actually can't believe they are doing this.
who attaches stuff to the promise directly????
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.
🥲
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.
This feels like an anti-pattern.
Glad the tests caught this though!
size-limit report 📦
|
I agree. Something feels off here. I don't know what. Maybe this will cause issues at some point. I also think flushing will be kinda messed up with this but I wanna worry about that later. |
Our canary tests for Next.js started failing #7452 - in particular Next.js version
13.2.4
.This was/is an actual bug that is fixed by that PR. Apparently, from that version onwards, Next.js attaches additional values to the promises returned from server components, and the way we wrapped server components we janked those values off the returned promise causing Next.js to crash.