-
Notifications
You must be signed in to change notification settings - Fork 924
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
No side-effects exports #2010
Comments
Thanks for the issue! It definitely looks like The base class itself should not be used when defining a custom element, since they will be missing styles. Additionally, some base classes in the future may be abstract and require final element classes to provide a property or implementation feature (such as a variant type). This is our recommended pattern for extending and customizing elements. import {LinearProgressBase} from '@material/mwc-linear-progress/mwc-linear-progress-base';
import {style} from '@material/mwc-linear-progress/mwc-linear-progress-css';
class LinearProgress extends LinearProgressBase {
// Override styles and functionality
static styles = [style];
}
// Scope or re-define tag name
customElements.define('mwc-linear-progress', LinearProgress); Hope this clarifies things for usage with ScopedElementsRegistry and side-effect-free imports! |
@asyncLiz Mmm I see your point, thanks a lot for the detailed explanation. Although, there are some base classes that are importing directly other non-base classes. For instance, in ButtonBase: import '@material/mwc-icon/mwc-icon';
import '@material/mwc-ripple/mwc-ripple'; This makes it so that even if I import |
That is because those classes directly depend on those elements being defined since they use them in their render templates. If they were not defined, the elements would not work correctly and could throw errors when being used. Unfortunately in those scenarios, I do not see an elegant way around the issue. It also does not make sense to me for those elements to be re-defined in a scoped element registry since the base classes depend on their tag names in the base class templates. I would be curious as to what your use case is for needing to re-define them. |
@asyncLiz sure :), BTW, really enjoying the quick feedback and the thoughtful discussion. My use case is an application with pluggable UIs, in which "blocks" of UI in the form of webcomponents are uploaded to a "repository" and lazily downloaded to the browser on demand. In this scenario, the overarching application is agnostic as to which frameworks/components the blocks contain, but as much as possible they need to be side-effect free to not rely on global definitions that can cause collision. This is the whole reason to use scoped registries, even if they need to be included via a polyfill (right now using this, but the open-wc alternative would work as well). These libraries react to a custom element tag being included in the dom of the elements and change it to a scoped version, that looks like Regarding the use of this materials-components library, the pain point is element versioning: if each of the blocks need to rely on globally defined elements, all of them need to agree on a version of those elements, instead of freely included whatever code and elements and defining them in a scoped manner. Happy to keep the conversation going if there are more doubts around this. |
@asyncLiz for now I'm pointing to this repo: https://github.com/guillemcordoba/scoped-material-components which reexports all the elements with scoped custom element registries in mind. |
Is your feature request related to a problem? Please describe.
I can't import some of the elements without actually defining them with the
customElements
tag. Some elements declare their Base class as abstract and some not, and I can only import the Base classes without side-effects.This makes it cumbersome to use this elements with something like ScopedElementsRegistry.
Describe the solution you'd like
All elements should provide an exportable non-abstract class, without any side-effects.
Describe alternatives you've considered
You could go either way, making all base classes non-abstract and exporting them, or making all of them abstract and providing another exported class with that specific use case in mind.
Additional context
The text was updated successfully, but these errors were encountered: