Skip to content

Conversation

lucasweng
Copy link
Contributor

Summary

Note: To keep the diff focused, this PR does not implement onError, onSettled, and useErrorBoundary. These will be addressed in a separate PR.

Test Plan

  • Tests pass locally
  • Package builds successfully

Copy link

pkg-pr-new bot commented Aug 24, 2025

More templates

@tanstack/db

npm i https://pkg.pr.new/@tanstack/db@441

@tanstack/db-ivm

npm i https://pkg.pr.new/@tanstack/db-ivm@441

@tanstack/electric-db-collection

npm i https://pkg.pr.new/@tanstack/electric-db-collection@441

@tanstack/query-db-collection

npm i https://pkg.pr.new/@tanstack/query-db-collection@441

@tanstack/react-db

npm i https://pkg.pr.new/@tanstack/react-db@441

@tanstack/solid-db

npm i https://pkg.pr.new/@tanstack/solid-db@441

@tanstack/svelte-db

npm i https://pkg.pr.new/@tanstack/svelte-db@441

@tanstack/trailbase-db-collection

npm i https://pkg.pr.new/@tanstack/trailbase-db-collection@441

@tanstack/vue-db

npm i https://pkg.pr.new/@tanstack/vue-db@441

commit: d7e8009

@KyleAMathews
Copy link
Collaborator

Copy link

changeset-bot bot commented Aug 27, 2025

🦋 Changeset detected

Latest commit: d7e8009

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@tanstack/query-db-collection Patch
@tanstack/db-example-react-todo Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@lucasweng
Copy link
Contributor Author

lucasweng commented Aug 27, 2025

ChatGPT's review https://chatgpt.com/share/68add190-8f64-800c-bc40-1218ca9d9e7f

Thanks @KyleAMathews. I updated the error tracking logic and added a retry test in a7cf838 per the review.

I didn't adopt the suggestion to have clearError throw on refetch failure though. The method's contract is to reset the error state, and throwing an exception could make it appear that the error state wasn't successfully cleared when it actually was, which may confuse users. If callers need to handle refetch failures, they can check the errorCount or isError state after calling clearError. Would like to know your thoughts on this 🙏

Edited: 62d3e86 added documentation for error tracking utils. The onError example in #347 will be included after onError is implemented.

Edited: 1947a57 Thinking about clearError again, it's simpler to just throw an error if the refetch fails. This is easier for users to handle than checking errorCount after it's been reset. To avoid the confusion I mentioned, I updated the JSDoc for clearError explaining that it can throw an error if the refetch fails."

Copy link
Collaborator

@KyleAMathews KyleAMathews left a comment

Choose a reason for hiding this comment

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

Great work!

Sorry for the delay getting this in. I'm just starting to come back from parental leave.

@KyleAMathews
Copy link
Collaborator

@lucasweng btw I made a few follow-up issues (linked here in the PR). Feel free to pick up any of them!

@lucasweng
Copy link
Contributor Author

lucasweng commented Sep 11, 2025

Sorry for the delay getting this in. I'm just starting to come back from parental leave.

No problems at all! Thanks for taking time to look into this 🙏🏻

Uziniii pushed a commit to Uziniii/db that referenced this pull request Sep 19, 2025
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.

2 participants