TanStack / query

🤖 Powerful asynchronous state management, server-state utilities and data fetching for the web. TS/JS, React Query, Solid Query, Svelte Query and Vue Query.
https://tanstack.com/query
MIT License
40.3k stars 2.7k forks source link

Unable to use more than one apps with QueryClientProvider on page #1764

Closed NZainchkovskiy closed 3 years ago

NZainchkovskiy commented 3 years ago

Describe the bug In my scenario I need to host more than one react application on one page. Each of app uses react-query v3, app wrapped with QueryClientProvider (I can reproduce it with CRA apps with just your "overview" sample code (https://codesandbox.io/s/github/tannerlinsley/react-query/tree/master/examples/simple)). When I open page with these apps, only one of them running fine, and others fails with error: No QueryClient set, use QueryClientProvider to set one Maybe it's because of using window.ReactQueryClientContext for context storage of app, what makes impossible to use many react-query context on one page with many apps (https://github.com/tannerlinsley/react-query/blob/master/src/react/QueryClientProvider.tsx#L9).

If I use just one app with many QueryClientProviders it's works ok.

If you can suggest any workarounds for this issue, I will appreciate it. Thank you.

To Reproduce Steps to reproduce the behavior:

  1. Build 2 apps with example from here: https://codesandbox.io/s/github/tannerlinsley/react-query/tree/master/examples/simple
  2. Host these 2 apps with one html page (I just build two CRA apps and then add div and scripts from one app index.html to another).
  3. Serve this overall html (I did just serve -s build).
  4. See error. First app crashes, but second runs fine.

Expected behavior Every separate apps runs without react-query related errors and with react-query functionality.

Additional context I tried to add some timeout for render of second app, and it's works but only for the first serve of html. Than, after page reload errors returns.

setTimeout(() => {
  ReactDOM.render(
    <React.StrictMode>
      <App />
    </React.StrictMode>,
    document.getElementById("app2")
  );
}, 1000);
sec0ndhand commented 3 years ago

I have the same issue, we have a rails app that we are slowly moving over to react and this has been a problem since we upgraded to v3. Love the library, let us know what we can do to help!

NZainchkovskiy commented 3 years ago

I think simpliest solution will be adding optional property to QueryClientProvider and using it for placing context to window. So for such edge cases one can use unique window property for each application.

There is quick and dirty concept. I think it's far from ideal and maybe @tannerlinsley will suggest something better, but for the idea:

import React from 'react'

import { QueryClient } from '../core'

const defaultClientContextName = "ReactQueryClientContext";

const QueryClientContext = (() => {
  const context = React.createContext<QueryClient | undefined>(undefined)
  if (typeof window !== 'undefined') {
    // @ts-ignore
    window[contextName ?? defaultClientContextName]  = context
  }
  return context
})()

function getQueryClientContext(contextName?: string) {
  return typeof window !== 'undefined'
    ? // @ts-ignore
      (window[contextName ?? defaultClientContextName] as React.Context<
        QueryClient | undefined
      >) ?? QueryClientContext
    : QueryClientContext
}

export const useQueryClient = (contextName?: string) => {
  const queryClient = React.useContext(getQueryClientContext(contextName))

  if (!queryClient) {
    throw new Error('No QueryClient set, use QueryClientProvider to set one')
  }

  return queryClient
}

export interface QueryClientProviderProps {
  client: QueryClient
  contextName?: string
}

export const QueryClientProvider: React.FC<QueryClientProviderProps> = ({
  client,
  contextName,
  children,
}) => {
  React.useEffect(() => {
    client.mount()
    return () => {
      client.unmount()
    }
  }, [client])

  const Context = getQueryClientContext(contextName)

  return <Context.Provider value={client}>{children}</Context.Provider>
}
TkDodo commented 3 years ago

Why do we have to attach the queryClient to the window at all @tannerlinsley @boschni ? And why is useQueryClient using that window.ReactQueryClientContext to look it up? Should this not just be:

- const queryClient = React.useContext(getQueryClientContext())
+ const queryClient = React.useContext(QueryClientContext)
boschni commented 3 years ago

Tracked it down to: https://github.com/tannerlinsley/react-query/pull/1360/commits/e8a22e914101fb35b48f6b335ba73644417583dd .

Looks like this was added to support multiple react query versions in one app but I don't know the specifics.

tannerlinsley commented 3 years ago

I'm looking into the issue right now. This was originally necessary for React Query's context to survive beyond different bundle boundaries, otherwise, the React.context reference would be created once for each bundle involved. If React context could work by matching up context by unique ID and not equality === 🤦‍♂️, this wouldn't be an issue.

I'll report back in a bit.

tannerlinsley commented 3 years ago

So far, I haven't been able to reproduce this in the way you described. Here is a codesandbox of my reproduction attempt: https://codesandbox.io/s/nice-water-y5ojm?file=/src/index.js And this is its deployment to vercel in production: https://csb-y5ojm-o3y0wov6c.vercel.app/

The deployment seems to be working as expected with no issues, however I did notice the following error: No QueryClient set, use QueryClientProvider to set one When in development mode and when hot reloading to a new state in the app.

tannerlinsley commented 3 years ago

It's my birthday today, so I'm going to put a pin in this one and check back tomorrow. If anyone can nail down the reproduction any better, let me know!

NZainchkovskiy commented 3 years ago

There is reproduction (release bundles deployed to netlify): https://react-query-issue-demo.netlify.app/ Key difference that there is two separate bundled apps, placed to one html pages. As you can see, there is only App2 rendered, and there is errors in console. If you want, I can push the repos with source code, but it's basically CRA with typescript bootstrapped code with App.tsx changed to your demo. App1 rendered to div with "app1" id, and app2 rendered to div with "app2". Our scenario: separated react apps, hosted at asp.net web application. There is another scenario - SharePoint Framework web parts for using with SharePoint Online. SharePoint and Microsoft Teams is huge and growing enterprise platform with large developers community. I think I am a biggest fan and advocate of react-query in Russian SharePoint developers community, and if I can help somehow to bring ability of using v3 of react-query in that scenarios, I will be happy to do that.

tannerlinsley commented 3 years ago

I see. I’ll have to dig into this tomorrow.

NZainchkovskiy commented 3 years ago

Thank you. Happy birthday!

etoxin commented 3 years ago

Happy Birthday! This is an amazing library. Thank you so much.

We have a micro frontends build with multiple react apps in a single DOM. We are also getting this exact issue.

I hope you have an amazing Birthday.

adamlusted-swipejobs commented 3 years ago

If React context could work by matching up context by unique ID and not equality === 🤦‍♂️, this wouldn't be an issue.

you may be able to leverage displayName in the context.

const MyContext = React.createContext(/* some value */);
MyContext.displayName = 'MyDisplayName';

https://reactjs.org/docs/context.html#contextdisplayname

tannerlinsley commented 3 years ago

That would help the display name in the devtools, but not the larger issue unfortunately.

Tanner Linsley On Feb 10, 2021, 4:52 PM -0700, Adam Lusted notifications@github.com, wrote:

If React context could work by matching up context by unique ID and not equality === 🤦‍♂️, this wouldn't be an issue. you may be able to leverage displayName in the context. const MyContext = React.createContext(/ some value /);

MyContext.displayName = 'MyDisplayName';

https://reactjs.org/docs/context.html#contextdisplayname — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

vago commented 3 years ago

Hi, not adding much to the discussion, except for +1'ing the issue. We are also in a similar situation. We are evaluating RQ and are pretty impressed with it, but this use-case is kind of a show-stopper for us to continue. Hopefully, the fine guys behind RQ will come up with a solution soon 👍

TkDodo commented 3 years ago

@tannerlinsley we are also in the process of using multiple microfrontends on the same page, each one using / wanting their own QueryClientProvider, son I’m sure we’ll see the issue as well quite soon.

I haven’t fully understood the need for attaching to the window yet, but if we need that, I think the optional context name suggested by @NZainchkovskiy would work. It could alternatively also maybe be a name/id field on the QueryClient itself.

tannerlinsley commented 3 years ago

Again, React contexts are not matched by display name or any string matching. They are matched with a reference equality comparison.

So let’s say you have a react query dep bundled in one app and another react query dep bundled in another. These two different bundles will not resolve the same React Query module instance when loaded in same browser instance (unless you’re using module federation, which is still very early and uncommon) and thus, you will have 2 separate context instances running simultaneously. The only ways to share and use React contexts across module boundaries are:

NZainchkovskiy commented 3 years ago

Maybe I missed something, but in our scenario we don't need to share react-query context between apps. It's completely separated apps. Just need to have them running on the same html page. Now, because of using window property, it's impossible. Just to clarify the issue.

d-rodionov commented 3 years ago

The third point is working. But now each provider creates a new instance of the React context and overwrites in window.ReactQueryClientContext. If use a previously created context from window.ReactQueryClientContext the queries work in many applications on a single page.

TkDodo commented 3 years ago

I did not know that sharing contexts between bundled apps on the same page is a thing. Judging from the responses here, I would think that this should be an opt-in feature, and the default should just be whatever context does. At least we need a way to have multiple microfrontends work with independent contexts on the same page.

NZainchkovskiy commented 3 years ago

About shared location — aside mentioned opt-in, I think there's some concurrent issue. As I mentioned before, settimeout resolves issue for the first run. I suspect that second instance check for shared instance before it's been created and create a new one. Maybe some lock will help.

tannerlinsley commented 3 years ago

I've got a potential fix here: #1805, but could use some extra validation/testing outside of our unit tests (which are all single-module context 🤷‍♂️ 😂 )

NZainchkovskiy commented 3 years ago

I've got a potential fix here: #1805, but could use some extra validation/testing outside of our unit tests (which are all single-module context 🤷‍♂️ 😂 )

Thank you so much! I tested it and now it's works like a charm!

vago commented 3 years ago

Hi! Thanks for the quick fix. I tried the changes and it works great! However, I noticed following warning in browser console. (Also ensured I had latest React/ React DOM).

Warning: Detected multiple renderers concurrently rendering the same context provider. This is currently unsupported.

Something to be worried about?

tannerlinsley commented 3 years ago

I’ve never seen that before TBH. Is your app working correctly?

vago commented 3 years ago

Thanks for the reply! The app seems to working as expected. I haven't seen that before either. Here's what i did.

  1. Was previously on RQ 3.5.16
  2. Tried to add same app bundle on couple of different locations on same page.
  3. One of the apps crashed for reasons originally mentioned in this issue
  4. Upgraded RQ to latest, 3.9.7
  5. Now, both app load OK, but I get this warning every time I interact with individual apps.

Please note: This warning does not occur if I have single RQ app on page.

Hi @NZainchkovskiy, can you please confirm if you noticed this warning in your test with the latest fix? Thanks.

tannerlinsley commented 3 years ago

Are you perhaps using React dom inside of another element that is being rendered with React dom?

https://github.com/reduxjs/react-redux/issues/355#issuecomment-607988421

vago commented 3 years ago

Thanks for the suggestion. I am pretty sure that's not the case. But i will dig deeper and try to come back with a codesandbox example. Thanks for the help.

vago commented 3 years ago

Hi again, so, in spirit of trying crude approaches, I commented out all of the code within getQueryClientContext() except for the defaultContext return statement and the warning went away. Maybe it is somehow related to sharing context across multiple react query apps. The warning does mention, QueryClientProvider in the stack trace. I understand this is crude and perhaps don't want to do in the library.

I am open to investigating more, appreciate any pointers/ suggestions/ leads. Kind of hitting the wall :(. Thanks!

tannerlinsley commented 3 years ago

From what I've seen, this can happen when you use the same context instance across multiple react apps on a single page, but the functionality of the apps is unchanged. I'll do some more digging, but every other github issue on this I've found doesn't really resolve in anything other than "just disregard the warning as long as everything is working" 🤷‍♂️

vago commented 3 years ago

Hi @tannerlinsley, so, I managed to have the issue consistently show up. I created a bare bone Django app that hosts couple of react (query) apps within django template. Wasn't super sure where/ how I could host the app, so, I added the source code in git at https://github.com/vago/react-query-issue. Also, present in the repo is a demo - react_query_demo.mov that shows it in action. Hope this helps.

Also, out of curiosity, once your BroadcastChannel related experiment goes thru', would there be a need for sharing context across React apps?

vago commented 3 years ago

Hi again, did you get a chance to look at the issue? Sorry for nagging, but I am looking forward to get this solved and continue with React Query. This has sort of paused our continuation. If it were just 1 time warning on load, it would still be ok to ignore, but the warning shows up every time we interact with the app - polluting dev console and decreasing developer confidence. Since React does not seem to support sharing of contexts across apps, would it be feasible to provide a configuration/ setting where we don't use shared contexts? Or some other way.. Looking for solutions/ workarounds to get going.

Thanks again for your help!

4ndv commented 3 years ago

Using RQ 3.12.0, and still have this problem. I'm running Rails app with multiple React instances in different places, and this (No QueryClient set, use QueryClientProvider to set one) happens even if there is only one react component on the page.

matheusramos commented 3 years ago

I am having the same issue, I needed to patch the lib with patch-package to keep using it.

diff --git a/node_modules/react-query/es/react/QueryClientProvider.js b/node_modules/react-query/es/react/QueryClientProvider.js
index 931fc28..6880fe8 100644
--- a/node_modules/react-query/es/react/QueryClientProvider.js
+++ b/node_modules/react-query/es/react/QueryClientProvider.js
@@ -8,7 +8,7 @@ var defaultContext = /*#__PURE__*/React.createContext(undefined); // We share th

 function getQueryClientContext() {
   // @ts-ignore (for global)
-  if (typeof window !== 'undefined') {
+  if (false) {
     if (!window.ReactQueryClientContext) {
       window.ReactQueryClientContext = defaultContext;
     }

We "turned off" context sharing, it solved the problem, but I don't think it was the best solution.

TkDodo commented 3 years ago

With all the issues being reported here, what would you think about turning context sharing off per default and making it opt-in via a prop passed to the QueryClient upon creation @tannerlinsley ?

tannerlinsley commented 3 years ago

I would be okay with that. Or just document how to do it in your own.

Tanner Linsley On Mar 3, 2021, 4:44 PM -0500, Dominik Dorfmeister notifications@github.com, wrote:

With all the issues being reported here, what would you think about turning context sharing off per default and making it opt-in via a prop passed to the QueryClient upon creation @tannerlinsley ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

4ndv commented 3 years ago

Moved from react-rails to react_on_rails, still happening. So, that's definitely wasn't an issue :(

TkDodo commented 3 years ago

what would you think about turning context sharing off per default and making it opt-in via a prop passed to the QueryClient upon creation

I took a quick look, but not really sure how to achieve this. if we pass a flag or a custom context name to the query client constructor or to the QueryClientProvider as a prop, we can set this up on the window, but every time we call useQueryClient, we'd need to know the name before we get the queryClient. But we don't have the name yet because we have not gotten the client yet. A bit of a chicken-egg problem.

For my understanding: What happens if I mount the same microfrontend twice on the same page? Will they share the same QueryClient and QueryCache? From what I can tell, we are "just" sharing the same instance of context, but I'm having a hard time grasping what that means or what that does exactly...

TkDodo commented 3 years ago

I think I have a solution involving another context 😅 , have a look here: https://github.com/tannerlinsley/react-query/pull/1912

can someone try this out maybe on their app?

4ndv commented 3 years ago

what would you think about turning context sharing off per default and making it opt-in via a prop passed to the QueryClient upon creation

I took a quick look, but not really sure how to achieve this. if we pass a flag or a custom context name to the query client constructor or to the QueryClientProvider as a prop, we can set this up on the window, but every time we call useQueryClient, we'd need to know the name before we get the queryClient. But we don't have the name yet because we have not gotten the client yet. A bit of a chicken-egg problem.

For my understanding: What happens if I mount the same microfrontend twice on the same page? Will they share the same QueryClient and QueryCache? From what I can tell, we are "just" sharing the same instance of context, but I'm having a hard time grasping what that means or what that does exactly...

For me this version and disabling context sharing finally fixed the issue with Rails! Thank you!

TkDodo commented 3 years ago

For me this version and disabling context sharing finally fixed the issue with Rails! Thank you!

very good to hear 👍