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

React query not working with astro file #11499

Closed
1 task done
pycanis opened this issue Jul 18, 2024 · 19 comments
Closed
1 task done

React query not working with astro file #11499

pycanis opened this issue Jul 18, 2024 · 19 comments
Labels
needs response Issue needs response from OP needs triage Issue needs to be triaged

Comments

@pycanis
Copy link

pycanis commented Jul 18, 2024

Astro Info

Astro                    v4.11.6
Node                     v20.11.0
System                   macOS (arm64)
Package Manager          npm
Output                   static
Adapter                  none
Integrations             @astrojs/react

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

Importing a React library directly in astro layout file results in No QueryClient set, use QueryClientProvider to set one error coming from @tanstack/react-query.

If I create a new React component that uses the library and is then imported in astro layout file, it's working fine.

Have a look at a reproducible example.

What's the expected result?

Importing a react library in astro layout file directly to be working with @tanstack/react-query.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-njjsdv?file=src%2Flayouts%2FLayout.astro

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Jul 18, 2024
@ematipico
Copy link
Member

Can you please the reproduction or the PR description? The reproduction isn't using @tanstack/react-query

@ematipico ematipico added needs repro Issue needs a reproduction needs response Issue needs response from OP labels Jul 19, 2024
Copy link
Contributor

Hello @pycanis. Please provide a minimal reproduction using a GitHub repository or StackBlitz. Issues marked with needs repro will be closed if they have no activity within 3 days.

@github-actions github-actions bot removed the needs triage Issue needs to be triaged label Jul 19, 2024
@pycanis
Copy link
Author

pycanis commented Jul 19, 2024

Can you please the reproduction or the PR description? The reproduction isn't using @tanstack/react-query

The reproduction was using a library (@lofik/react) that uses @tanstack/react-query in a standard way. I've removed it and used @tanstack/react-query directly. The issue still persists.

https://stackblitz.com/edit/github-njjsdv-zcx9t1?file=src%2Fpages%2Findex.astro

  • With theclient:only="react" directive on QueryClientProvider, the error is TypeError: client.mount is not a function.
  • Without theclient:only="react" directive on QueryClientProvider, the error is Error: No QueryClient set, use QueryClientProvider to set one.

@florian-lefebvre florian-lefebvre added needs triage Issue needs to be triaged and removed needs response Issue needs response from OP needs repro Issue needs a reproduction labels Jul 19, 2024
@pycanis
Copy link
Author

pycanis commented Jul 22, 2024

Is there anything I can help with? I'd really appreciate having this looked into 🙏

@ematipico
Copy link
Member

I spent some time triaging the issue, and I believe this is expected, considering the way you're setting up your project.

Removing <Layout /> which creates some indirection, you're trying to do something like this:

<QueryClientProvider client:only="react" client={queryClient}>
  <NotWorking />
</QueryClientProvider>

You have to understand that client:only is the boundary that tells Astro which component needs to be created on the client. <QueryClientProvider /> will be deferred on the client, however all it's outside si created on the server.

In this particular example, you're creating queryClient on the server, and that's why the library is rightfully complaining.

Closing because this is working as expected. Please, next time, reach out to Discord.

@ematipico ematipico closed this as not planned Won't fix, can't repro, duplicate, stale Jul 22, 2024
@pycanis
Copy link
Author

pycanis commented Jul 22, 2024

I'm not really sure. I've updated the reproduction with the wrapper around QueryClientProvider so everything including the query client (new QueryClient()) initialization should be happening on the client. The error is the same. It seems like it's got to do with the layout because the same seems to work in pages (index.astro).

https://stackblitz.com/edit/github-njjsdv-zcx9t1

Shall I move the discussion to discord?

@ematipico
Copy link
Member

Yes please

@pycanis
Copy link
Author

pycanis commented Jul 22, 2024

Sorry for bothering you again but I'm not getting much help on discord. I've made the reproduction in the simplest way possible, could you have a look please?

It seems like the issue is with the <slot /> placeholder. When used, it's not working but the exact same component passed directly works.

https://stackblitz.com/edit/github-njjsdv-zcx9t1

Could you try to explain to me why "THIS IS NOT WORKING" string is not rendered in the UI?

@ematipico ematipico reopened this Jul 22, 2024
@joshuaiz
Copy link

@pycanis a couple issues I am seeing with your StackBlitz:

  1. In your Layout component, your <Wrapper> component is missing a client directive e.g. client:load. https://docs.astro.build/en/reference/directives-reference/#client-directives. Without that, no JS will load at all.

  2. In that same Layout component, you can't use an Astro <slot /> as a child of a React component. An Astro component can never be a "child" of a React component - only the other way around.

Your Wrapper needs to be around React components only. Once you add a React component any children must also be React. This makes sense because the Astro components are on the server and then the client-side hydration for reactive components happens down the component tree. If you place a server component in the middle of that — in this case your <slot>, Astro will not know what to do.

@ematipico ematipico added the needs response Issue needs response from OP label Jul 23, 2024
@pycanis
Copy link
Author

pycanis commented Jul 23, 2024

@joshuaiz thank you for clearing a few things up for me.

Is there currently a way to have an Astro island that uses React Context to not re-render the context on page change, only the content inside of the context?

I've updated the StackBlitz again to demonstrate.

https://stackblitz.com/edit/github-njjsdv-zcx9t1

I think it's obvious that the ContextWrapper needs to be higher up the tree but with what you pointed out, I don't see a way to do it.

My goal is to NOT see ContextWrapper rerendered! in the console on page transition.

@joshuaiz
Copy link

Hi @pycanis this is always a challenge with context + providers. Depends on what you are querying and how often you need to refresh the data but there should be a way to use useMemo() so that the context only changes if the data within changes.

Here's an example:

https://medium.com/@trisianto/react-query-how-to-memoize-results-from-usequeries-hook-eaed9a0ec700

@ematipico ematipico closed this as not planned Won't fix, can't repro, duplicate, stale Jul 23, 2024
@pycanis
Copy link
Author

pycanis commented Jul 23, 2024

It's not about the query results. I just need the same Astro island to use the same react context (QueryClientProvider in this case, but it can be anything e.g. initializing a websocket connection) - there's no point in re-rendering it if it's the same for multiple pages.

I'm starting to feel like it's not possible with Astro although this is a fairly common use case..

@joshuaiz
Copy link

Got it. I think the main point is there is no "Context" with Astro - there's no way to wrap Astro components like that. But, you can add your Context/Wrapper at the top level or anywhere else...consider these two examples:

<Layout><!-- Astro -->
    <Wrapper client:load><!-- React from here down -->
        {children}<!-- React children -->
    </Wrapper>
</Layout>

^^ Here we are adding the wrapper high in the component tree.

<Layout><!-- Astro -->
    <AstroComponent>
        <Wrapper client:load><!-- React from here down -->
            {children}<!-- React children -->
        </Wrapper>
        <AstroComponent>
            <AstroComponent>
                <slot /><!-- Astro children -->
                <Wrapper client:load><!-- React from here down -->
                    {children}<!-- React children -->
                </Wrapper>
            </AstroComponent>
        </AstroComponent>
        <AnotherAstroComponent>
            <slot /><!-- Astro children -->
        </AnotherAstroComponent>
    </AstroComponent>
</Layout>

^^ Here we are adding the first wrapper high-ish and then another wrapper further down. We also have slots that are only between Astro components. Note that the two wrappers will have their own context(!). You can't wrap your entire app in a provider like with pure React or share context across Astro islands.

@joshuaiz
Copy link

joshuaiz commented Jul 23, 2024

One more point to be made here: if you don't need the cache invalidation and re-fetching aspects of TanStack Query, you can use Astro's top-level fetch + await, fetch the data on the server in Astro and pass it down to React components if you need reactivity or in Astro if you don't:

<!-- Astro component -->
---
import Layout from '/layout/Layout.astro';

const response = await fetch('https://api.example.com/data');
const data = await response.json();
---
<Layout>
    <ReactComponent data={data} client:load />
    <!-- OR use the data in Astro -->
    {data.map((item) => (
        <div>{item.name}</div>
    ))}
    <!-- OR pass down to another Astro component -->
</Layout>

The point is: you don't need React to fetch data and fetching it on the server means the data is already ready for your components. https://docs.astro.build/en/guides/data-fetching/

@pycanis
Copy link
Author

pycanis commented Jul 24, 2024

Okay, I understand the component nesting pattern. It makes sense and what I need works with that. My main problem is that navigating between pages re-renders all the components, not only ones that changed. I admit to be "thinking in react" terms which probably doesn't suit the Astro concepts.

Regarding the data fetching - I need to be fetching on the client because I'm fetching from a local SQLite database.

Essentially I just need a part of the application that lives on /dashboard/* to behave like a typical SPA. I thought I could use the skeleton from astro component that is shared across all dashboard pages (this part being pre-rendered directly as HTML) and only have JS to take over the small part of the page, but on all dashboard pages. It seems I can do that but Astro doesn't do any diffing on page change and considers the same Astro island on another page to be a different Astro island.

Is there maybe a way to not create Astro page for every dashboard page but only the index and let the client side react take over the routing there? Probably not going to work for hard refresh e.g. on /dashboard/categories..

Sorry for pushing hard in this direction. Astro is like a breath of fresh air (coming from Nextjs) so I took a week to rewrite my app to use Astro only to learn the last piece of puzzle missing.

@GabrielPedroza
Copy link

GabrielPedroza commented Nov 5, 2024

Hi @pycanis! Running into this issue as well. Seems like its an intended side effect from astro but I wanted how you structured the solution for this. Thanks so much in advance!

edit: followed this link that you sent but doesn't seem to work from me: <https://stackblitz.com/edit/github-njjsdv-zcx9t1

@pycanis
Copy link
Author

pycanis commented Nov 5, 2024

Hey @GabrielPedroza, have a look here. The solution is having the whole SPA under single astro page. More specifically here and here. Hope that helps.

@GabrielPedroza
Copy link

Thanks for the quick reply! Just to make sure I understand, you are basically just adding a bunch of routes that will use react query to a rootRouter in a singular place (App.tsx) and you will load the corresponding component depending on the url param. So if you are in /dashboard, you will load the dashboard component on the client side utilizing the static params. Also, when you mention SPA, it is because react query is essentially being loaded in a singular react component and renders the component conditionally depending on the static param that you are in. Is that all correct?

@pycanis
Copy link
Author

pycanis commented Nov 5, 2024

React query is loaded just once when you navigate to any of the /dashboard* route, correct. The important thing is the use of hash history, which essentially makes the server serving your website think the user is still on the /dashboard route even though you might already be somewhere deeper, like /dashboard/#/statistics. That's the job of the client side router to handle.

That way, you benefit from having static pages along with full fledged client side app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs response Issue needs response from OP needs triage Issue needs to be triaged
Projects
None yet
Development

No branches or pull requests

6 participants
@ematipico @joshuaiz @florian-lefebvre @GabrielPedroza @pycanis and others