denoland / fresh

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

Docs about serializability are misleading #2263

Closed adamzerner closed 8 months ago

adamzerner commented 8 months ago

Interactive islands (docs) seems to indicate that you can't pass non-serializable values like functions and JSX as properties to islands. However, as Sharing state between islands demonstrates, you can do so. You just have to do so from an island as opposed to a non-island component.

For example, if you're in NonIslandComponent and you try to do <Island onClick={...} />, it won't work. Similarly, if you try to do <Island slot={<SomeJsx />} />, it won't work. But, if you're inside of ParentIsland, doing <ChildIsland onClick={...} /> will work. And so will <ChildIsland slot={<SomeJsx />} />.

CAYdenberg commented 8 months ago

When you use an island within an island, the child island is really just a component. The only reason to include it in your islands folder is if you intend to use it as a top-level island/entry-point.

adamzerner commented 8 months ago

I see. Thanks for explaining.

I don't think it is relevant though. Let me elaborate and use an example.

Suppose that you have a MyButton component and want to use it to toggle whether some text is visible. The following will not work.

routes/test.tsx

import { useSignal } from "@preact/signals";
import { MyButton } from "../islands/my-button.tsx";

export default () => {
  const isVisible = useSignal(false);

  return (
    <main>
      <h1>Demo</h1>
      <section>
        {isVisible.value &&
          <div>text to hide/show</div>}
        <MyButton
          onClick={() => {
            isVisible.value = !isVisible.value;
          }}
        >
          toggle
        </MyButton>
      </section>
    </main>
  );
};

components/my-button.tsx

import { ComponentChild, JSX } from "preact";

export const MyButton = (
  { children, ...rest }:
    & { children: ComponentChild }
    & JSX.HTMLAttributes<HTMLButtonElement>,
) => {
  return <button {...rest}>{children}</button>;
};

However, if we extract out <section>...</section> into it's own island like so:

islands/toggle-section.tsx

import { MyButton } from "@/islands/my-button.tsx";
import { useSignal } from "@preact/signals";

export const ToggleSection = () => {
  const isVisible = useSignal(false);

  return (
    <section>
      {isVisible.value &&
        <div>text to hide/show</div>}
      <MyButton
        onClick={() => {
          isVisible.value = !isVisible.value;
        }}
      >
        toggle
      </MyButton>
    </section>
  );
};

and change routes/test.tsx to:

import { ToggleSection } from "@/islands/toggle-section.tsx";

export default () => {
  return (
    <main>
      <h1>Demo</h1>
      <ToggleSection />
    </main>
  );
};

it will work.

Backing up, in this example we have a need to listen for clicks on MyButton and do something afterwards. To do this, we need to pass a function to onClick of MyButton. From what I could tell, the way to do this is to have the <MyButton onClick={...}> code inside of an island.

The docs say that you can't pass non-serializable values like functions as prop values. But as this example shows, that is not true. Furthermore, being able to do so seems like an important thing because if you can't, how would you eg. build that MyButton component in such a way that lets users respond to click events?

Maybe I am missing something or thinking about this incorrectly though. If so, please let me know.

marvinhagemeister commented 8 months ago

@adamzerner The example is flawed. It won't work because routes are only ever rendered on the server. The signal condition

  {isVisible.value &&
        <div>text to hide/show</div>}

will never be re-rendered. Extracting that part to its own island and passing the signal as a prop will make it reactive.

adamzerner commented 8 months ago

After discussing on #2264, I finally understand what you guys are saying here. Inside of ToggleSection when we have <MyButton onClick={() => { ... }}>, we're not actually passing a function into an island (as a property's value). <MyButton> is not an island because <ToggleSection> is what established the island boundary.

The docs say stuff about not being able to pass functions to islands, but here with <MyButton onClick={() => { ... }}> we are not actually passing a function to an island. And furthermore, the docs do actually address this here.

This all makes sense now. Sorry for the confusion, and thank you for explaining.