fix(pages-router): await Promise-shaped gssp props#1568
Conversation
Next.js explicitly supports a Promise value for `getServerSideProps`
`props`. Pages returning `{ props: Promise.resolve(...) }` previously
rendered empty pageProps because vinext passed the unresolved Promise
straight through to the serializer. Match Next.js' `render.tsx` by
detecting a thenable `props` and awaiting it before continuing.
The fix is applied symmetrically to the dev server gssp dispatch
(`dev-server.ts`) and the prod page-data dispatch (`pages-page-data.ts`)
so the two stay in sync per AGENTS.md. Other return shapes (`notFound`,
`redirect`, `revalidate`, fallback) are untouched.
Closes #1459
|
/bigbonk review |
commit: |
|
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code is correct and focused. Both the dev server (dev-server.ts:532-540) and the shared prod helper (pages-page-data.ts:390-399) now correctly detect thenable props and await them before serialisation. The fixture page and test coverage (both dev and prod) are good.
Two observations:
1. Thenable detection vs instanceof Promise
Next.js uses (data as any).props instanceof Promise (see packages/next/src/server/render.tsx, line ~1145 on canary). This PR uses typeof result.props.then === "function", which is strictly more permissive (catches any thenable, not just native Promises). That's fine — it's defense-in-depth and won't cause false positives on normal objects. Just noting the difference for posterity.
2. getStaticProps props are not Promise-awaited — but that matches Next.js
I checked whether the same pattern applies to getStaticProps. In Next.js's render.tsx, the deferredContent / Promise-await logic is gated behind the getServerSideProps branch only — getStaticProps props are consumed synchronously via Object.assign({}, props.pageProps, 'props' in data ? data.props : undefined) without any Promise check. So vinext's getStaticProps code paths (4 locations across dev-server.ts and pages-page-data.ts) not handling Promise props is actually correct parity with Next.js. No action needed there.
LGTM — the fix matches Next.js semantics, the two code paths that needed fixing are fixed, and the test plan is solid.
|
Review posted and approved on PR #1568. Summary of findings:
|
1 participant
Closes #1459
Summary
getServerSidePropsreturning{ props: Promise.resolve(...) }now awaits the props value before serialising, matching Next.js.Test plan
vp checkpasses