fix(examples): guard missing filmId param in react star-wars example#10143
fix(examples): guard missing filmId param in react star-wars example#10143grzdev wants to merge 1 commit intoTanStack:mainfrom
Conversation
|
📝 WalkthroughWalkthroughA refactoring of Film.tsx that destructures filmId from useParams(), adds validation for invalid film IDs, and improves type safety by changing the characters map parameter type from any to string. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/react/star-wars/src/Film.tsx (1)
8-14:⚠️ Potential issue | 🔴 CriticalRules of Hooks violation: early return before
useQuerycall.The conditional return on line 8 causes
useQuery(line 12) to be skipped on some renders, violating React's Rules of Hooks. This will produce a runtime error iffilmIdtransitions between present and absent.Move
useQueryabove the guard and use theenabledoption instead:Proposed fix
const { filmId } = useParams() + const { data, status } = useQuery({ + queryKey: ['film', filmId], + queryFn: () => getFilm(filmId!), + enabled: !!filmId, + }) + if (!filmId) { return <p>Invalid film ID</p> } - const { data, status } = useQuery({ - queryKey: ['film', filmId], - queryFn: () => getFilm(filmId), - })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/react/star-wars/src/Film.tsx` around lines 8 - 14, The early return when filmId is falsy causes a Rules of Hooks violation because useQuery is conditionally skipped; move the useQuery call for the Film component above the guard and pass enabled: Boolean(filmId) (or enabled: !!filmId) in the useQuery options so the query is inactive when filmId is missing, then keep the existing guard to render the "Invalid film ID" UI when filmId is falsy; refer to the useQuery invocation, queryKey ['film', filmId], and getFilm to locate where to change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@examples/react/star-wars/src/Film.tsx`:
- Around line 8-14: The early return when filmId is falsy causes a Rules of
Hooks violation because useQuery is conditionally skipped; move the useQuery
call for the Film component above the guard and pass enabled: Boolean(filmId)
(or enabled: !!filmId) in the useQuery options so the query is inactive when
filmId is missing, then keep the existing guard to render the "Invalid film ID"
UI when filmId is falsy; refer to the useQuery invocation, queryKey ['film',
filmId], and getFilm to locate where to change.
What
Adds a safety guard for a missing
filmIdroute parameter in the React Star Wars example.Why
The example previously used a non-null assertion: