denoland / fresh

The next-gen web framework.
https://fresh.deno.dev
MIT License
12.25k stars 627 forks source link

Island Slots Don't Work With `f-client-nav` #2444

Open lishaduck opened 4 months ago

lishaduck commented 4 months ago

Hello! I've been hunting down the cause of a bug in my website on and off for the last few days, and I finally think I figured it out. It seems that partial client navigation and passing JSX as children to islands no longer (?) seem to work together.

Context

https://why-switch.deno.dev/[^1] is using HeadlessUI, which as of v2.0, requires it to be uses in a client context (#2426). I'm working around it by adding in dummy implementations and putting the actual code behind an IS_BROWSER. I recently added a component for floating "Info" buttons. It was soon noticed that the infos sometimes didn't render. I tried a few things, and it seemed fixed. Then, I noticed it again a few times yesterday and today. I was adding another page with the info components, and noticed that, when I navigated to a different page, the infos on that page displayed the same info. I looked in the inspector, and it seems that infos are all using the same <template> slot, which is the issue. If you don't start on a page with a slot, it will never get rendered. This also explains why reloading the page always fixed it. It was rerendering with the correct server serialization. For now, my solution is to just turn off f-client-nav. It works, but it isn't as nice UX.

Source

Presumably, you'll want the source, so here it is: https://github.com/PHS-TSA/webmaster-23-24/blob/miscui/src/islands/Info.tsx If you need a SSCCE, I can provide one.

[^1]: That url goes to my current development branch's last broken deployment before I disabled partial CSR, as it is what added the <Info> component.

thegaryroberts commented 4 months ago

I stumbled across the same problem today! The solution for me was to include “children” in my alternate (IS_BROWSER === false) render path, but hiding the content with CSS.

lishaduck commented 4 months ago

I'll have to try that when I get a chance, thanks for the tip!

marvinhagemeister commented 4 months ago

Had some time to look into this, but I'm not sure how I can reproduce the described issue. What I've done:

  1. Commented out the OpenAPI stuff, otherwise it crashes on startup
  2. Set f-client-nav={true} in _app.tsx
  3. Go to the about page and check the info popovers. They all work for me.
lishaduck commented 4 months ago

Did you navigate straight to the info popovers? Try navigating to a different page, then to the about page.

It's primarily found by going to /green/, click on the infos, then click on the header to /about/, and click the infos. They'll be the same infos, but reloading swaps them out, so you can see that they're different after a reload.

The issue isn't found in the attribute, it's the actual navigation.

lishaduck commented 4 months ago

Oh, and the link I posted (https://why-switch-66b9jr0wkg78.deno.dev/) reproduces fine. You don't need to clone it (though I guess you'd still want to to verify my hunch). It's associated with commit 0c2630041a52c1103e60ef45718aa6dfb9a7af0a for the last broken source. (The commit itself is unrelated. The one that introduced the bug is literally just the one that created the <Info>s. They've always been broken.) I'll try to see if I can find some time today to see if @thegaryroberts's solution works.

marvinhagemeister commented 4 months ago

Ah you're right. For some reason I missed the live link, thanks for helping me find it. I can reproduce the issue now.

lishaduck commented 4 months ago

I can verify that @thegaryroberts's solution works.

The diff: [^1] ```diff diff --git a/src/islands/Info.tsx b/src/islands/Info.tsx index 94b8f6a..0fd491b 100644 --- a/src/islands/Info.tsx +++ b/src/islands/Info.tsx @@ -21,6 +21,7 @@ export function Info({ children }: InfoProps): JSX.Element { )} > + ); } diff --git a/src/routes/_app.tsx b/src/routes/_app.tsx index d8c0dd1..f56b5ca 100644 --- a/src/routes/_app.tsx +++ b/src/routes/_app.tsx @@ -74,7 +74,7 @@ export default function App({ Component }: PageProps): JSX.Element { - + ```