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.

Sign up for GitHub

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

Jump to bottom

refactor: remove contextSharing #4723

Merged
merged 18 commits into from Jan 8, 2023
Merged

refactor: remove contextSharing #4723

merged 18 commits into from Jan 8, 2023

Conversation

tludlow
Copy link

@tludlow tludlow commented Dec 28, 2022 •
edited

This PR attempts to close #4681, one of the v5 react query milestone issues.

I have:

  • Removed contextSharing from react-query and updated / removed the tests.
  • Removed contextSharing from solid-query and updated / removed the tests.
  • Removed contextSharing from vue-query and updated / removed the tests.
  • Removed the contextSharing documentation from the QueryClientProvider reference documentation
  • Added a section to the v5 migration guide about the removal of this property. I think this documentation could probably be better, I used information from both the issue remove contextSharing #4681 and the linked discussion in ContextSharing not working for different modules #2810 to try and explain this. I suppose I could add an example of what I have described in this documentation, are there any good examples of how to document a code example for multiple packages like this within other parts of the documentation?

@tludlow
docs(query-client-provider-reference): remove contextSharing reference
c830600 
remove contextSharing property reference as it is being removed
@tludlow
refactor(react-query): remove contextSharing
18d0043 
remove the contextSharing property from the QueryClientProvider

BREAKING CHANGE:  contextSharing is removed from the QueryClientProvider
@tludlow
test(react-query): remove contextSharing
6606276 
remove contextSharing from QueryClientProvider tests
@tludlow
refactor(solid-query): remove contextSharing
e8fb453 
remove the contextSharing property from the QueryClientProvider

BREAKING CHANGE:  contextSharing is removed from the QueryClientProvider
@tludlow
test(solid-query): remove contextSharing
f37f655 
remove contextSharing from QueryClientProvider tests
@tludlow
refactor(vue-query): remove contextSharing
892f14d 
remove the contextSharing property from the ClientOptions

BREAKING CHANGE:  contextSharing is removed from the QueryClientProvider
@tludlow
test(vue-query): remove contextSharing
fda2849 
remove contextSharing tests and functionality from the vueQueryPlugin tests
@tludlow
docs(migrating-to-react-query-5): document the removal of contextSharing
0590427
@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 28, 2022 •
edited

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 a636682:

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

Moshyfawn
Moshyfawn reviewed Dec 28, 2022
View reviewed changes
Copy link
Contributor

@Moshyfawn Moshyfawn left a comment

Choose a reason for hiding this comment

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

I have a couple of suggestions; please, let me know what you think ;)

packages/react-query/src/QueryClientProvider.tsx Show resolved Hide resolved
packages/react-query/src/QueryClientProvider.tsx Show resolved Hide resolved
packages/react-query/src/QueryClientProvider.tsx Outdated Show resolved Hide resolved
packages/react-query/src/QueryClientProvider.tsx Outdated Show resolved Hide resolved
Moshyfawn
Moshyfawn approved these changes Dec 28, 2022
View reviewed changes
Copy link
Contributor

@Moshyfawn Moshyfawn left a comment

Choose a reason for hiding this comment

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

Easier to read and less convoluted, IMO. Thx, @tludlow!

@Moshyfawn
Copy link
Contributor

Moshyfawn commented Dec 28, 2022 •
edited

Why does clicking "Review Changes" on a specific commit approves the whole PR.. I'll never know xD

@tludlow
refactor(react-query): rename QueryClientProvider context
82003f6 
renaming this context then makes it match that of the solid-query implementation
@tludlow
refactor(solid-query): destructure query client provider props
96b5eb5 
destructuring these props then makes it match the react-query implementation
ardeora
ardeora approved these changes Dec 29, 2022
View reviewed changes
Copy link
Contributor

@ardeora ardeora left a comment •
edited

Choose a reason for hiding this comment

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

Thanks for working on this! I reviewed the solid-query changes. This looks perfect :D

@codecov-commenter
Copy link

codecov-commenter commented Dec 29, 2022 •
edited

Codecov Report

❗ No coverage uploaded for pull request base (v5@59a34da). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files 
@@          Coverage Diff          @@
##             v5    #4723   +/-   ##
=====================================
  Coverage      ?   90.73%           
=====================================
  Files         ?       89           
  Lines         ?     3487           
  Branches      ?      896           
=====================================
  Hits          ?     3164           
  Misses        ?      301           
  Partials      ?       22

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 TkDodo linked an issue Dec 29, 2022 that may be closed by this pull request
remove contextSharing #4681
Closed
@TkDodo
Copy link
Collaborator

TkDodo commented Jan 5, 2023

can you please fix the conflicts ?

@tludlow
Copy link
Author

tludlow commented Jan 5, 2023

can you please fix the conflicts ?

Fixed.

Note to self: don't use the github web merge conflicts tool

@TkDodo TkDodo merged commit 3b45591 into TanStack:v5 Jan 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TkDodo TkDodo TkDodo approved these changes

@Moshyfawn Moshyfawn Moshyfawn approved these changes

@DamianOsipiuk DamianOsipiuk DamianOsipiuk left review comments

@ardeora ardeora ardeora approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

remove contextSharing
6 participants
@tludlow @Moshyfawn @codecov-commenter @TkDodo @DamianOsipiuk @ardeora