qwik icon indicating copy to clipboard operation
qwik copied to clipboard

creating an onGet in a layout and an index file collide with each other.

Open lifeiscontent opened this issue 3 years ago • 7 comments

Qwik Version

0.9.0

Operating System (or Browser)

Chrome / MacOS

Node Version (if applicable)

16.13.2

Which component is affected?

Qwik City

Expected Behaviour

when specifying multiple nested onGet calls between layout.tsx and index.tsx files I'd expect them all to resolve with their specified resource data.

Actual Behaviour

when specifying multiple nested onGet calls between layout.tsx and index.tsx files they seem to all resolve to the leaf layout/index that had an onGet defined.

Additional Information

if this is as designed, that's fine, but I'm not sure where in the qwik city docs it define this behavior.

this is the page I ran into this issue on: https://github.com/lifeiscontent/qwik-realworld/blob/main/src/routes/index.tsx and layout: https://github.com/lifeiscontent/qwik-realworld/blob/main/src/routes/layout.tsx

lifeiscontent avatar Oct 02 '22 03:10 lifeiscontent

Also, if it's helpful, this app isn't finished by any means, but this is the starting point of how I ran into this issue, any feedback on how you would do things differently would be greatly appreciated, happy to also add this as a Qwik example on the site! https://github.com/lifeiscontent/qwik-realworld

lifeiscontent avatar Oct 02 '22 03:10 lifeiscontent

It would be a nice mental model if each onGet() in the chain of nested layouts would be called. Although I think that might require a change to the useEndpoint() API so that you could specify which onGet()'s data you want to access. Perhaps this could be modelled as a kind of context? Or perhaps onGet() could call useContextProvider() to attach data to a well known context object? And then rather than useEndpoint() one could just useContext()?

petebacondarwin avatar Oct 04 '22 08:10 petebacondarwin

@petebacondarwin what is the current recommendation for fetching data in a larger application built on qwik / qwik city?

lifeiscontent avatar Oct 09 '22 17:10 lifeiscontent

If you are using QwikCity to create an MPA (i.e. no client-side navigation) then I think simply putting an onGet() handler in each page is a reasonable approach. That way you can control closely what data is retrieved for the specific page being displayed. It gets more complicated if you start using client-side navigation since the old page template can currently be executed again with the new page data. You might have more power if you make use useResource() hooks rather than relying upon onGet() handlers. I don't yet think we have nailed down what the best architectural pattern is for larger applications.

petebacondarwin avatar Oct 10 '22 09:10 petebacondarwin

not sure if it's helpful, but imo https://remix.run/ has one of the best experiences for dealing with data fetching/mutation. might be worth copying/taking inspiration from.

lifeiscontent avatar Oct 15 '22 23:10 lifeiscontent

Im also running into this issue. I have a main layout.jsx that fetches user posts to display in a sidebar using onGet, and then a pages/[slug] route that fetches individual page data using onGet. When I'm at the page/[slug] route, the main layout route returns data from the lowest onGet. I tried using useResource without any luck, but perhaps I was implementing it wrong.

EDIT: I've figured it out by using useResource instead of onGet, though I'd love to rely on onGet as its only called from the server whereas useResource is called both places, ya?

ramsaylanier avatar Oct 31 '22 22:10 ramsaylanier

Hi, i'm playing with qwik/qwik-city, it seems i'm trying to use onGet with the same structure as you @ramsaylanier

On local it seems to work, but once deployed it does not : see this PR and the last deploy on netlify

qdelettre avatar Nov 01 '22 18:11 qdelettre

thanks for the report @lifeiscontent is this still the case with the latest version?

shairez avatar Nov 29 '22 19:11 shairez

@shairez not sure, I haven't played with Qwik again since I opened the issue.

lifeiscontent avatar Dec 12 '22 01:12 lifeiscontent

thanks @lifeiscontent !

Looks like the new server actions should solve this issue will wait for Manu to closed this one after he's done

shairez avatar Dec 12 '22 10:12 shairez