Skip to content

fix(preact-query): allow retryOnMount when throwOnError is function#10972

Open
dfedoryshchev wants to merge 1 commit into
TanStack:mainfrom
dfedoryshchev:fix/preact-query-throwonerror-function
Open

fix(preact-query): allow retryOnMount when throwOnError is function#10972
dfedoryshchev wants to merge 1 commit into
TanStack:mainfrom
dfedoryshchev:fix/preact-query-throwonerror-function

Conversation

@dfedoryshchev

@dfedoryshchev dfedoryshchev commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Summary

The preact-query adapter never evaluates a function-form throwOnError. ensurePreventErrorBoundaryRetry treats any throwOnError as truthy and disables retryOnMount, so a query whose throwOnError function returns false is wrongly prevented from retrying on mount.

This brings preact-query in line with react-query, where the same issue was fixed in #9338: when throwOnError is a function, it is called against the actual query error and only suppresses retry-on-mount when it returns true.

  • Pass the resolved query into ensurePreventErrorBoundaryRetry from useBaseQuery and useQueries.
  • Evaluate the function-form throwOnError via shouldThrowError before deciding to disable retryOnMount.
  • Added tests covering throwOnError: () => false (retries on mount) and () => true (does not).

Summary by CodeRabbit

  • Bug Fixes
    • Fixed an issue in Preact Query where queries would not retry on component mount when error-throwing behavior was defined as a dynamic function.

@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

ensurePreventErrorBoundaryRetry in preact-query is updated to accept the cached Query object and evaluate throwOnError as a function via shouldThrowError before deciding to suppress retryOnMount. Both useBaseQuery and useQueries now look up and pass the cached query. Two new tests and a changeset entry cover the fix.

Changes

throwOnError function retryOnMount fix

Layer / File(s) Summary
ensurePreventErrorBoundaryRetry logic and call sites
packages/preact-query/src/errorBoundaryUtils.ts, packages/preact-query/src/useBaseQuery.ts, packages/preact-query/src/useQueries.ts
ensurePreventErrorBoundaryRetry gains an optional query parameter and resolves throwOnError via shouldThrowError when it is a function; useBaseQuery and useQueries each fetch the cached query by hash and pass it into the updated call.
Tests and changeset
packages/preact-query/src/__tests__/useQuery.test.tsx, .changeset/preact-query-throwonerror-function.md
Two new useQuery tests verify throwOnError: () => false allows retry on remount while throwOnError: () => true suppresses it via ErrorBoundary. A changeset entry records the patch.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Hoppin' through the error maze,
A function check deserves some praise—
throwOnError once fooled the gate,
Now shouldThrowError seals its fate.
Retry on mount, no longer blocked,
The boundary's logic cleanly docked! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main fix: allowing retryOnMount when throwOnError is a function, which directly matches the core change across all modified files.
Description check ✅ Passed The description provides a clear summary of the problem and solution, explains the rationale by referencing the react-query fix, and outlines the specific changes made. However, the checklist items are not marked, which may indicate incomplete submission.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
packages/preact-query/src/useQueries.ts (1)

244-248: ⚡ Quick win

Add a useQueries regression test for this exact path.

This call-site now participates in the same throwOnError function evaluation behavior, but the new tests shown only exercise useQuery (useBaseQuery). A companion useQueries test for both () => false and () => true would 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

📥 Commits

Reviewing files that changed from the base of the PR and between 4f11927 and b04451c.

📒 Files selected for processing (5)
  • .changeset/preact-query-throwonerror-function.md
  • packages/preact-query/src/__tests__/useQuery.test.tsx
  • packages/preact-query/src/errorBoundaryUtils.ts
  • packages/preact-query/src/useBaseQuery.ts
  • packages/preact-query/src/useQueries.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant