Skip to content

Conversation

@prateek3255
Copy link
Contributor

Make devtools and all related helpers type safe

@vercel
Copy link

vercel bot commented Sep 18, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/tannerlinsley/react-query/47rxk7Z27kvmTYXhjfhWidzBXuF6
✅ Preview: https://react-query-git-fork-prateek3255-refactor-f86c2b-tannerlinsley.vercel.app

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 18, 2021

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:

Sandbox Source
tannerlinsley/react-query: basic Configuration
tannerlinsley/react-query: basic-typescript Configuration

@TkDodo
Copy link
Collaborator

TkDodo commented Sep 18, 2021

@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

@prateek3255
Copy link
Contributor Author

@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

image

Copy link
Collaborator

@TkDodo TkDodo left a 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

Copy link
Contributor Author

@prateek3255 prateek3255 left a 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

@TkDodo
Copy link
Collaborator

TkDodo commented Sep 18, 2021

One more thing I am not sure why prettier has added spaces at the beginning at some places

Hm, not sure either. But the devtools directory should be included in our prettier checks, as it's under src:

https://github.com/tannerlinsley/react-query/blob/afcba6066d238b9ef34386e6da4f372b3e6f7ca0/package.json#L42

so if the pipeline is not complaining, it should be fine :)

Copy link
Collaborator

@TkDodo TkDodo left a comment

Choose a reason for hiding this comment

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

very good work 🚀

@tannerlinsley
Copy link
Member

🎉 This PR is included in version 3.24.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@prateek3255 prateek3255 deleted the refactor/add-devtools-types branch October 17, 2021 08:14
Comment on lines +134 to +136
"resolutions": {
"@types/react": "^16.9.41",
"@types/react-dom": "^16.9.8"
Copy link
Collaborator

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!

Copy link
Contributor Author

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.

TkDodo added a commit to TkDodo/react-query that referenced this pull request Apr 21, 2022
TkDodo added a commit that referenced this pull request Apr 21, 2022
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants