fix(preact-query): allow retryOnMount when throwOnError is function#10972
fix(preact-query): allow retryOnMount when throwOnError is function#10972dfedoryshchev wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthrough
ChangesthrowOnError function retryOnMount fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.
🧹 Nitpick comments (1)
packages/preact-query/src/useQueries.ts (1)
244-248: ⚡ Quick winAdd a
useQueriesregression test for this exact path.This call-site now participates in the same
throwOnErrorfunction evaluation behavior, but the new tests shown only exerciseuseQuery(useBaseQuery). A companionuseQueriestest for both() => falseand() => truewould lock this fix against regressions on this branch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/preact-query/src/useQueries.ts` around lines 244 - 248, The code path in useQueries at the forEach loop with ensurePreventErrorBoundaryRetry now participates in throwOnError function evaluation behavior, but existing tests only cover useQuery through useBaseQuery. Add regression tests for the useQueries hook that specifically test the throwOnError behavior with both a function returning false and a function returning true to ensure this fix is protected against future regressions on this code branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/preact-query/src/useQueries.ts`:
- Around line 244-248: The code path in useQueries at the forEach loop with
ensurePreventErrorBoundaryRetry now participates in throwOnError function
evaluation behavior, but existing tests only cover useQuery through
useBaseQuery. Add regression tests for the useQueries hook that specifically
test the throwOnError behavior with both a function returning false and a
function returning true to ensure this fix is protected against future
regressions on this code branch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a70e181a-1341-4f75-bbd6-4a7ddbb6d6e4
📒 Files selected for processing (5)
.changeset/preact-query-throwonerror-function.mdpackages/preact-query/src/__tests__/useQuery.test.tsxpackages/preact-query/src/errorBoundaryUtils.tspackages/preact-query/src/useBaseQuery.tspackages/preact-query/src/useQueries.ts
Summary
The preact-query adapter never evaluates a function-form
throwOnError.ensurePreventErrorBoundaryRetrytreats anythrowOnErroras truthy and disablesretryOnMount, so a query whosethrowOnErrorfunction returnsfalseis wrongly prevented from retrying on mount.This brings preact-query in line with react-query, where the same issue was fixed in #9338: when
throwOnErroris a function, it is called against the actual query error and only suppresses retry-on-mount when it returnstrue.queryintoensurePreventErrorBoundaryRetryfromuseBaseQueryanduseQueries.throwOnErrorviashouldThrowErrorbefore deciding to disableretryOnMount.throwOnError: () => false(retries on mount) and() => true(does not).Summary by CodeRabbit