Skip to content

Conversation

boschni
Copy link
Collaborator

@boschni boschni commented Aug 24, 2020

No description provided.

@vercel
Copy link

vercel bot commented Aug 24, 2020

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/f9gnm5vxe
✅ Preview: https://react-query-git-fork-boschni-feature-refetch-on-reconnect.tannerlinsley.vercel.app

@boschni boschni force-pushed the feature/refetch-on-reconnect branch from ec7c633 to 03dbb92 Compare August 24, 2020 19:09
@tannerlinsley tannerlinsley merged commit 473af3d into TanStack:master Aug 24, 2020
@tannerlinsley
Copy link
Collaborator

🎉 This PR is included in version 2.12.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

isCancelledError,
isError,
setConsole,
stableStringify,
Copy link

Choose a reason for hiding this comment

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

@boschni could you explain why stableStringify is no longer exposed? I realize this util was not documented, it was a part of the public interface exposed by the library and it ended up being a breaking change for us... It was convenient for key comparison...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi! It has been removed because it is an undocumented internal function which happened to be used for key serialization (it was also not exposed as a key serialization function). Could you explain a bit more about your use case? Then we might be able to come up with a proper API for it.

Copy link

@9still 9still Sep 10, 2020

Choose a reason for hiding this comment

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

@boschni Thanks for the reply! Our use-case is simple. We have some generic functionality in our app, which wraps react query, and simply wants to detect if the query key has changed. Because keys can be heterogeneous in structure, there is no good way to compare them. We could deep compare the objects on our own, but that requires pulling in a separate implementation, and more importantly risks not fully aligning with the comparison logic used by react-query. We assumed that since the serialization routine was exported, it was effectively the supported if undocumented approach @tannerlinsley had exposed to do just that.

I realize that you can rightly view these utils as a leaky abstraction, and truly appreciate all the work you guys are doing, but just a kind request as a semver consideration for the future:

Since the removed utils were a part of the external interface, it seems more appropriate to reserve the cleanup for a major version, with at least a comment on the PR explaining the change.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added getDefaultReactQueryConfig()in #1001 which can be to get the default queryKeySerializerFn . Would that be sufficient?

Copy link

Choose a reason for hiding this comment

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

That sounds awesome! Will check it out on Monday. Thanks so much for adding it so quickly!!!

Copy link

Choose a reason for hiding this comment

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

@boschni We've switched using the new method you added & it works great! Thanks again!

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