Skip to content

remove contextSharing #4681

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

Closed
TkDodo opened this issue Dec 23, 2022 · 9 comments · Fixed by #4723
Closed

remove contextSharing #4681

TkDodo opened this issue Dec 23, 2022 · 9 comments · Fixed by #4723
Assignees
Labels
Milestone

Comments

@TkDodo
Copy link
Collaborator

TkDodo commented Dec 23, 2022

context

The QueryClientProvider has a prop contextSharing: boolean. The docs say:

Set this to true to enable context sharing, which will share the first and at least one instance of the context across the window to ensure that if React Query is used across different bundles or microfrontends they will all use the same instance of context, regardless of module scoping.

To be honest - I don't really know how this property is working. There were some discussion on this issue that suggest that it is not really useful.

For microfrontends, isolation is often preferred. With v4, we introduced the option to pass a custom context, which allows for exactly that.

If you want your app to use the same client when it's composed of multiple packages, all you'd need to do is create one QueryClient in your app and let the different bundles pick those up. As long as they all use the same version of TanStack Query, this should work fine.

proposal

  • ✅ Deprecate contextSharing in v4
  • Remove contextSharing from QueryClientProvider in v5
@Moshyfawn
Copy link
Contributor

Moshyfawn commented Dec 28, 2022

I removed contextSharing from the React QueryClientProvider in #4722. Is this what you have in mind?

Just checking if I should commit to it :D

@Moshyfawn
Copy link
Contributor

Fair enough, my bad ;)

@tludlow
Copy link

tludlow commented Dec 28, 2022

@Moshyfawn It looks like we both created PRs for this almost at the exact same time 😆. I guess it's my fault for not saying I will work on this beforehand (I didn't expect to actually finish it, I was just looking into the TanStack Query code base). It looks like you only implemented the changes for the react-query implementation. I have removed it from react, solid and vue and updated the tests + documentation where required. If you have any additions / changes you think I should add to my PR I'd be happy to work with you on anything.

@Moshyfawn
Copy link
Contributor

Moshyfawn commented Dec 28, 2022

@Moshyfawn It looks like we both created PRs for this almost at the exact same time 😆. I guess it's my fault for not saying I will work on this beforehand (I didn't expect to actually finish it, I was just looking into the TanStack Query code base). It looks like you only implemented the changes for the react-query implementation. I have removed it from react, solid and vue and updated the tests + documentation where required. If you have any additions / changes you think I should add to my PR I'd be happy to work with you on anything.

Ahaha, what are the chances :D I've closed my PR in favour of yours as you've done most of the job already.

I left a couple of suggestions on yours; please, let me know what you think ;)

@TkDodo TkDodo linked a pull request Dec 29, 2022 that will close this issue
@TkDodo TkDodo closed this as completed Jan 8, 2023
@yujiaomo
Copy link

To be honest - I don't really know how this property is working

Hi, I am not sure if I can explain the use case clearly, but I will try here
custom context does not achieve the same as the contextSharing because custom context assumes that we can all add a context provider in the outer layer that wraps several micro-frontend application, this is not a case for webpack 5 module federation. The use case is different client apps exposes different components, and a host app (which you may not have access to or the ability to add a parent common context provider) remotes the different client apps. The contextSharing allows the client apps to indicate they want to share the context without the host app knowing(which may not use react-query at all), but client apps will be able to access the same queryClient instance which is stored in the windows object. Please refer to #1764 (comment) where this property was initially added

Please reconsider this deprecation for the use case or suggest an alternative workaround without it thanks

@TkDodo
Copy link
Collaborator Author

TkDodo commented Apr 1, 2023

custom context assumes that we can all add a context provider in the outer layer that wraps several micro-frontend application, this is not a case for webpack 5 module federation

in v5, where context sharing is removed, we allow to pass a QueryClient directly instead of a context. So, you would need to create a QueryClient instance in the outer layer and pass that around. Or, you write that QueryClient to window.QueryClient and then use that to read in your micro-frontend application. That's pretty much the same that context sharing did.

Just my personal opinion: micro-frontends are supposed to be fully isolated applications. Having a shared query cache seems like it would violate that principle. You'd need to make sure that those isolated micro-frontends wouldn't write to the same location in the cache, which means they'd need to know about each other ...

@vctqs1
Copy link

vctqs1 commented Sep 18, 2023

@TkDodo Do we have any plan to use react-query without having wrapping QueryClientProvider. Like the way in v5, we can pass queryClient in useQuery directly, But we still have wrapping

<QueryClientProvider queryClient=queryClient>
....
</QueryClientProvider>

Expecting something like that but without QueryClientProvider

@TkDodo
Copy link
Collaborator Author

TkDodo commented Sep 18, 2023

@vctqs1 if you pass a queryClient instance directly to useQuery in v5, you don't need the Provider.

@vctqs1
Copy link

vctqs1 commented Sep 18, 2023

Wow, that is exactly what I'm looking for. Very glad to hear that. Thank you

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 a pull request may close this issue.

6 participants