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

expose CancelOptions & SetDataOptions #2855

Merged
merged 2 commits into from Oct 29, 2021
Merged

Conversation

KATT
Copy link
Contributor

@KATT KATT commented Oct 28, 2021

Background: it would be nice if I didn't have to do this :)

Technically, this is a breaking change - the other idea I had was to just expose these types in the types.ts-file but then the linter complained about circular dep.

It's hard structuring what is "internal" and what is "external" types - the way I go about it in tRPC is that I have folders called internal/ in various places and I make sure to only cherry-pick from those, and they aren't cherry-picked I do export *

@vercel
Copy link

vercel bot commented Oct 28, 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/tanstack/react-query/Hp9ECaWfwkZAMhuPW5jo1PuhoVGD
✅ Preview: https://react-query-git-fork-katt-katt-cancel-tanstack.vercel.app

@codesandbox-ci
Copy link

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.

@KATT
Copy link
Contributor Author

KATT commented Oct 28, 2021

Actually - will do another version

@KATT KATT closed this Oct 28, 2021
@KATT KATT deleted the katt/cancel branch October 28, 2021 23:29
@KATT KATT restored the katt/cancel branch October 28, 2021 23:30
@KATT KATT reopened this Oct 28, 2021
@KATT
Copy link
Contributor Author

KATT commented Oct 28, 2021

Actually number 2 - my other idea created a dependency cycle

KATT added a commit to KATT/react-query that referenced this pull request Oct 28, 2021
@TkDodo
Copy link
Collaborator

TkDodo commented Oct 29, 2021

theoretically, you could infer those types by extracting them from the Parameters of setQueryData.

Technically, this is a breaking change

why would this be a breaking change?

@KATT
Copy link
Contributor Author

KATT commented Oct 29, 2021

theoretically, you could infer those types by extracting them from the Parameters of setQueryData.

I did that at first but it doesn't work with invalidateQueries:

invalidateQueries<TPageData = unknown>(filters?: InvalidateQueryFilters<TPageData>, options?: InvalidateOptions): Promise<void>;
invalidateQueries<TPageData = unknown>(queryKey?: QueryKey, filters?: InvalidateQueryFilters<TPageData>, options?: InvalidateOptions): Promise<void>;

Technically, this is a breaking change

why would this be a breaking change?

It's changing publicly exposed types. Will break tRPC for instance since I merged that PR, but I'll just fix it and bump the react-query peer dependency once this one is merged.

@TkDodo
Copy link
Collaborator

TkDodo commented Oct 29, 2021

It's changing publicly exposed types.

I think they are only considered publicly exposed when being imported directly from react-query, like:

import { QueryContext } from 'react-query'

if you import it from 'react-query/types/core/query' I don't think that is considered public :)

also, per the docs:

Changes to types in this repository are considered non-breaking and are usually released as patch semver changes (otherwise every type enhancement would be a major version!).

So I'm happy to merge this if you are

@KATT
Copy link
Contributor Author

KATT commented Oct 29, 2021

I'm happy!

@TkDodo TkDodo merged commit f21d932 into TanStack:master Oct 29, 2021
@tannerlinsley
Copy link
Collaborator

🎉 This PR is included in version 3.29.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@KATT
Copy link
Contributor Author

KATT commented Oct 29, 2021

@all-contributors add @KATT for code? :)

@allcontributors
Copy link
Contributor

@KATT

This project's configuration file has malformed JSON: .all-contributorsrc. Error:: Unexpected token ] in JSON at position 3864

@KATT KATT deleted the katt/cancel branch October 29, 2021 23:40
@TkDodo
Copy link
Collaborator

TkDodo commented Oct 30, 2021

@KATT would you like to fix the .all-contributorsrc file, too 😁 ?

@KATT
Copy link
Contributor Author

KATT commented Nov 3, 2021

@KATT would you like to fix the .all-contributorsrc file, too 😁 ?

I can't see what's wrong with it!

@TkDodo
Copy link
Collaborator

TkDodo commented Nov 6, 2021

@KATT I think it might be the trailing comma which I added here manually 🤦

https://github.com/tannerlinsley/react-query/blob/f348673dce003e82a818cc3cc093dea79b0177ea/.all-contributorsrc#L150

@TkDodo
Copy link
Collaborator

TkDodo commented Nov 17, 2021

@all-contributors please add @KATT for code

@allcontributors
Copy link
Contributor

@TkDodo

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

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.

None yet

3 participants