fix: pass thenable params and searchParams to probePage()#763
Conversation
probePage() passed raw null-prototype params instead of thenable params, causing TypeError for any page using the Next.js 15+ async params pattern (await params). The probe also omitted searchParams entirely. Both issues caused the probe to silently fail, defeating its purpose of early notFound() and redirect() detection. The layout probe already used the correct thenable params — the page probe was the only inconsistent path.
commit: |
|
/bigbonk review |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean, well-tested fix for a real bug. The core change is small and correct: probePage() now receives the same thenable params/searchParams shape that buildPageElement() gives to the real render, so async-params pages no longer throw TypeError during probe.
The test coverage is thorough — unit tests for the probe logic, an explicit regression test documenting the old broken behavior, and integration tests with fixture pages exercising the full dev-server path. The searchParams conversion logic correctly mirrors the spObj construction in buildPageElement() (lines 967-978).
One minor observation below, but nothing blocking.
| const _probeSearchObj = {}; | ||
| url.searchParams.forEach(function(v, k) { | ||
| if (k in _probeSearchObj) { | ||
| _probeSearchObj[k] = Array.isArray(_probeSearchObj[k]) ? _probeSearchObj[k].concat(v) : [_probeSearchObj[k], v]; | ||
| } else { | ||
| _probeSearchObj[k] = v; | ||
| } | ||
| }); | ||
| const _asyncSearchParams = makeThenableParams(_probeSearchObj); |
There was a problem hiding this comment.
Nit: this URLSearchParams-to-plain-object conversion is duplicated from buildPageElement() (lines 967-978). They're identical except for the hasSearchParams tracking. If you wanted to reduce the duplication you could extract a small searchParamsToObject(urlSearchParams) helper at the top of the generated entry (next to makeThenableParams), but given this is codegen and the AGENTS.md guidance to keep entries thin, I wouldn't block on it — just flagging for awareness in case this conversion shows up a third time.
| it("probe silently fails when searchParams is omitted and page awaits it", async () => { | ||
| const REDIRECT_ERROR = new Error("NEXT_REDIRECT"); | ||
|
|
||
| // When the old probePage() omitted searchParams, the component received | ||
| // undefined for that prop. `await undefined` produces undefined, then | ||
| // destructuring undefined throws TypeError. The probe catches it but | ||
| // doesn't recognize it as a special error, so it returns null. | ||
| const renderPageSpecialError = vi.fn(async () => new Response(null, { status: 307 })); | ||
|
|
||
| const response = await probeAppPageBeforeRender({ | ||
| hasLoadingBoundary: false, | ||
| layoutCount: 0, | ||
| probeLayoutAt() { | ||
| return null; | ||
| }, | ||
| probePage() { | ||
| // Simulate what happens at runtime when searchParams is not passed: | ||
| // the page component receives no searchParams prop, then tries to | ||
| // destructure it after await. This throws TypeError. | ||
| return Promise.resolve().then(() => { | ||
| throw new TypeError("Cannot destructure property 'dest' of undefined"); | ||
| }); | ||
| }, | ||
| renderLayoutSpecialError() { | ||
| throw new Error("unreachable"); | ||
| }, | ||
| renderPageSpecialError, | ||
| resolveSpecialError(error) { | ||
| return error === REDIRECT_ERROR | ||
| ? { kind: "redirect", location: "/about", statusCode: 307 } | ||
| : null; | ||
| }, | ||
| runWithSuppressedHookWarning(probe) { | ||
| return probe(); | ||
| }, | ||
| }); | ||
|
|
||
| // The probe catches the TypeError but resolveSpecialError returns null | ||
| // for it (TypeError is not a special error) so the probe returns null. | ||
| // The redirect is never detected early. | ||
| expect(response).toBeNull(); | ||
| expect(renderPageSpecialError).not.toHaveBeenCalled(); | ||
| }); |
There was a problem hiding this comment.
Nice — explicitly documenting the broken behavior as a test case is a good pattern. This makes it clear what the old code path did and protects against regressions if the probe's error handling ever changes.
|
Review submitted. Here's a summary: Approved — this is a clean, well-scoped fix for a real bug. What the PR fixes: What the fix does: Computes One minor note: The URLSearchParams-to-plain-object conversion is now duplicated between the probe setup and |
2 participants
Summary
probePage()passed raw null-prototype params instead of thenable params, causing TypeError for pages usingawait params(Next.js 15+ pattern)searchParams, causing pages usingawait searchParamsto failnotFound()/redirect()detection)The fix computes
_asyncSearchParams(URLSearchParams converted to a plain object then wrapped inmakeThenableParams()) alongside_asyncLayoutParams, and passes both to theprobePage()callback — matching the same prop shape thatbuildPageElement()gives to the real render.Test plan
notFound()from an async-params page when params are thenableredirect()from an async-searchParams page when searchParams are thenableawait paramsthennotFound()returns 404await searchParamsthenredirect()returns 307tests/features.test.ts— all 264 tests pass