luwes / wesc

We are the Superlative Components!
https://wesc-nextjs.vercel.app
21 stars 0 forks source link

fix: move recursive renderChildren call after appendChild #3

Closed dwirz closed 11 months ago

dwirz commented 11 months ago

Hi there @luwes, first of all very nice project it helped us a lot with SSR our web components within a Next.js app directory project. šŸŽ‰

What I encountered is if we have web components which use the <slot>-element within another web components <slot>-element the react renderChildren function gets somehow stucked in an infinite loop. I was able to fix it by moving the recursive call of renderChildren after the appendChild call.

Since our code is on our customers private repository i can't provide you with some reproduction code.

Dummy codewise it looks something like this:

// React JSX
return (<CustomDropdown text="Dropdown" hint="Hint" label="Label" ref={ref}>
    <CustomFlyout> // slot of custom-dropdown web component
      <CustomFlyoutItem item="item-1" label="Item 1" /> // slot of custom-flyout web component
      <CustomFlyoutItem item="item-2" label="Item 2" /> // slot of custom-flyout web component
      <CustomFlyoutItem item="item-3" label="Item 3" /> // slot of custom-flyout web component
      <CustomFlyoutItem item="item-4" label="Item 4" /> // slot of custom-flyout web component
    </CustomFlyout>
  </CustomDropdown>);
netlify[bot] commented 11 months ago

Deploy Preview for wesc-eleventy canceled.

Name Link
Latest commit 0b754b579df1a8e44a68667e1b3bf64ea0ddeebc
Latest deploy log https://app.netlify.com/sites/wesc-eleventy/deploys/6523c6f39955430008658716
vercel[bot] commented 11 months ago

The latest updates on your projects. Learn more about Vercel for Git ā†—ļøŽ

Name Status Preview Comments Updated (UTC)
wesc-astro āœ… Ready (Inspect) Visit Preview šŸ’¬ Add feedback Oct 9, 2023 9:26am
wesc-nextjs āœ… Ready (Inspect) Visit Preview šŸ’¬ Add feedback Oct 9, 2023 9:26am
wesc-sveltekit āœ… Ready (Inspect) Visit Preview šŸ’¬ Add feedback Oct 9, 2023 9:26am
netlify[bot] commented 11 months ago

Deploy Preview for wesc-node canceled.

Name Link
Latest commit 0b754b579df1a8e44a68667e1b3bf64ea0ddeebc
Latest deploy log https://app.netlify.com/sites/wesc-node/deploys/6523c6f4341e560008ccd5d2
netlify[bot] commented 11 months ago

Deploy Preview for wesc-remixrun canceled.

Name Link
Latest commit 0b754b579df1a8e44a68667e1b3bf64ea0ddeebc
Latest deploy log https://app.netlify.com/sites/wesc-remixrun/deploys/6523c6f45cc1ab00087a66de
luwes commented 11 months ago

thank you! I will try to review this week

dwirz commented 11 months ago

Hello @luwes

it was easier for me to copy your code and adapt it to our needs in our monorepo. Since we are developing our Web Components with Stencil.js and there are two changes (#4914 and #4916) in their code that we need, I created a minimal reproduction repository.

My version of your code is here ./packages/web-components-react-wrapper/lib. The main changes are the ones I described in this PR and PR #4, see .../lib/client/render.ts#L27. Additionally, I added a renderCustomElements that renders all of our custom elements that are inside the React Wrapped Web component.

If you have any questions/ideas/feedback, let me know šŸ™‚ .

IMO you can close this PR and the other one if you don't need my changes.

Thanks a lot for your work

luwes commented 11 months ago

All good! I'm glad the project is useful to you.

This PR LGTM! Will merge it in.