-
Notifications
You must be signed in to change notification settings - Fork 25
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
RFC: Dual light/shadow components #77
base: master
Are you sure you want to change the base?
Conversation
<template lwc:render-mode="dual"> | ||
<h1>Hello world</h1> | ||
</template> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if dual is the default render mode for every component?
If all components were always both light or shadow, couldn't the render-mode directive be simplified to a boolean to force the render mode to be light?
if lwc:light-dom directive is present
render mode = light
else
render mode = shadow
In my opinion, using this directive as a boolean looks better because light
mode is an exception to the framework, and there is no 3rd option. I mean, shadow dom must be used unless something tells the contrary.
If this render mode directive could be simplified to a boolean, I would like to suggest these options:
lwc:light-dom
lwc:light-mode
lwc:light
//child
<template>
<h1>
Im dual by default.
If my parent does not say that I must be rendered using light mode,
Im going to default to shadow mode
</h1>
</template>
//parent forces child to use light mode
<template>
<x-child lwc:light>render light</x-child>
</template>
//parent does not say anything, therefore child is rendered using shadow mode
<template>
<x-child>render shadow</x-child>
</template>
For the use case where a component forces the render mode to light
in every context, I believe that using a variable called useLightDom
inside the render()
method would look better. It seems better because this variable, like template
and styleSheet
, also changes the default render behavior of the component.
//child
export default class Child extends LightningElement {
render(){
return {
useLightDom: true,
template: ...,
styleSheet: ...,
... //any other attribute that changes the default render behavior
}
}
}
If the child is configured to use light
mode, the parent would not need to add the lwc:light
directive
//parent
<template>
<x-child>render light</x-child> //light mode is used because the child is forcing it
</template>
3rd party devs would have to document which components render in light mode no matter the context, because developers wouldn't be able to determine this information just reading the parent component's template.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AllanOricil Thanks for the feedback! I appreciate it a lot. 🙂
couldn't the render-mode directive be simplified to a boolean
I believe the reason we went with a string enum originally was because we weren't sure if someday we would want to support more exotic cases, like closed shadow mode or "open-stylable" shadow mode. Although maybe that would argue that we should use a different name for "dual"
– perhaps "any"
or something.
What if dual is the default render mode for every component?
I can see the case for "dual by default," but the main issue I see is that components that were not designed with light DOM in mind (or with shadow DOM in mind, for light DOM components) may be forced into a mode that breaks them. Whereas with my proposal, light/shadow components only render in that one mode unless they explicitly opt-in.
I believe that using a variable called
useLightDom
inside therender()
This would be very convenient from the component authoring perspective, but unfortunately it would greatly complicate how the engine is implemented for the component to switch between light and shadow mode dynamically during its lifetime. For instance, if light-DOM styles were added to the <head>
, now those have to be converted into shadow DOM styles in the shadow root (or vice versa). And while it's possible to attach a new shadow root to an existing light DOM component, it's not possible to remove a shadow root (the browser does not provide a "remove" analog to attachShadow
).
3rd party companies would have document which components render in light mode no matter the context, because developers wouldn't be able to determine this information by just reading the parent component's template.
I don't think they would need to document it either way, because we should be able to expose the static renderMode
in the metadata. So it would be available to e.g. a drag-and-drop builder UI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if dual is the default render mode for every component?
That is non-backward compatible for you. Additionally, we could say that you can, as a consumer of the component, always only select the mode that you know the consumer component works... saying that it is a responsibility of the consumer to always specify has two big drawbacks that I can't really sign off on:
- if the current version of the component does work in both modes (just by pure luck), and the consumer select light, then the owner of the component modifies that, and inadvertently breaks light, he never really sign on for that, now we have breaking changes.
- now the consumer has to learn that there is to mode, and which components are broken and needs to be forced to a particular mode of operation, that seems very wrong to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand it now 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I'm supportive of this proposal.
Update: I added an |
```html | ||
<template lwc:render-mode="light"> | ||
<!-- x-component will render as light --> | ||
<x-component lwc:render-mode="inherit"></x-component> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
two main comments:
<x-component lwc:render-mode="inherit"></x-component>
it could be<x-component lwc:render-mode="dual"></x-component>
and it will work the same, it does the same asinherit
but without introducing yet another token.
it seems to me that this should be an error, if you're choosing the mode in the template, why using inherit down here? you should be specific, because you have chosen the context in which this mode will operate by doing it at the template level. you're not typing less, you're not gaining anything... it is now just more ambiguous for the developer to try to remember what mode the template is running (meaning scroll up).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it could be
<x-component lwc:render-mode="dual"></x-component>
and it will work the same, it does the same as inherit but without introducing yet another token.
We could do that, yeah. I don't have a strong opinion.
you're not typing less, you're not gaining anything
Are you saying that the "inherit" behavior should be implied by omitting the lwc:render-mode
on the <x-component>
altogether? This is fine, except that it introduces ambiguity on how a component behaves depending on the template it is rendered in. In a light/shadow template, it will be whatever default the component author (in this case the author of <x-component>
) chose, whereas in a dual template, it will inherit from that template. To me, this is more confusing than making the omission of lwc:render-mode
always mean "whatever default the component author chose."
<template> | ||
<x-slottable> | ||
<!-- Mode is inherited from this template, not from x-slottable's template --> | ||
<x-component lwc:render-mode="inherit"></x-component> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in this example, who dictates the mode? how can an author look at this code an know? what is the process of knowing what mode this is going to run on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in this example, who dictates the mode?
The top-level <template>
on Line 171 dictates the mode. In this case, it will be shadow mode, because there is no render-mode
on line 171.
how can an author look at this code an know?
They would need to glance up to the top <template>
in the template they are currently authoring. 🙂
what is the process of knowing what mode this is going to run on?
Again, glance up top. 🙂 And if <x-component>
does not support dual mode, you will get a runtime error.
Rendered