-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Add types for devtools #2688
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
Add types for devtools #2688
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/tannerlinsley/react-query/47rxk7Z27kvmTYXhjfhWidzBXuF6 |
|
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 fbf6249:
|
|
@prateek3255 wow, thank you so much. This was on my list for quite some time, but I never got to it 🚀 . It will make working with the devtools so much better in the future |
|
@TkDodo I have added the types for devtools and related utils except for Explorer (the recursion was making it a bit difficult to understand and hard to add types). One more thing I am not sure why prettier has added spaces at the beginning at some places, so would recommend you use these "Hide whitespace changes" setting while reviewing so as to avoid the unnecessary white space changes |
TkDodo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks really good 👍 , just some small questions
prateek3255
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TkDodo These are a couple of places I couldn't figure out what's wrong so had to end up using any
Hm, not sure either. But the so if the pipeline is not complaining, it should be fine :) |
TkDodo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very good work 🚀
|
🎉 This PR is included in version 3.24.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
| "resolutions": { | ||
| "@types/react": "^16.9.41", | ||
| "@types/react-dom": "^16.9.8" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@prateek3255 I'm just stumbling upon this and I don't really know what this does or what this is for. I must have missed this when we merged this. Can you explain what this does or why we need it? And do we need to update it for v4 to react18 or something? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TkDodo From what I remember this was added because there was some bunch of type errors when adding DOM related types to devtools and its utils probably because of some conflicting @types/react of some other package. So adding resolutions helped resolve to the type version we wanted to use.
From what I see now is that we are not getting any type errors while compiling both on master and alpha if we remove this, so this can just be safely removed.
as discussed here: TanStack#2688 (comment)
* fix(devtools): devtools should not import relatively from react-query * fix(hydration): properly remove unused hydration entry point hydration moved to the core, and the build entry point was already removed for v4. this is just a proper cleanup. * fix(persistQueryClient): rename isHydrating to isRestoring as we currently have no plans to re-use this for useHydrate, it would be confusing to not get true for this value in those cases * fix(persistQueryClient): document useIsRestoring * fix: make QueryErrorResetBoundary value stable we want a constant value for the lifetime of the QueryErrorResetBoundary component; useMemo doesn't guarantee that. * chore: remove resolutions from package.json as discussed here: #2688 (comment) * fix: log message we don't have a queryKey at this point if a string was used due to how the overloads try to spread things * Update docs/src/pages/plugins/persistQueryClient.md

Make devtools and all related helpers type safe