fresh icon indicating copy to clipboard operation
fresh copied to clipboard

Element cloning in island doesn't work as expected

Open guy-borderless opened this issue 1 year ago • 6 comments

see example repo

Only way I can clone an element on both client and server is to pass its children inside an island. e.g. islands/clone.tsx

export function Clone1H2(p: CounterProps) {
  return (
    <Clone1 count={p.count}>
      <h2 className="data-[navfocus=true]:text-xl">
        works on client and server
      </h2>
    </Clone1>
  );
}

export function Clone1(
  p: {
    children: JSX.Element;
    attributeName?: string;
  } & CounterProps,
) {
  return (
    <p.children.type
      {...p.children.props}
      data-navfocus={p.count.value % 2 === 1 ? "true" : "false"}
    />
  );
}

If all the following would work the same it would be more expected and IMO usefull: islands/clone.tsx

export function Clone1H2(p: CounterProps) {
  return (
    <Clone1 count={p.count}>
      <h2 className="data-[navfocus=true]:text-xl">
        works on client and server
      </h2>
    </Clone1>
  );
}

export function Clone1(
  p: {
    children: JSX.Element;
    attributeName?: string;
  } & CounterProps,
) {
  return (
    <p.children.type
      {...p.children.props}
      data-navfocus={p.count.value % 2 === 1 ? "true" : "false"}
    />
  );
}

export function Clone2H2(p: CounterProps) {
  return (
    <Clone2 count={p.count}>
      <h2 className="data-[navfocus=true]:text-xl">
        works only on server
      </h2>
    </Clone2>
  );
}

export function Clone2(
  p: {
    children: JSX.Element;
    attributeName?: string;
  } & CounterProps,
) {
  return (
    cloneElement(p.children, {
      ["data-navfocus"]: p.count.value % 2 === 1 ? "true" : "false",
    })
  );
}

and in index.tsx:

        <Clone1H2 count={count} />
        <Clone2H2 count={count} />
        <Clone1 count={count}>
          <h2 className="data-[navfocus=true]:text-xl">
            doesn't work
          </h2>
        </Clone1>
        <Clone2 count={count}>
          <h2 className="data-[navfocus=true]:text-xl">
            doesn't work
          </h2>
        </Clone2>
        <Clone1 count={count}>
          <H2 />
        </Clone1>
        <Clone2 count={count}>
          <H2 />
        </Clone2>

image

guy-borderless avatar May 26 '24 18:05 guy-borderless

The behavior between server components and client components is a lot weird, I have this exact issue with my first serious project

Is there a way to just only do client-side-rendering? i'd like to avoid this kind of stuff

Iuriiiiiii avatar Jun 15 '24 21:06 Iuriiiiiii

Sounds like it could be a bug with how its happening - I'm going to try recreating and take a look

Mrashes avatar Jul 03 '24 16:07 Mrashes

Can recreate - Not super well versed in this so I'm not positive how we could remediate.

to @Iuriiiiiii - You can use IS_BROWSER to client side render only. It may be beneficial if we could use the 'use client' pattern in the future (like react-server-components/nextjs) but definitely out of my purview

Mrashes avatar Jul 03 '24 16:07 Mrashes

cloneElement makes me feel weird and uncomfortable and given React considers it a "legacy API" I'm not sure how long Preact will continue to support it. Let alone at the route/island juncture which is already a bit of an edge case.

There are some suggested alternative (ie better) patterns in the React docs: https://react.dev/reference/react/cloneElement#alternatives

CAYdenberg avatar Jul 19 '24 16:07 CAYdenberg

cloneElement makes me feel weird and uncomfortable and given React considers it a "legacy API" I'm not sure how long Preact will continue to support it. Let alone at the route/island juncture which is already a bit of an edge case.

There are some suggested alternative (ie better) patterns in the React docs: https://react.dev/reference/react/cloneElement#alternatives

I was checking a pattern where the children are server rendered and a minimal island component just adds attributes. The children are styled with tailwind attribute selectors (e.g. data-[highlight]:bg-blue). This pattern allows the children to be server rendered, and in general a lot more code to be server rendered. Notwithstanding, I agree that since it's considered legacy API it's probably not a good place to put effort in. Wish this could have been done though.

guy-borderless avatar Jul 19 '24 18:07 guy-borderless

Under the hood Fresh needs to wrap children to make all the island + server rendered content able to work well together. This in turn means that cloneElement receives a wrapper element and not the original vnodes. I'm afraid I'm not aware of any way to make cloneElement work with that.

marvinhagemeister avatar May 13 '25 10:05 marvinhagemeister