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

feat(devtools): enable setting loading/error via devtools #4352

Merged
merged 41 commits into from
Mar 15, 2023

Conversation

paul-sachs
Copy link
Contributor

@paul-sachs paul-sachs commented Oct 21, 2022

Allows the devtools to toggle loading and errors states (at least until the next fetch is triggered).

Things to do:

  • Make sure onError get's triggered
Screen.Recording.2022-10-21.at.10.41.06.AM.mov

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 21, 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 a4ff17e:

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

@TkDodo TkDodo marked this pull request as ready for review January 23, 2023 12:10
@codecov-commenter
Copy link

codecov-commenter commented Jan 23, 2023

Codecov Report

Base: 91.84% // Head: 92.16% // Increases project coverage by +0.31% 🎉

Coverage data is based on head (356e26e) compared to base (48e806b).
Patch coverage: 97.56% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4352      +/-   ##
==========================================
+ Coverage   91.84%   92.16%   +0.31%     
==========================================
  Files         111      111              
  Lines        4146     4187      +41     
  Branches     1072     1087      +15     
==========================================
+ Hits         3808     3859      +51     
+ Misses        317      307      -10     
  Partials       21       21              
Impacted Files Coverage Δ
packages/query-core/src/query.ts 99.02% <ø> (ø)
...kages/react-query-devtools/src/styledComponents.ts 92.85% <ø> (ø)
packages/react-query-devtools/src/devtools.tsx 89.09% <97.56%> (+5.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.

@TkDodo
Copy link
Collaborator

TkDodo commented Jan 23, 2023

@paul-sachs thanks for this PR, and sorry that it went unnoticed for a while. I've fixed some issues and did a bit of refactoring.

What would be missing from my perspective would be:

  • some tests of the new behaviour
  • a way to make the loading state revert back when clicking the button a second time, and maybe some visual feedback that we are now in loading state (similar to what the offline toggle button does)

would you like to continue contributing this?

@paul-sachs
Copy link
Contributor Author

Hi @TkDodo I'll take a look for sure. I'll need to familiarize myself with the testing setup but that should be fine. Storing the previous state is a little tricky if I remember correctly. I think we can have it retrigger the query to restore behaviour. Either way, I'll look into it.

@TkDodo
Copy link
Collaborator

TkDodo commented Jan 23, 2023

I think we can have it retrigger the query to restore behaviour. Either way, I'll look into it.

That's fine - I like the resetQueries approach that you've already taken for the error state. But the only way to get out of the loading state right now is to click refetch, and I think it would be good if the button behaved like a toggle. Alternatively, we could keep the last state in a ref before overwriting it and then restore it maybe?

@paul-sachs
Copy link
Contributor Author

I'd worry what would happen if something toggles loading state during loading. I'm not sure how setState works internally, don't know if it will abort the on going promise. Feels a little like reset is our safest bet. Either way really, the toggle behaviour makes sense.

@paul-sachs
Copy link
Contributor Author

paul-sachs commented Jan 23, 2023

So I've fixed the toggle state, however this approach doesn't seem to work in suspense mode since the setState doesn't trigger a promise... This will need something else to work through that issue.

@paul-sachs
Copy link
Contributor Author

Was hoping to do something like:

activeQuery.fetch({
  queryFn: () => new Promise((resolve, reject) => {}),
  cacheTime: -1,
})

But i don't think that's replacing the queryFn like I thought it would. It triggers a refetch of the existing queryFn...

@paul-sachs
Copy link
Contributor Author

Ah no, i was incorrect. It is using the updated queryFn. At least, I think it is. The problem is trying to store the previous queryFn so I can restore it...

@paul-sachs
Copy link
Contributor Author

paul-sachs commented Jan 23, 2023

Alright, managed to get suspense working. Had to store some query state inside queryMeta so not sure I feel great about that, but it does keep query state all together. Maybe there's a better place to store it?

Next, I'll take a look at the testing infrastructure to figure out how to write some tests.

@paul-sachs
Copy link
Contributor Author

paul-sachs commented Jan 23, 2023

@TkDodo Tests are up and green. I've represented the current state of the query in the buttons themselves but let me know if you want a totally different direction.

@vercel
Copy link

vercel bot commented Jan 26, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated
query ⬜️ Ignored (Inspect) Mar 15, 2023 at 8:09AM (UTC)

@paul-sachs
Copy link
Contributor Author

Alright, finally got to fixing prettier + lint issues.

@TkDodo TkDodo mentioned this pull request Mar 14, 2023
9 tasks
TkDodo and others added 25 commits March 15, 2023 09:07
* chore: extract package validation to an extra script

and run it during CI

* chore: add missing `build:types` script to solid-query
* docs: update link for v2 docs

* Update README.md

---------

Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
…k#5063)

Do not stretch query status label (fresh, fetching, paused, stale, inactive) shown on Query Details view.
…anStack#5041)

Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
…utationCache (TanStack#5075)

* feat(query-core): Add global onSettled callbacks for QueryCache and MutationCache

* test: tests for query onSettled callback

* test: tests for mutation onSettled callback

* docs: onSettled callbacks
this fixes an issue around stale closures where callbacks are not updated, thus are called with wrong values in the closure
* fix(eslint-plugin): improve object property checks

* prettier
* adding quick caching test

* reverting workflow change

* updating pr yml to test nx speed

* fixing yml on property

* fixing yml on property

* fixing target name

* upping to running4 in parallel

* upping to running 5 in parallel

* upping to running 6 in parallel

* upping to running 7 in parallel

* upping to running 8 in parallel

* upping to running 9 in parallel

* upping to running 10 in parallel

* opting for --parallel=5

* cleaning up nx.json

* revert touching of ci.yml file

* reverting on property of pr.yml file

* updating root pacakge.json test:lib command

* fixing frozen lockfile error in ci

---------

Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* chore: include scripts directory in linting

and fix issues

* chore: add missing dependencies chalk and semver
@TkDodo TkDodo merged commit b7089d1 into TanStack:main Mar 15, 2023
@paul-sachs
Copy link
Contributor Author

@TkDodo My apologies for letting this drop off. I'll submit a follow on PR with the suggested changes. Thanks so much for merging this, it'll make modifying loading states so much easier!

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.