Skip to content

ContextSharing not working for different modules #2810

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
reh2ur opened this issue Oct 19, 2021 · 25 comments
Closed

ContextSharing not working for different modules #2810

reh2ur opened this issue Oct 19, 2021 · 25 comments

Comments

@reh2ur
Copy link

reh2ur commented Oct 19, 2021

Describe the bug
When using the QueryClientProvider in an npm package and requiring the query client via useQueryClient in the consumer app, I am always getting the following error:

Error: No QueryClient set, use QueryClientProvider to set one

It seems like the logic for getting the shared context is broken.

Expected behavior
Using the useQueryClient should check for the shared ReactQueryClientContext and use it if existing.

Additional context
With #1912 the context sharing was disabled by default, and seems to have been disabled at all.
It seems like the QueryClientSharingContext is always false, as it is initialized with that value:
https://github.com/tannerlinsley/react-query/blob/05a8057e2be64915d2a12b716d8131f113598dbd/src/react/QueryClientProvider.tsx#L12
https://github.com/tannerlinsley/react-query/blob/05a8057e2be64915d2a12b716d8131f113598dbd/src/react/QueryClientProvider.tsx#L34

Shouldn't the context be initialized with whether there is a shared context or a boolean value, passed to the useQueryClient hook?

@TkDodo
Copy link
Collaborator

TkDodo commented Oct 19, 2021

right, I think you'd need a QueryClientProvider to turn contextSharing on, which kinda defeats the purpose 🤔 . Can you create a codesandbox example that works with a version before #1912 was merged and doesn't work with the latest version? Thanks.

@reh2ur
Copy link
Author

reh2ur commented Oct 19, 2021

Thanks for the quick reply! Actually I found the issue: I had a higher version of React-Query in my peer dependencies for my npm module. That's why it wasn't using the same version in both projects. Took me a while to figure that out...
Sorry for any inconveniences and never mind :)

@reh2ur reh2ur closed this as completed Oct 19, 2021
@TkDodo
Copy link
Collaborator

TkDodo commented Oct 19, 2021

okay cool, but maybe I'm not fully understanding contextSharing then 😅 . Can you maybe tell me what you are doing with it and how it works for you? Does it allow you to call useQuery without being in a QueryClientProvider ?

@reh2ur
Copy link
Author

reh2ur commented Oct 19, 2021

Sure thing!
We are experimenting with extracting shared logic from multiple NextJS projects into an internally used library. I am trying to create a reusable Wrapper component that encapsulates UI elements as well as support for data fetching. This <Wrapper /> would take care of rendering the QueryClientProvider with an appropriate client, so that the consumer - as a child of Wrapper - could just call useQuery without having to deal with the actual client.

The library has react-query as a peer dependency, which is why the useQuery hooks of the consumer app and the QueryClientProvider of our shared module have the same react-query instances.

Of course, if the peer deps version does not match with the consumer version, webpack will use two different instances for both of them, which is why the context was not shared 😅

@vsylvestre
Copy link

@reh2ur I'd be curious to have more details as to how you fixed this, since I'm in a very similar situation.

We have a “shared library” which is imported as a package in various projects within a monorepo. I‘m trying to transfer what we call the DataFetchingLayer component, which contains the logic to instantiate the query client and wraps everything with QueryClientProvider, into that library to be shared by the different projects. However, I get the same error you mentioned in your OP, caused by the use of useQuery in the projects that import the library. The only way I can fix this is by having another QueryClientProvider on the projects’ side, which kind of defeats the purpose of having it in a shared library.

Similarly to your situation, we have react-query as a peer dependency in the library. But the version is the same as that of the projects‘. They‘re all bundled with Webpack, so I added react-query as an “external” in my Webpack config, and that didn‘t help. I tried bumping react-query to the latest version for all of them, but that didn‘t work either.

Did you happen to do anything else other than making sure that the versions matched?

@scottdickerson
Copy link

scottdickerson commented Jun 10, 2022

this is exactly what we're trying and this is still happening for me as well @vsylvestre and @reh2ur, details here: #3697

I checked the react-query versions, they are the same between consumer and provider as well, peerDependencies and devDependencies on the MFE provider of react-query: 3.39.1, and dependencies on the consumer side of react-query: 3.39.1

@TkDodo
Copy link
Collaborator

TkDodo commented Jun 10, 2022

you'd need to provide a reproducible example of some sorts please.
also, in v4, there is the option to pass a custom query context for scoped providers. Have a look at the example in the migration guide: https://tanstack.com/query/v4/docs/guides/migrating-to-react-query-4#custom-contexts-for-multiple-providers

@scottdickerson
Copy link

I was able to force both MFEs to use the same react-query instance which fixed this issue:

 const config: ModuleFederationPluginOptions = {
    name: userConfig.moduleName,
    filename: 'remoteEntry.js',
    remotes: getRemotes(userConfig),
    exposes: userConfig.exposes,
    shared: {
      ...packages.dependencies,
      react: {
        singleton: true,
        requiredVersion: '>= 16.14.0',
      },
      'react-dom': {
        singleton: true,
        requiredVersion: '>= 16.14.0',
      },
      'react-query': {
        singleton: true,
        requiredVersion: '>= 3.39.1',
      },
    },
  };

@petejodo
Copy link

I do think context sharing is broken and that's because the QueryClientSharingContext is created across all instances of react-query and so the sharing context itself isn't shared. That context is the way the value passed in for contextSharing in QueryClientProvider by passing it into that shared context's provider.

That's how useQueryClient knows to use context sharing

Here's what a fix could maybe look like

@TkDodo
Copy link
Collaborator

TkDodo commented Aug 12, 2022

I'm honestly pretty unsure what contextSharing is and what it's supposed to be doing. Is it necessary / useful to anyone? Isn't the new feature in v4 enough where you can pass a custom context around? If so, I would be in favour of deprecating it ...

@petejodo
Copy link

petejodo commented Aug 15, 2022

yeah I'm not sure. What's weird is that it's not absolutely clear how you'd use either feature. Because providers are rendered at the top of the tree, in a microfrontend setup every separate module would still have to be passed the context, check if a context was already provided and if not, render a provider. This could be done very similarly without passing context though and just be done with only the client. Here's the example of a module's provider:

import React from 'react';
import { QueryClient, QueryClientProvider, useQueryClient } from '@tanstack/react-query';

type ProviderProps = {
  children: React.ReactNode;
  client: QueryClient;
};

export default function Provider({ children, client }: ProviderProps) {
  try {
    useQueryClient();
    return children;
  } catch (_e) {
    return (
      <QueryClientProvider
        // if client instance types match, then we know the same version of react-query got loaded
        // otherwise just create a new client for this module
        client={client instanceof QueryClient ? client : new QueryClient()}
      >
        {children}
      </QueryClientProvider>
    );
  }
}

with context getting passed, it would look very similar:

import React from 'react';
import { QueryClient, QueryClientProvider, useQueryClient } from '@tanstack/react-query';

type ProviderProps = {
  children: React.ReactNode;
  client: QueryClient;
  context: React.Context<QueryClient | undefined>;
};

export default function Provider({ children, client, context }: ProviderProps) {
  try {
    useQueryClient({ context });
    return children;
  } catch (_e) {
    return (
      <QueryClientProvider
        client={client instanceof QueryClient ? client : new QueryClient()}
        // if a client is provided from above but none exists in context and client instance types match
        // add it to the provided context
        context={client instanceof QueryClient ? context : undefined}
      >
        {children}
      </QueryClientProvider>
    );
  }
}

and so I'm not exactly sure what the purpose of context is either. And actually the example without context is a bit nicer because it prevents the need for passing it to all usages of useQueryClient

Edit the non-context example does however break react query devtools

@ecyrbe
Copy link
Contributor

ecyrbe commented Oct 2, 2022

you'd need to provide a reproducible example of some sorts please. also, in v4, there is the option to pass a custom query context for scoped providers. Have a look at the example in the migration guide: https://tanstack.com/query/v4/docs/guides/migrating-to-react-query-4#custom-contexts-for-multiple-providers

So like i said on V5 discussion context sharing is used in the context of microfrontends.

Big picture here :
Context sharing allows users of microfrontend to evoid handling context instanciation manually and run into issues of which one should share the context. More elaborate answer on this in v5 discussion.

Now context isolation is an option if handled by each microfrontend to override the default react-query context instance.
But it would be bad for performance reasons.
Users of microfrontend will indeed prefer to share context to take advantage of shared cache and evoid duplicate request of common server states. For this to work, you then need to have some kind of key normalisation in your project.

So you need to either:

  • activate today shared context for react to be happy else react tells you that context already exists (react need to be a shared singleton library to work properly in microfrontends)
  • instanciate an isolated context in your microfrontend (haven't tried this option yet, performance concerns)
  • instanciate a shared context in a singleton shared library (with Module Federation ) that is a superset of react-query. This common library would need to do what react-query does today with context sharing. So reinvating the wheel in each project with the possibility of doing it badly. (haven't tried this option yet)

@TkDodo
Copy link
Collaborator

TkDodo commented Oct 3, 2022

@ecyrbe I'm not entirely sure how you've setup microfrontends, but why wouldn't the hosting application just create the QueryClient and the Provider, and it would then be picked up by the microfrontends when they call useQueryClient ?

Are the microfrontends mountings themselves to different roots? If so, creating the context and passing it to the microfrontends as shown in the custom-contexts docs should work? Albeit a bit more boilerplate as every call to useQuery / useQueryClient would need to use that context ...

I just have the feeling that taking the context and putting it onto the window object is also a bit dirty.

@ecyrbe
Copy link
Contributor

ecyrbe commented Oct 4, 2022

@TkDodo

  • First a quick higlight : when using a library in a micro frontend you need to either use it in isolation or as a shared one. And i agree that you usually want to use isolation first. But for tanstack query, you usually want to use it shared. Indeed, Tanstack query is server state that you usually want to share for perf reasons, while application state is what you want to isolate.
  • Like i said. React/tanstack query should not be a dependancy of your host. Because your host is the one that should be agnostic to frameworks/libs. So you can't import react/tanstack in the host to share react query, this would break isolation
  • What will be done if you deprecate shared state, is the third option i listed, if you want to share server state. It would be to let micro frontend users handle the shared state themselves. I'm not saying it's impossible to do. But it will certainly be a big pain point compared to what is available now.
  • Now about the solutions on how to share state inside a shared singleton library. There are many options if you know what technology is used for doing micro frontends, if there is a cross communication channel setup in the microfrontends, etc. But tanstack query can't possibly know any of that. So the easiest way of sharing state is indeed to use the global this object.

@TkDodo
Copy link
Collaborator

TkDodo commented Oct 7, 2022

First a quick higlight

I understand, make sense.

So you can't import react/tanstack in the host to share react query, this would break isolation

That i don't understand so much. If I want to share a QueryClient between MFEs, why is that not the responsibility of the hosting app? It doesn't need to know about react - it just needs a dependency to @tanstack/query-core, where the queryClient lives.

Isn't it in general a bit magical that the first MFE that mounts decides what the context is? Can that not lead to random behaviour?

@ecyrbe
Copy link
Contributor

ecyrbe commented Oct 7, 2022

The issue seems to be with the context setup more than query client instanciation.
Meaning, if i only have 2 out 10 MFE that use react, i only want to setup context on those. Not on the host. Without shared context, i'll have to create a shared context myself on a shared singleton library, a bit more work, but doable.

So, If you don't want to maintain this i'll find a way to do the necessary on my app. Don't worry.

@TkDodo
Copy link
Collaborator

TkDodo commented Oct 9, 2022

@ecyrbe if there is a legit use-case for microfrontends that is not easily solvable otherwise, this could in fact be a good feature. It's just not clear to me (and others) what it does exactly. You've done a good job explaining it :)

what I still don't quite understand is: We are not sharing the QueryClient - merely the React context. so how does this share data between MFEs?

@ecyrbe
Copy link
Contributor

ecyrbe commented Oct 9, 2022

Oh, maybe i'm misstaken too. Because i thought that using shared context was sharing QueryClient. Can one context handle many QueryClients ?

@TkDodo
Copy link
Collaborator

TkDodo commented Oct 9, 2022

Can one context handle many QueryClients ?

no 😅. In order to share data, you'd need to use the same queryClient

@ecyrbe
Copy link
Contributor

ecyrbe commented Oct 9, 2022

I should take a look at the current code implementation then. Because, i have shared context setup on all mfe, doing nothing special with query client and expecting it to use the first one instanciated. at least last time i cheked, that's what i thought was happening.
I i'll poke something to have definitive view of what current code does and let you know.

@TkDodo
Copy link
Collaborator

TkDodo commented Oct 9, 2022

Here is a quick codesandbox reproduction that renders the same App twice with two different mountpoints. Each App gets a new QueryClient() passed in, but the context is shared.

https://codesandbox.io/s/tender-dust-z7695p?file=/src/index.jsx

As you can see in the random print, the data is not shared between the two.

If you go the devtools and log window.ReactQueryClientContext, it will show an object that looks something like this:

$$typeof: Symbol(react.context)
Consumer: {$$typeof: Symbol(react.context), _context: {…}, …}
Provider: {$$typeof: Symbol(react.provider), _context: {…}}
_currentRenderer: {}
_currentRenderer2: null
_currentValue: undefined
_currentValue2: undefined
_defaultValue: null
_globalName: null
_threadCount: 0
[[Prototype]]: Object

@cmacdonnacha
Copy link

Here is a quick codesandbox reproduction that renders the same App twice with two different mountpoints. Each App gets a new QueryClient() passed in, but the context is shared.

https://codesandbox.io/s/tender-dust-z7695p?file=/src/index.jsx

As you can see in the random print, the data is not shared between the two.

If you go the devtools and log window.ReactQueryClientContext, it will show an object that looks something like this:

$$typeof: Symbol(react.context)
Consumer: {$$typeof: Symbol(react.context), _context: {…}, …}
Provider: {$$typeof: Symbol(react.provider), _context: {…}}
_currentRenderer: {}
_currentRenderer2: null
_currentValue: undefined
_currentValue2: undefined
_defaultValue: null
_globalName: null
_threadCount: 0
[[Prototype]]: Object

Hey @TkDodo

Sorry, this is going back a year. We have a scenario where we do want to share the data between both Apps. Any idea how this can be achieved?

@TkDodo
Copy link
Collaborator

TkDodo commented Nov 15, 2023

In v5, you can pass a queryClient directly to all hooks as second argument. You can "share" that client however you want to, but we are not doing anything like that.

@cmacdonnacha
Copy link

cmacdonnacha commented Nov 15, 2023

In v5, you can pass a queryClient directly to all hooks as second argument. You can "share" that client however you want to, but we are not doing anything like that.

We basically have a legacy Rails app and have replaced the traditional rails views with React based pages instead. However, we're using react-rails and that means that each high level component or page is rendered separately and is essentially its own app. For example, we have:

  • Header
  • Sidenav
  • Page content
  • Footer

Because each of these is a separate app, they all create their own new QueryClient. So I would love to have a way to share a single query client with all 4 so we can share data between them. For example when I cause a mutation in the page content, I want the header to reflect that. The hack right now is to reload the browser when we need data to update, but it's not ideal UX.

I know this is not your problem but I was wondering if you had any thoughts? Share the client via local storage? Use the window?

I guess one more simple way would be to import queryClient from a shared module and the same instance is shared since ES6 Modules are singletons and the instance is created when module is loaded. Just not sure what kind of performance issues or other knock on affects this could possibly have.

// queryClient
import { QueryClient } from 'react-query';

const queryClient = new QueryClient();

export default queryClient;
// header
import queryClient from './queryClient';
<QueryClientProvider client={queryClient}>
// pageContent
import queryClient from './queryClient';
<QueryClientProvider client={queryClient}>

@TkDodo
Copy link
Collaborator

TkDodo commented Nov 15, 2023

import queryClient from a shared module

yep this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants