Files
Nathan Nguyen f5bee1a26f fix(app-router): preserve unstable_catchError boundaries (#1906)
* fix(app-router): preserve unstable_catchError boundaries

* test(e2e): add Next-parity test for shouldRerenderAppPageWithGlobalError (#1906)

Add a test proving that for uncaught post-shell RSC errors (with no
client boundary), the server preserves the original HTML stream and the
client renders global-error. This is the Next.js behavior that justifies
keeping shouldRerenderAppPageWithGlobalError unconditional at false.

The server cannot detect userland client boundaries like
unstable_catchError from the route manifest, so the only safe way to
get a global-error rerender is to make a targeted error-boundary test.

The test uses a loading.tsx boundary so the error is truly post-shell,
not caught by the pre-shell error-recovery catch block.

* fix(error): restore sentinel wrapper for falsy thrown values in unstable_catchError

Restores the _CatchErrorState sentinel wrapper ({ thrownValue: unknown }) so
falsy thrown values (null, undefined, 0, "") are routed to the fallback
instead of being treated as no error. Also restores ErrorInfo.error as unknown.

Adds back unit tests for falsy values, class-component lifecycle, and
unstable_retry behavior parity with the component-rendered fallback shape.

Refs: #1906

* fix(error): align ambient ErrorInfo.error type with runtime implementation

* refactor(app-router): remove shouldRerenderAppPageWithGlobalError dead code

The function always returned false and its _options parameter and
ShouldRerenderAppPageWithGlobalErrorOptions type were vestigial.
The call site at app-page-render.ts:892-904 was therefore never taken.

Remove the function, type, import, call site, and the now-redundant
unit test, leaving the shell-level recovery path in
renderAppPageHtmlStreamWithRecovery as the sole error recovery seam.

Addresses ask-bonk review comment on #1906.

* fix(test): restore typed globalAny view and document Fallback as any intent

Addresses two non-blocking review findings from ask-bonk on PR #1906:

- Restored the typed `globalThis as unknown as { window?: unknown }`
  pattern for window stubs in the unstable_catchError test section,
  replacing the `(globalThis as any).window` pattern that was a type-safety
  regression from the prior rewrite.
- Added an inline comment explaining why `Fallback as any` is necessary
  in the class-component lifecycle test: the test probes the inner
  _CatchError class directly, where the fallback function is used as a
  React component type (receiving a single `{ props, errorInfo }` object)
  rather than the public API signature `(props, errorInfo)`.

* chore: trigger CI

* fix(test): rename forwardedProps to props in unstable-catchError test

Addresses ask-bonk review finding on PR #1906: the test was seeding
the inner _CatchError class with the stale key name forwardedProps,
but the production wrapper now passes props (and isPagesRouter).
Since this test only exercises the App Router unstable_retry path
(which reads neither key), behavior was unaffected, but renaming
aligns the test with the current boundary shape for future readers.

* refactor(server): remove dead routeHasLocalBoundary field

Removing shouldRerenderAppPageWithGlobalError (commit abb416b)
deleted its only reader of routeHasLocalBoundary. The field is still
declared in the options type and populated on every render, but
nothing consumes it anymore. Removes:

- routeHasLocalBoundary from AppPageRenderOptions type
- The computation in app-page-dispatch.ts
- Two test stubs in app-page-render.test.ts

Addresses ask-bonk review finding on PR #1906.
2026-06-11 23:21:52 +01:00
..