Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: rename useErrorBoundar option to throwError #4697

Merged
merged 33 commits into from Dec 30, 2022
Merged

refactor: rename useErrorBoundar option to throwError #4697

merged 33 commits into from Dec 30, 2022

Conversation

Moshyfawn
Copy link
Contributor

@Moshyfawn Moshyfawn commented Dec 23, 2022

throwError name makes the most sense as it's a common name within request packages like ky (ex: throwHttpErrors)

Closes: #4677

@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 23, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 47acfc5:

Sandbox Source
@tanstack/query-example-react-basic-typescript Configuration
@tanstack/query-example-solid-basic-typescript Configuration
@tanstack/query-example-vue-basic Configuration

@Moshyfawn Moshyfawn marked this pull request as ready for review December 23, 2022 20:58
@TkDodo
Copy link
Collaborator

TkDodo commented Dec 25, 2022

throwHttpErrors

good point about that. Do you thin throwErrors would be better than throwError ?

Also, can you please add a paragraph to the migration guide:

## Breaking Changes
v5 is a major version, so there are some breaking changes to be aware of:

Oh, and the PR needs to go to the v5 branch please, not main 😅

@codecov-commenter
Copy link

codecov-commenter commented Dec 25, 2022

Codecov Report

❗ No coverage uploaded for pull request base (v5@67ed215). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@          Coverage Diff          @@
##             v5    #4697   +/-   ##
=====================================
  Coverage      ?   92.39%           
=====================================
  Files         ?       89           
  Lines         ?     3748           
  Branches      ?      985           
=====================================
  Hits          ?     3463           
  Misses        ?      269           
  Partials      ?       16           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Moshyfawn Moshyfawn changed the base branch from main to v5 December 28, 2022 20:53
@Moshyfawn
Copy link
Contributor Author

Moshyfawn commented Dec 28, 2022

throwHttpErrors

good point about that. Do you thin throwErrors would be better than throwError ?

I'm not sure about the plural version as it might imply that there are multiple errors being thrown, which is not the case, I believe. I'm happy to change it to the plural version if you think it is more fitting.

Although, you're able to reset the error boundary, at which point there can be another error thrown, which makes it multiple..

@Moshyfawn
Copy link
Contributor Author

Moshyfawn commented Dec 28, 2022

Also, can you please add a paragraph to the migration guide:

## Breaking Changes
v5 is a major version, so there are some breaking changes to be aware of:

Is it fine that the id is "migrating-to-react-query-5" and the guide title is "Migrating to TanStack Query v5"?

@Moshyfawn
Copy link
Contributor Author

Also, can you please add a paragraph to the migration guide:

## Breaking Changes
v5 is a major version, so there are some breaking changes to be aware of:

Here you go 04b033b

@TkDodo
Copy link
Collaborator

TkDodo commented Dec 29, 2022

Is it fine that the id is "migrating-to-react-query-5" and the guide title is "Migrating to TanStack Query v5"?

hm, the id is the slug in the url. So maybe it should just be migrate-to-v5. I can do this directly on the v5 branch though.

I'm not sure about the plural version as it might imply that there are multiple errors being thrown, which is not the case, I believe. I'm happy to change it to the plural version if you think it is more fitting.

Although, you're able to reset the error boundary, at which point there can be another error thrown, which makes it multiple..

yeah let'a go with throwErrors please. It can be different kind of errors, like an error from the queryFn or an error from the selectFn. It can also be different errors at different times as you mentioned. I would read it as: "throw all errors that could potentially occur"

@Moshyfawn
Copy link
Contributor Author

Moshyfawn commented Dec 30, 2022

yeah let'a go with throwErrors please. It can be different kind of errors, like an error from the queryFn or an error from the selectFn. It can also be different errors at different times as you mentioned. I would read it as: "throw all errors that could potentially occur"

Makes sense! Will do

@TkDodo TkDodo merged commit af2e27e into TanStack:v5 Dec 30, 2022
@Moshyfawn Moshyfawn deleted the refactor/throw-error branch December 30, 2022 21:30
@TkDodo
Copy link
Collaborator

TkDodo commented Dec 30, 2022

@allcontributors add @Moshyfawn for code

@allcontributors
Copy link
Contributor

@TkDodo

I've put up a pull request to add @Moshyfawn! 🎉

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.

rename useErrorBoundary
3 participants