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

[Astro] createResource results get mixed up when rendering server only components with Suspense #2131

Open
PeterDraex opened this issue Apr 16, 2024 · 9 comments
Labels
bug Something isn't working

Comments

@PeterDraex
Copy link

Describe the bug

resource.data() is returning data from another resource, in another component

Your Example Website or App

https://stackblitz.com/edit/github-pqy8j7-y8z78u?file=src%2Fcomponents%2FPage.tsx

Steps to Reproduce the Bug or Issue

  1. Open Page.tsx
  2. See that pageQuery resolves to 'pageQuery API response' in it's fetcher function, but when you check pageQuery.data(), it contains "Component A API response"

Expected behavior

This text is displayed

pageQuery.data: "pageQuery API response"

Screenshots or Videos

No response

Platform

Windows, Chrome 123

Additional context

@birkskyum:

It appear to happen because the data from the CompA resource isn't used anywhere, and then it ends up being picked up inside Page instead of the resource defined later there...

@ryansolid
Copy link
Member

The setup for this is really odd. In that Astro seems to SSR Solid twice at least in dev and that is causing a whole bunch of chaos (not to mention sort of terrible for performance). I'm going to try to comment out the Astro code and see if there is any issue on Solid's side.

@PeterDraex
Copy link
Author

Astro seems to SSR Solid twice

Unfortunately, that's by design according to withastro/astro#8368

Unfortunately this is because we can't detect if a given function is a Solid component until we run it. Our React and Preact integrations do the same thing.

@ryansolid
Copy link
Member

Ok positive is I don't think this is the cause even if it causes some weirdness in terms of multiple copies of resources. I will continue to investigate.

@ryansolid
Copy link
Member

Ok I see the problem I just need to decide where the fix goes. There is an optimization it looks like someone put in to Solid-Astro integration to generate the non-hydration markup when you don't add client directives to your Solid components. NoHydration doesn't increment component IDs and parallel Suspense is resetting at the same id making those resources have identical IDs.

I admit I wasn't expecting this optimization. But I think this could happen in Solid's own Island implementation so I guess I need to make sure Suspense generates its own unique IDs. It will add depth but I'm not sure how else to avoid it.

@ryansolid ryansolid changed the title createResource results get mixed up in SSR [Astro] createResource results get mixed up when rendering server only components with Suspense Apr 17, 2024
@ryansolid ryansolid added the bug Something isn't working label Apr 17, 2024
@PeterDraex
Copy link
Author

Hi, can I ask if this will be fixed in foreseeable future? Or is there any other way to avoid sending serialized data to the browser?

@ryansolid
Copy link
Member

ryansolid commented Jun 28, 2024

It's tricky to fix. I'd kinda rather see the Astro adapter drop the optimization in the short term rather than try to introduce new alternative functionality into Solid. I'm doing a release today and timeboxing it. I've been looking at this for the last hour if I can solve it in the next it goes in it, otherwise it will take some time.

Part of the challenge is reducing it to the minimal actually correct example because like data.loading doesn't Suspend and if the second boundary suspends there is no problem either. So I'm not sure what the bug is I can just see the symptoms of it. LIke in the reproduction no one is reading the data and triggering Suspense except the page, which is highly unusual. And it only happens when the Islands aren't hydrated. Any fix would need to not break hydration either.

@ryansolid
Copy link
Member

Oh I guess I should mention there is a no scripts option in our render function that I think could be turned on in the no hydration case like yours but that isn't what is causing this issue and it would need to be exposed or set by the Astro renderer.

The problem here is id collision during SSR. I can fix it by removing an optimization to reduce id depth with no-hydration.. basically reducing the efficiency of the no-hydration optimization but I think it will work. I will need to test it though in other SSR environments.

@ryansolid
Copy link
Member

It will work but it would break SolidStart so I can't make this a patch release. It changes the SSR behavior of libraries depending on <NoHydrate>. They will have to account for the change in ID generation. So I can't fix this now.

I think that if you are in the no hydration scenario fetch your data outside of Solid and pass it through is my recommendation. There is nothing interactive so the data fetching doesn't need to live in Solid. All the other cases seem to work.

@elite174
Copy link
Contributor

elite174 commented Oct 6, 2024

withastro/astro#12131

This could be a fix for one renderer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants