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

Export autoload discover function and support shadow roots #1236

Merged

Conversation

jaredcwhite
Copy link
Contributor

@jaredcwhite jaredcwhite commented Mar 11, 2023

This allows for a discover(this.shadowRoot) call within a consumer's web component to opt-into autoloading. Otherwise, if someone has built a component which includes a Shoelace component in the component's shadow DOM, autoloading won't work.

(Note: this doesn't add a mutation observer, so it's a one-time discovery. Most component shadow trees are pretty stable I imagine, so I think that's fine, but if it feels too much like a potential gotcha, we could consider opting-in to a mutation observer for the tree.)

@vercel
Copy link

vercel bot commented Mar 11, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
shoelace ✅ Ready (Inspect) Visit Preview Mar 11, 2023 at 5:39PM (UTC)

@claviska
Copy link
Member

I figured this would come pretty quickly. The reason I held off is because we probably don't want to encourage autoloading in custom elements in the majority of cases. In fact, the only scenario we probably do want to support is when folks build their apps using Lit, FAST, or similar libraries as a framework.

In other words, they use a custom element app container, a custom element router, etc. and because of that, pretty much everything in their app happens to live in a single shadow root. I don't think it makes sense to use an autoloader for independent, third-party components, however.

I'm happy to merge this, but I'm not sure where to document it (or even if we should just yet). Any thoughts on where that should live in the docs?

@claviska claviska merged commit 0e6e2ab into shoelace-style:next Mar 13, 2023
@claviska
Copy link
Member

Regarding the mutation observer — I'm with you. It's only a few lines to setup your own if you really need it, but I'd be OK with exporting a factory function that removes the boilerplate if this becomes a popular request.

@jaredcwhite
Copy link
Contributor Author

I'm not sure where to document it (or even if we should just yet).

Yeah, I'm waffling on that too. I think it's fine as an easter egg of sorts for now, and see if people even end up needing that functionality all that often.

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 this pull request may close these issues.

2 participants