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

HTMLSlotElement.assignedNodes() #131

Closed
thepassle opened this issue Apr 22, 2022 · 13 comments · Fixed by #209 or loopdive/linkedom#4
Closed

HTMLSlotElement.assignedNodes() #131

thepassle opened this issue Apr 22, 2022 · 13 comments · Fixed by #209 or loopdive/linkedom#4
Labels
enhancement New feature or request

Comments

@thepassle
Copy link

Currently HTMLSlotElement.assignedNodes() doesn't seem to be supported, I have a couple of components that I'd like to SSR that make use of this API. Do you think assignedNodes() could at some point be supported?

@WebReflection
Copy link
Owner

ShadowDOM is still not SSR friendly… once it is, stuff related to SSR will land, in a way or another, but so far I never cared about ShadowDOM even on the client side (it’s not needed) so don’t expect much to work.

PRs welcome though 👍

@thepassle
Copy link
Author

so don’t expect much to work.

Most of the custom elements ive SSRed that use shadow dom seem to work fine 🙂 Im just missing assignedNodes on the slot element

Ill try and see about making a PR, thanks

@WebReflection
Copy link
Owner

DSD is not out though, or is it? That’s what I meant (Declarative ShadowDOM)

@thepassle
Copy link
Author

Hmm I was hoping that this would only be a small change to HTMLSlotElement by adding the assignedNodes() method, but it looks like this.shadowRoot.querySelector('slot') doesnt actually return a HTMLSlotElement, but an Element, and it seems like all elements have the same nodeType. Im not sure if I'll be able to contribute that

@WebReflection WebReflection added the enhancement New feature or request label Jun 7, 2022
@WebReflection
Copy link
Owner

@thepassle sorry for late reply ... you need to register special elements otherwise these are created as Element instances ... see any registered element to understand how that's done, the Options is the latest that got added: 155dfb9

@luwes
Copy link
Contributor

luwes commented Jun 10, 2023

@WebReflection
Copy link
Owner

haven't checked but as I don't care/need Shadow DOM too much and it's not (I hope) on any critical perf path, feel free to open that PR, thanks!

@thepassle
Copy link
Author

That would still be helpful to me @luwes , so id be very appreciative if you could make that PR :)

@trusktr
Copy link

trusktr commented Jun 15, 2023

I would love to have this as well. It is impossible to use custom elements without needing ShadowDOM. If you do composition in any other framework (props.children in React/Preact/Solid, slots in Vue/Svelte, etc), then you need ShadowDOM for the same (highly useful!) patterns with custom elements.

@WebReflection
Copy link
Owner

Folks I’m afraid life happens and extra time for stuff I don’t need/use daily, even for reviews I’d love to merge, is super hard to find. I’ll do my best to follow up on this whenever I can put my brain and eyes on it, and as ETA that’s not likely going to happen before the end of the month, but I’ll try any weekend I can.

@mash-graz
Copy link
Contributor

mash-graz commented Oct 18, 2023

@WebReflection

Could please take a look at the already provided PR (#209) for this issue.

IMHO it's a very important improvement, because many of us want to utilize linkedom mainly for SSR rendering of Custom Elements and all its horrible obstacles in a sufficient standard compliant manner.

Sure, there are other workarounds possible and few forks including this patch are also available.
Nevertheless. I would highly appreciate a more proper solution, which keeps bug reports, continuous improvements and reliable maintenance at solely one well known place together.

@WebReflection
Copy link
Owner

I'm still very busy ... I'll try to merge after review some PR over the weekend

@mash-graz
Copy link
Contributor

... I'll try to merge after review some PR over the weekend

Thank you for your pleasant reply!

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

Successfully merging a pull request may close this issue.

5 participants