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

Children props and event handlers are lost when rendered to DOM #27

Open
adrian2x opened this issue Apr 26, 2023 · 4 comments
Open

Children props and event handlers are lost when rendered to DOM #27

adrian2x opened this issue Apr 26, 2023 · 4 comments
Labels
enhancement New feature or request preact Relates to preactement package react Relates to reactement package

Comments

@adrian2x
Copy link

adrian2x commented Apr 26, 2023

I have some code that defines a Tabs component like tab-list. I am using it like this:

<tab-list>
    <div data-key="mouse" title="Mouse">
      <DemoContent />
    </div>
    <div data-key="keyboard" title="Keyboard">Keyboard Settings</div>
    <div data-key="gamepad" title="Gamepad">Gamepad Settings</div>
</tab-list>

function DemoContent() {
  return (
    <div>
      <h2>Sample title</h2>
      <button onClick={() => alert()}>Submit</button>
    </div>
  )
}

This does not work because the children passed to the Tabs component is just an empty element without any props or children.

However, if I you the children inside a slot, they slot is a node with the correct children and props BUT the event listeners don't fire. So this renders the static content, but it is not interactive.

  1. Can we avoid using the slot and parse any children correctly?
  2. How can we get the event listeners for nested components to work?
@adrian2x
Copy link
Author

I'm happy to open a PR for this if you got any tips on how it can be implemented. I'm not a preact expert but I'm very curious about their rendering APIs and how it can interop with other libraries, like this one!

@jahilldev
Copy link
Owner

jahilldev commented May 6, 2023

Hi @adrian2x,

Thanks for reaching out.

It'd be really helpful if you could provide a simple reproduction of the issue for me to look at.

That way I can more easily identify exactly where any improvements to the package might be made.

Once I have that, I can point you in the right direction 👍

@adrian2x
Copy link
Author

adrian2x commented May 8, 2023

@jahilldev
The preact repro case - https://stackblitz.com/edit/vitejs-vite-d4gtz6?file=src/app.jsx

Here's a react repro too, for comparison - https://stackblitz.com/edit/react-kyo5xw?file=src/App.js

In both cases, I have a slot that includes a nested component and regular children passed into the custom element. Neither approach has the event handlers working correctly.

@jahilldev
Copy link
Owner

jahilldev commented Jul 4, 2023

Hi @adrian2x,

I've finally had a chance to take a look at this for you; So this really comes down to how the top level children prop works for hydrated custom elements.

When the custom element is bootstrapped on the client, all HTML content within the custom element (tab-list in your case) is assigned to a property on that custom element as a string (which is then parsed into a render tree). This means the custom element has no knowledge of the (P)React component that renders it initially, hence no event listeners.

The obvious solution to this is to move your <Nested /> component into the render of the custom element itself, e.g:

function TabList() {
  return (
    <div style={{ border: '1px solid red' }}>
      {/*[...]*/}
      <Nested />
    </div>
  );
}

This way, your custom element component will have a reference to that child component, and have all of the logic required to bind event listeners, etc.

To provide the ability to pass child components to a custom element wrapper itself, like you're doing in the example provided, would first require the package to support nested custom elements, and provide a way of parsing component references in the HTML string, and marry them up to a respective component instance. This is non-trivial.

I feel like the solution I proposed above handles 99% of use cases, and any others (for the most part), can be mitigated via a re-org of your render tree.

I've had quite a few questions around supporting nested custom elements (the first requirement), so I'm open to providing this functionality. I'm hoping to put together a POC when I get time, but all contributions are more than welcome.

I hope that answers some of your questions / puts you on the right path 👍

@jahilldev jahilldev added enhancement New feature or request preact Relates to preactement package react Relates to reactement package labels Jul 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request preact Relates to preactement package react Relates to reactement package
Projects
None yet
Development

No branches or pull requests

2 participants