fix: rewrite error for react hooks without directive#844
Conversation
|
/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.
Good approach — rewriting the cryptic "useState is not a function" into an actionable error message is the right call, and the test coverage + fixture look solid.
Two issues to address:
1. Missing hooks in the regex pattern
The regex catches 11 client-only hooks, but React 19 has 14 hooks absent from the react-server export condition. Three are missing:
useActionStateuseOptimisticuseEffectEvent
All three will produce the same "X is not a function" TypeError on the server but won't be rewritten by the current pattern. They should be added.
2. Consider reusing throwClientHookError instead of duplicating the message
The error message string is now defined in two places:
shims/client-hook-error.ts:10-13—throwClientHookError()server/app-page-boundary-render.ts:412-414—rewriteClientHookError()
These serve different purposes (throw-on-call vs. rewrite-after-catch), so having two functions is fine, but the message template itself could be extracted into a shared constant or the rewrite function could import and use the same format helper. This is a minor nit — if you'd prefer to keep them separate for clarity, a comment cross-referencing the two locations would be enough. Just want to avoid them drifting apart silently.
| // "useState is not a function". Rewrite into an actionable message matching | ||
| // the format used by the next/navigation shims (see client-hook-error.ts). | ||
| const _clientHookPattern = | ||
| /\b(useState|useEffect|useReducer|useRef|useContext|useLayoutEffect|useInsertionEffect|useSyncExternalStore|useTransition|useImperativeHandle|useDeferredValue)\b.*is not a function/; |
There was a problem hiding this comment.
Three React 19 client-only hooks are missing from this pattern: useActionState, useOptimistic, and useEffectEvent. All three are absent from the react-server export condition in React 19.2.5 and will produce the same "X is not a function" TypeError when called in a Server Component.
| /\b(useState|useEffect|useReducer|useRef|useContext|useLayoutEffect|useInsertionEffect|useSyncExternalStore|useTransition|useImperativeHandle|useDeferredValue)\b.*is not a function/; | |
| const _clientHookPattern = | |
| /\b(useState|useEffect|useReducer|useRef|useContext|useLayoutEffect|useInsertionEffect|useSyncExternalStore|useTransition|useImperativeHandle|useDeferredValue|useActionState|useOptimistic|useEffectEvent)\b.*is not a function/; |
| `${match[1]}() only works in Client Components. Add the "use client" directive ` + | ||
| `at the top of the file to use it. Read more: ` + | ||
| `https://nextjs.org/docs/messages/react-client-hook-in-server-component`; | ||
| } |
There was a problem hiding this comment.
Nit: this message template is duplicated from shims/client-hook-error.ts:10-13. Consider extracting the format string into a shared helper (e.g., buildClientHookErrorMessage(hookName) in client-hook-error.ts) that both throwClientHookError and rewriteClientHookError can call. That way the wording stays in sync if it ever changes.
Alternatively, just adding a // Keep in sync with shims/client-hook-error.ts comment here would be fine.
|
Review posted successfully on PR #844. Here's a summary of the feedback: Request Changes with two items:
|
|
/bigbonk re-review |
There was a problem hiding this comment.
Both items from the previous review are addressed:
-
Hooks list is now complete. All 14 React hooks absent from the
react-serverexport condition are present in the regex. Verified against the actualreact@19.2.5package exports — the list matches exactly.react-domhooks (useFormStatus,useFormState) are correctly excluded since they come from a different import path and would produce a different error pattern. -
Shared error message.
buildClientHookErrorMessageis properly extracted inclient-hook-error.tsand imported by boththrowClientHookError(shim-time throw) andrewriteClientHookError(catch-time rewrite). Single source of truth.
Coverage analysis: rewriteClientHookError is called in renderAppPageErrorBoundary, which is the convergence point for all user-facing App Router error rendering — buildAppPageElement catch, SSR stream recovery catch, and RSC stream global error re-render all flow through it. Pages Router paths are not applicable (no Server Components, so all hooks are available).
One minor DX note (not blocking): the rscOnError callback in the generated RSC entry logs errors to the server console without the rewrite, so server-side logs during RSC streaming would still show the raw "useState is not a function" message. Only affects developer log inspection, not the browser-facing error. Could be a follow-up.
Test and fixture look good. LGTM.
|
Approved PR #844. Here's a summary of the review: Approved — both items from the previous review are addressed:
Coverage analysis: One minor non-blocking note: The |
1 participant
fixes #834