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" not replaced on mount when shadow DOM is disabled #41

Open
mbohgard opened this issue Aug 29, 2020 · 9 comments · May be fixed by #56
Open

"Children" not replaced on mount when shadow DOM is disabled #41

mbohgard opened this issue Aug 29, 2020 · 9 comments · May be fixed by #56

Comments

@mbohgard
Copy link

mbohgard commented Aug 29, 2020

We have a design system with components written in React. I was thinking of rendering those with preact compat and then wrapping those in preact-custom-element to be able to use the components in a Vue/Angular or vanilla js app. I get a few issues tho.

  1. Children of the custom element in html is not replaced when the Preact component mounts. I tried the "solution" to this by @donpark in this issue thread but I can't just switch out {children} for <slot /> if I want to keep compatibility with React and also it didn't work when I tried it.
    Using a shadow DOM here, passing shadow: true to the register function of preact-custom-element works but since I'm using Emotion as a CSS-in-JS I need the global styling support. There are ways of controlling where Emotion inserts styles and use shadow DOM here but that would probably mess with other teams implementation as of now. I would like it to be able to use this lib without enabling shadow DOM.

2. I can't get property bindings to work at all as demonstrated in this PR. Attributes work fine, but properties doesn't end up as props on the Preact component (whether shadow DOM is used or not).

All the issues is demonstrated in this sandbox.

@mbohgard
Copy link
Author

@marvinhagemeister Do you have any ideas perhaps?

@marvinhagemeister
Copy link
Member

marvinhagemeister commented Aug 31, 2020

Woa, it's the prettiest bug repro we've received in any Preact repositories by a long shot! Love the animation effect 👍

Sounds like preact-custom-element needs the mentioned features to satisfy the described use case. Wrapping children automatically with a slot is one of those and the other is getting slot wrapping to work without shadow DOM.

I can't get property bindings to work at all as demonstrated in this PR. Attributes work fine, but properties doesn't end up as props on the Preact component (whether shadow DOM is used or not).

How does it show up in the linked sandbox? Whenever I click any of the buttons the number increases and the animation looks correct to me. - nvm, I see the issue now 👍

@wbern
Copy link

wbern commented Sep 7, 2020

@mbohgard given our current org developments, what do you think of this issue now?

@mbohgard mbohgard changed the title Children not replaced on mount & prop callback not propagated to component instance "Children" not replaced on mount when shadow DOM is disabled Sep 7, 2020
@mbohgard
Copy link
Author

mbohgard commented Sep 7, 2020

@marvinhagemeister Hah, thanks for the design compliments. This was used internally for a demonstration for higher ups. Had to be somewhat pretty to sell the idea ;)
Also thanks for the super fast work of getting property bindings to work! 👏

@wbern Yes the second part of this issue(s) is solved. There is still the issue that remains when having shadow DOM disabled (issue title updated). Personally, we can work around this, but maybe it should be mentioned in the docs what limitations there are while having shadow DOM disabled if there is no solution coming.

@paull39
Copy link

paull39 commented Sep 18, 2020

@mbohgard

Hey!

Do you mind to share a snippet of your workaround?

const foo: FunctionalComponent<any> = (props) => {
    return (
        <h1>My Heading</h1>
        <div class="SomeSpecialContaienrWithAriaStuff">{props.children}</div>
    )
}

// register as CustomElement
register(foo, "my-foo");

Serverside Rendered HTML:

<my-foo>
    <some-special-custom-element>
        Lorem doFoo
    </some-special-custom-element>
</my-foo>

The Result with those snippets is for me:

<my-foo>
        <h1>My Heading</h1>
        <div class="SomeSpecialContaienrWithAriaStuff"></div>
        <some-special-custom-element>
            Lorem doFoo
        </some-special-custom-element>
</my-foo>

Preferred Solution:

<my-foo>
        <h1>My Heading</h1>
        <div class="SomeSpecialContaienrWithAriaStuff">
             <some-special-custom-element>
                Lorem doFoo
             </some-special-custom-element>
         </div>
</my-foo>

If this is the atomic problem, please tell me i really would like to dig into this nice project!

@jvdsande jvdsande linked a pull request Jan 6, 2021 that will close this issue
@Nicolab
Copy link

Nicolab commented Nov 27, 2021

I have the same issue. @paull39 (and other) do you have found a solution?

@Nicolab
Copy link

Nicolab commented Nov 30, 2021

I found a solution, this package works well: https://github.com/jhukdev/preactement

@finalclass
Copy link

For me this still does not work. If you change shadow to false in this example: https://codesandbox.io/s/preact-custom-element-rendering-content-issue-forked-38xpc?file=/src/index.js it stops working as expected.

pwolfert added a commit to CMSgov/design-system that referenced this issue Feb 1, 2023
If I were doing this for real, I'd make a separate web components bundle so there are no side effects in the npm package. Problem right now is being able to pass `children` to web components. Going to try https://github.com/jahilldev/component-elements/tree/main/packages/preactement#readme next based on [this issue thread](preactjs/preact-custom-element#41)
@mufaddalmw
Copy link

I found a solution, this package works well: https://github.com/jhukdev/preactement

there is issue in this library, please check jahilldev/component-elements#26 it doesn't support nested custom-elements on DOM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants