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

RFC: Light DOM Support #44

Merged
merged 30 commits into from Sep 10, 2021
Merged

RFC: Light DOM Support #44

merged 30 commits into from Sep 10, 2021

Conversation

abdulsattar
Copy link
Contributor

Rendered

Based on the initial RFC #40

Co-Authored-By: Ted Conn <tconn@salesforce.com>
text/0000-light-dom.md Outdated Show resolved Hide resolved
text/0000-light-dom.md Outdated Show resolved Hide resolved
text/0000-light-dom.md Outdated Show resolved Hide resolved
text/0000-light-dom.md Outdated Show resolved Hide resolved
text/0000-light-dom.md Outdated Show resolved Hide resolved

- **`this.template`**

In `LightningElement`, `this.template` returns the shadow-root. It will return `null` in `MacroElement`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will return null in MacroElement.

How do I reference the root element in a light dom component?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With LightningElement, all the properties and methods on the this value interacts with the host element (this.getAttribute, this.querySelector, this.classList). And this.template allows to interact with the shadow root. However, LWC never exposes the host element itself to the component instance. You can get access to it, via this.template.host but it is more a hack than anything else.

The same model is preserved with MacroElement. It is possible to interact with the host element using methods and properties on the this value. But there is currently no workaround (like this.template.host) to access the host element from a MacroElement component instance.

If accessing the host element is necessary, I would vote for making it available to both LitghtningElement and MacroElement in a similar fashion. For example via a host getter.

- In some applications, light-dom components may not be allowed... it’s up to the app context
- **what is the behavior when it’s not allowed?**
- Some applications might disable light-dom as a whole
- Some applications might disable light-dom selectively using a “privileged code” model
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

way more detail necessary here. Does this mean that a component was authored in the LightDom but app requested it in the shadow dom for ex?

text/0000-light-dom.md Outdated Show resolved Hide resolved

There is no migration of the existing components needed. The behaviors of existing components using Shadow DOM remain the same.

### Synthetic Shadow DOM
Copy link
Collaborator

@tedconn tedconn Mar 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think many people are asking "if everything is light DOM then what purpose does Synthetic Shadow DOM serve? The way I tried to answer that question today is "there will always be components somehow in the shadow DOM so we will always need an emulation." but maybe I am wrong?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are trying to get out of this model, where some components will always need emulation.

Synthetic shadow purpose was created to emulate shadow DOM on browsers that don't support shadow DOM natively. As we are sunsetting IE11 support, we shouldn't need synthetic shadow anymore.

@tedconn tedconn changed the title RFC: Add Light DOM RFC RFC: Light DOM Support Mar 18, 2021

## Summary

LWC currently enforces the use of Shadow DOM for every component. This proposal aims to provide a new option, a toggle, which instead lets a component attach its content as children in the Light DOM.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rephrase this sentence. Shadow DOM is not an enforcement, it is just the default.

This proposal is not a toggle, because it is not a 1-1 mapping between shadow DOM and light DOM. It will certainly require developers to rewrite partially their component. I see this proposal as a brand new type of components (with new requirements and restrictions).

Suggested change
LWC currently enforces the use of Shadow DOM for every component. This proposal aims to provide a new option, a toggle, which instead lets a component attach its content as children in the Light DOM.
As of today, all the LWC components inheriting from `LightningElement` render their content to the shadow DOM. This proposal introduces a brand new kind of component, which renders its content as children in the Light DOM.

text/0000-light-dom.md Outdated Show resolved Hide resolved

```html
<app-container-blue-light>
▼ <div>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<div>
<div>

text/0000-light-dom.md Outdated Show resolved Hide resolved
</app-counter-blue-light>
```

As a result, when the content of a component lives in its children, it can be accessed like any other content in the Document host, and thus behave like any other content (styling, APIs, accessibility, third party tooling...).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
As a result, when the content of a component lives in its children, it can be accessed like any other content in the Document host, and thus behave like any other content (styling, APIs, accessibility, third party tooling...).
As a result, when the content of a component resides in the Light DOM, it can be accessed like any other content in the Document host, and thus behave like any other content (styling, APIs, accessibility, third party tooling...).


- **`this.template`**

In `LightningElement`, `this.template` returns the shadow-root. It will return `null` in `MacroElement`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With LightningElement, all the properties and methods on the this value interacts with the host element (this.getAttribute, this.querySelector, this.classList). And this.template allows to interact with the shadow root. However, LWC never exposes the host element itself to the component instance. You can get access to it, via this.template.host but it is more a hack than anything else.

The same model is preserved with MacroElement. It is possible to interact with the host element using methods and properties on the this value. But there is currently no workaround (like this.template.host) to access the host element from a MacroElement component instance.

If accessing the host element is necessary, I would vote for making it available to both LitghtningElement and MacroElement in a similar fashion. For example via a host getter.


There is no migration of the existing components needed. The behaviors of existing components using Shadow DOM remain the same.

### Synthetic Shadow DOM
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are trying to get out of this model, where some components will always need emulation.

Synthetic shadow purpose was created to emulate shadow DOM on browsers that don't support shadow DOM natively. As we are sunsetting IE11 support, we shouldn't need synthetic shadow anymore.

Comment on lines 195 to 200
The selection of Light DOM should not be impacted by the use of synthetic shadow instead of native shadow. Now, the goal is to get rid of synthetic shadow, but this is hardly possible today because of:

- Salesforce Lightning Components do not work with native shadow today
- Shadow DOM has accessibility issues

Could we think of a lightweight synthetic shadow that do not override the global methods but provides enough functions to the base components to work while letting third party integration tools work? This can be a different DOM option, which is an hybrid between Shadow DOM and Light DOM.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this section belongs to the proposal.
It overlaps more with @ravijayaramappa's work on Shadow DOM mixed mode.

Comment on lines 274 to 279
content = evaluateStylesheetsContent(
stylesheets,
hostSelector,
shadowSelector,
!syntheticShadow,
macroSelector
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
content = evaluateStylesheetsContent(
stylesheets,
hostSelector,
shadowSelector,
!syntheticShadow,
macroSelector
content = evaluateStylesheetsContent(
stylesheets,
hostSelector,
shadowSelector,
!syntheticShadow,
macroSelector
)


Could we think of a lightweight synthetic shadow that do not override the global methods but provides enough functions to the base components to work while letting third party integration tools work? This can be a different DOM option, which is an hybrid between Shadow DOM and Light DOM.

## Internal Implementation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would extract this section out of the RFC and move this content to a spike document.
The internal implementation might change over time, while the RFC should remain accurate on the long run.

}
```

`MacroElement` is a new class that will be exported from the `lwc` module. Its API and functionality remain identical to `LightningElement` with the primary difference of not using Shadow DOM.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit concerned about exposing light DOM via MacroElement. I can break my concerns into two groups: 1) deviation from other WC frameworks, and 2) potential class proliferation.

Deviation from other WC frameworks

Here is how other frameworks handle light DOM:

Stencil

@Component({
  tag: 'my-component',
  shadow: false // or omitted
})
class MyComponent {}

Fast

@customElement({
  name: 'my-component',
  template,
  shadowOptions: null // or `{ mode: 'closed' }` for closed shadow
})
class MyComponent extends FASTElement {}

Lit

class MyComponent extends LitElement {
  createRenderRoot() {
    return this // or return `this.attachShadow(...)` here
  }
}

So we have 2 frameworks opting for decorators, and another opting for an overridable method. LWC would be the only WC library using separate classes (unless I missed one).

Potential class proliferation

In the future, we may want to support closed shadow roots (edit: my mistake, didn't realize we already did 🤦 ). Would that be another class?

import { ClosedShadowElement } from 'lwc'

There is also a proposal for "open stylable roots" (WICG/webcomponents#909). Would this be another one?

import { OpenStylableElement } from 'lwc'

In that proposal, there is even some talk of attachShadow() eventually looking like this:

this.attachShadow({
  mode: 'open',
  styles: 'closed'
})

If the options bag for attachShadow continues to grow like this, then we could end up with a combinatorial explosion of classes to handle every possible case.

My personal preference would be for decorators or mixins, but mostly I'm just concerned about choosing classes here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the original reason we chose a different class was to emphasize that these are different kinds of components because the programming model is different. I'm not sure if we still feel that is the case.

I'm curious how the createElement API will change which currently takes the mode.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tedconn Ah my mistake, I didn't even know we supported closed shadow roots. 🙂 It seems that createElement would be the right place to put the options bag (if it becomes a big bag!).

So it looks like my concerns about class proliferation might not apply, since we only plan on having MacroElement for this one case (light DOM). But then why not have light DOM as an option in createElement?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But then why not have light DOM as an option in createElement

Because we need an API on the component, so component authors can toggle, not on the engine. Something like this would be nicer though, right?

import { LightningElement } from 'lwc';

export default class LightDomElement extends LightningElement {

  static shadowOptions = {
    mode: null
  }

}

Copy link
Member

@pmdartus pmdartus Mar 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed in the past 2 options, using a static field or using a brand new base class. Until the decorator proposal goes back to stage 3, I don't think it is safe to introduce a brand new decorator.

The main advantage of introducing a brand new class over static fields is around component inheritance. You can shadow (by inadvertence or intentionally) a base class static field. It means that you can turn a Shadow DOM component into a Light DOM one (and vice-versa) by extending from it. Such override can't be done when inheriting from a different base class.

In the following example, let say that we introduce a new shadowDOM static boolean to indicate whether the component rendered in the Light DOM or in the Shadow DOM. It is something we should discourage.

import { LigthningElement } from 'lwc';

class ShadowDOMComponent extends LightningElement {
  static shadowDOM = true;
}

class LigthDOMComponent extends ShadowDOMComponent {
  static shadowDOM = false;
}

Now that you speak about class proliferation, I am more favorable to adding a signal via a static field.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After discussion with the team, we landed on the static field rather than MacroElement, correct? AFAICR, we concluded:

  • Any solution we land on (classes, field, method, decorators, etc.) needs to be resolved at runtime. In other words, developers can always do shenanigans such that you need to do a runtime evaluation to figure out whether the element is shadow or light. Since the LWC compiler to know at compile-time, we would just have to teach developers to use patterns that are friendly for compile-time evaluation.
  • Decorators are not standard yet
  • Classes may result in class proliferation
  • Static fields seem to have nice ergonomics
  • So we should go with static fields

Is this right? @pmdartus @abdulsattar

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right.


In Light DOM, `<slot>` will denote the place where the slotted component will be attached. The `<slot>` element itself won't be rendered. The slotted content (or the fallback content) will be flattened to the parent element at runtime.

Since the `<slot>` element itself isn't rendered, adding attributes or event listeners to the `<slot>` element in the template will throw a compiler error.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of Stencil, Fast, and Lit, only Stencil opted to make slots "work" with light DOM.

Fast says:

If you choose to render to the Light DOM, you will not be able to compose the content, use slots, or leverage encapsulated styles.

Lit is also against slots in light DOM:

I don't think <slot> makes much sense outside of shadow DOM [...]

Based on discussion in this thread, it seems like Stencil's choice may have resulted in significant performance overhead, which is understandable given that a lot of behavior you get "for free" with slots in shadow DOM now has to be reimplemented for light DOM.

Before committing to this, I'd be interested to see some more details about how we plan to handle slots in light DOM:

  • How does MutationObserver work?
  • How does the timing between slotted versus non-slotted children work?
  • Would CSS selectors be able to traverse these boundaries?

I'd also be interested to see some benchmarks, to understand if the issues Stencil ran into were just problems with Stencil's implementation, or if they're inherent to trying to make slots "work" in light DOM.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another interesting point of comparison: Aurelia has slots, but they explicitly mark it as <au-slot>, not <slot>:

An obvious question might be "Why not simply 'turn off' shadow DOM, and use the slot itself"? We feel that goes to the opposite direction of Aurelia's promise of keeping things as close to native behavior as possible. Moreover, using a different name like au-slot makes it clear that the native slot is not used in this case, however still bringing slotting behavior to use.

</x-a>
```

Opting out of this scoping is not supported. There's no way for a component author to say a CSS rule shouldn't be scoped to that specific component (and its children). If global scoping is desired, a global stylesheet can be injected manually.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With synthetic shadow DOM, we implemented a system for style scoping similar to this (using attribute selectors). However, with shadow DOM, it was kind of implied that eventually we would migrate to native shadow, so the synthetic system wasn't designed to last forever. With this system, though, we are committing to support a new styling system indefinitely, including whatever implementation details we land on, which will impact compatibility moving forward. For instance:

  1. Whether to use attribute selectors / classes / etc. for scoping? This has performance implications (browsers tend to have more optimizations for class than for attribute selectors) as well as specificity implications (e.g. specificity conflicts with global styles). FWIW, Vue uses attribute selectors whereas Svelte uses class selectors.
  2. Whether styles are inherited to the children or not. This also seems to me to have performance implications, since I'm not sure how we could implement this, except by concatenating attributes down the descendant tree. (So for instance, would the 5th light DOM grandchild have 5 separate attributes on every DOM node?)
  3. Whether styles are inherited to light DOM "slots" or not, and whether something like (for instance) sibling selectors would work across both "slot" and non-"slot" light DOM children

It seems to me that the safest option is to not provide any kind of style scoping for light DOM elements. I.e. "if you want style scoping, use shadow DOM." This avoids making any commitments toward one particular styling system in light DOM.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the goal is to land on the most natural ergonomics for the developer, including developers already versed in LWC.

Styles in LWC are scoped today, but for end users this is not a natural consequence of the shadow DOM encapsulation, rather it just makes sense since the authoring model is per-component. This is why Vue scopes styles, not because it aligns with the Shadow DOM model. For us that is just a bonus.

Not scoping styles would mean that without any extra code on our side, styles would leak into shadow DOM children, which would not match the native shadow DOM behavior when we turn off synthetic.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tedconn

Styles in LWC are scoped today, but for end users this is not a natural consequence of the shadow DOM encapsulation

It is due to synthetic-shadow, sure, but the long-term goal is to migrate to native shadow DOM. So then it would be a natural consequence. 🙂

This is why Vue scopes styles, not because it aligns with the Shadow DOM model.

I'm not saying scoped styles aren't good for developer ergonomics; I cede that point. 🙂 My point is just that "which scoped styles?" is an important question, especially since we have to indefinitely maintain whatever we choose today.

Not scoping styles would mean that without any extra code on our side, styles would leak into shadow DOM children

I'm not sure I follow this. Today, in a purely non-LWC, non-synthetic world, I can have a light DOM-using component with a shadow DOM-using component as its child, and the only styles that leak in are inheritable styles like font-family and color, which is per the spec. It should work this way both in native shadow DOM and in our synthetic shadow DOM.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should work this way both in native shadow DOM and in our synthetic shadow DOM.

I may be wrong but my thinking was that If styles aren't scoped, in synthetic shadow DOM, those styles will leak in, because there's no other mechanism to prevent that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure, so I just ran a quick check. It looks like if you add the style body { font-family: monospace; } to a Salesforce page, it will indeed leak in to synthetic shadow DOM components. (This is the same as what would happen with a light DOM-using component wrapping a shadow DOM-using component.) So it appears we are following the spec correctly on that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

those styles will leak in, because there's no other mechanism to prevent that.

@tedconn You can prevent inherited styles to be applied by resetting all the properties on the host element. It works in both native shadow and synthetic shadow.

:host {
  all: initial;
}

We discussed CSS scoping at length and we reached the conclusion that it something we would like to preserve with Light DOM for developer ergonomic reasons, as @tedconn mentioned.

We considered the following approach to style scoping:

  1. component content scoping: Styles are applied to the content the component renders and aren't applied to children components. This approach is similar to the way styles are scoped today in synthetic shadow. It also behaves the same as Vue, Svelte, and Angular. An HTML attribute is added to all the CSS selectors, the same attribute is also added to all the elements rendered by the component.

  2. component subtree scoping: Styles are applied to the content the component renders and also to the content rendered in the children's components. This is a similar approach to Aura styling. All the CSS selectors are prefixed with a CSS attribute selector and the same attribute is applied to the root element to scope the subtree.

/* Input */
.foo .bar {}

/* Ouput: component content scoping */
.foo[data-scope] .bar[data-scope] {}

/* Output: component subtree scoping */
[data-scope] .foo .bar {}

We decided to go with the component subtree scoping approach for the following reasons:

  • This approach map with the way querySelector works in the light DOM. It applies to the entire subtree and not only the current element.
  • One of the main reason for using light DOM is to let a component author override it children components style. This is possible with the subtree scoping approach but not with the content scoping one. Overriding children component styles with the subtree scoping approach would require the introduction of a non-standard CSS selector to escape the scoping (>>> in Vue and Angular or :global in Svelte).

The main downside for me of the component subtree scoping approach is that you can run into a style clashing issue where the parent component can unintentionally override children's component styles because the parent selector has a higher specificity.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that makes sense. Two comments:

First, I'm not sure that preferring Aura's styling model to Vue/Svelte's styling model is the best choice from the perspective of developer ergonomics. Aura is well-known inside Salesforce, but not so much outside. (Also if I'm not mistaken, most React CSS-in-JS libraries follow something more like the Vue/Svelte model.) Personally I would find it a bit surprising that parent light components can affect child light components' styles.

Second, for this CSS:

[data-scope] .foo .bar {}

I find this to be a bit scary from a performance perspective. Imagine something like:

/* Input */
div {}

/* Output */
[data-scope] div {}

Now you're potentially the telling the browser to check the ancestor chain for every div to see if its attributes match. For class selectors, there is already a Bloom filter to handle this, but I'm not sure if browsers have gotten around to optimizing attribute selectors in the same way. I may have to whip up a benchmark to test this. 🙂

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I would find it a bit surprising that parent light components can affect child light components' styles.

This is good to know. We can survey developers internally to see which approach they find the most intuitive.

Now you're potentially the telling the browser to check the ancestor chain for every div to see if its attributes match.

I don't think we can use classes because component developers can set classes via the template (from outside) or in the component via this.classList (from within). We would need to special-case this scoping class and ensure that it is always present even when the author manipulates the component classes. I am not sure if we can do this in an efficient way.

text/0000-light-dom.md Outdated Show resolved Hide resolved
Comment on lines 193 to 197
this.querySelector('button').addEventListener('click', this.handleClick);
}

handleClick() {
this.dispatchEvent(new CustomEvent('custom', { detail: { macro: true } }));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given how the event listener is added, what's the expected this within the event handler?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this example, the this value is the button element. Event listeners added via the template are the only ones binding the this value to the class.

@pmdartus
Copy link
Member

pmdartus commented May 3, 2021

This is an interesting proposal @caridy. Correct me if I am wrong but as far as I understand the counter-proposal, the main difference current proposal is around style injection. More specifically the capability to style the host element. The rest of the proposal remains unchanged: DOM access, slotting, and security. Right?

@nolanlawson
Copy link
Contributor

slotAssignment: 'manual' is the most important bit, which is part of the Imperative Shadow DOM Distribution API

This seems to be only shipped in Chrome, although the two other vendors are noted as "positive." Is this proposal viable if we have to polyfill for non-Chrome browsers?

@caridy
Copy link
Contributor

caridy commented May 3, 2021

slotAssignment: 'manual' is the most important bit, which is part of the Imperative Shadow DOM Distribution API

This seems to be only shipped in Chrome, although the two other vendors are noted as "positive." Is this proposal viable if we have to polyfill for non-Chrome browsers?

Right, it is not everywhere, but works just fine with what we have today in LWC with the manual allocation, and polyfill it if needed.

@nolanlawson
Copy link
Contributor

@caridy I don't quite understand – if the feature is polyfillable, then what benefit do we get from using it? Performance?

Also I'm not sure how we would polyfill it. Did you have something in mind?

@nolanlawson
Copy link
Contributor

More specifically the capability to style the host element.

Actually Caridy's proposal would change the meaning of :host. In the current light DOM design, a light child component can style its shadow parent using :host. With Caridy's proposal, :host would style the light child.

That assumes, though, that we inject the <style> into the light DOM element's shadow root. Which would also mean that a local style like div would not work to style a local div – you would need ::slotted(div). Also light DOM styles would not leak out globally.

OTOH we can also keep the existing mechanism and just have a light DOM element inject its style into its nearest shadow root. This would align Caridy's proposal with the current design w.r.t. styles.

text/0000-light-dom.md Outdated Show resolved Hide resolved
- [LitElement](https://lit-element.polymer-project.org/api/classes/_lit_element_.litelement.html#createrenderroot)
- [MS Fast Element](https://fast.design/docs/fast-element/working-with-shadow-dom#shadow-dom-configuration)

## Detailed design

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The list of invariants and constraints is missing. From the live discussion on this RFC today it became obvious that there are many you're working with.

  • Support all standard Light DOM APIs at runtime
  • What the developer codes in the template is what they see in the rendered DOM
  • Align with standards where possible; when no standard exist align with pre-existing LWC concepts, and finally other frameworks
  • A constraint is that the LWC compiler operates on 1 file at a time, not one component (eg HTML + Javascript)

There are many others that need to get captured here. This'll bring visibility to the rationale driving this design.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still fuzzy about:

What the developer codes in the template is what they see in the rendered DOM

what I see in the PR is that the content of the slot is flatten, so the slot element itself never gets rendered.

console.log(LightningElement.shadow); // true
```

The `shadow` property is looked up by the LWC engine when the component is instantiated to determine how it should render. Changing the value of the `shadow` static property after the instantiation doesn't influence whether components are rendered in the light DOM or in the shadow DOM.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it influence components created after the shadow property's value is changed?

Should this value be made non-writable at runtime?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of how we're handling Babel loose mode, I'm not sure it's possible to enforce this. I could be wrong though.

As-is though, if shadow is redefined after the constructor() is called, then the change would have no effect.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, so it looks like it's possible, but we have two options:

  1. Disallow setting shadow a second time.
  2. If any instance of FooElement runs its constructor(), disallow setting shadow again.

The downside of the first is that users may legitimately want to set shadow more than once (complex app logic) before calling the constructor. The downside of the second is that it's a bit odd than a single instance of a class would have an impact on the entire class.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the current implementation updating the shadow attribute after the component is created has no effect. The attribute is read during the construction phase and then cached in the VM for the lifetime of the component.

Making the static field non-writable doesn't provide much guaranty if the field is defined as a getter:

import { LightningElement } from 'lwc';

export default class FooElement extends LightingElement {
  static get shadow() {
    return Math.random() > 0.5;
  }
}

We can for example cache the value shadow value after the FooElement instantiation and reuse it for all the future FooElement instantiations. I am not personally in favor of making the first component instantiation special.


Shadow DOM (in combination with Lightning Locker) encapsulates components and prevents unauthorized access into the shadow tree. With Light DOM though, the DOM is open for traversal by other components. Since Light DOM is not the default, the component author has to opt-in to it, the burden of security falls on them. Developers need to understand that they are "opening" their component for access from outside when they opt in to Light DOM.

Light DOM will be behind a feature flag that can be set for the runtime. It can be turned on/off on a per container basis. The LWC engine will throw at runtime is a component attempts to render in the light DOM when the feature flag is disabled.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be turned on/off on a per container basis.

Please define container.

text/0000-light-dom.md Outdated Show resolved Hide resolved
text/0000-light-dom.md Outdated Show resolved Hide resolved
text/0000-light-dom.md Outdated Show resolved Hide resolved

Consumer applications require DOM traversal and observability of an application’s anatomy from the document root. Without this, theming becomes hard and 3rd party applications do not run properly:

- **Theming and branding**

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As Dean said in DRB today, UIP has the same (near-term future) requirements around theming and branding for LEX and LWR apps as Communities.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe the LEX requirements are the same as what Communities is requesting because the number of actors in the system is very different. Let's discuss offline.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hsterlingsfdc Could you make sure to bring the LWC team into the loop when this will be discussed/designed? I would like to better understand the LEX and LWR requirements in this regard.


In Light DOM, `<slot>` will denote the place where the slotted component will be attached. The `<slot>` element itself won't be rendered. The slotted content (or the fallback content) will be flattened to the parent element at runtime.

Since the `<slot>` element itself isn't rendered, adding attributes or event listeners to the `<slot>` element in the template will throw a compiler error.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This affects builders if it is not a real slot. This may mean every single builder (LAB is only one of the many builders Salesforce has) has to add special code to process components dragged into another component's light dom slot.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately it would be impossible to use real slots for light DOM, because slots as defined by the web platform are a part of shadow DOM.

Whatever we decide on for "light DOM slots," it will not be the same as shadow DOM slots.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Follow-up question: Today, we have internal LWC container components like tabset and accordion that are exposed in LAB (via an aura wrapper). ISV LWC container components (which are just components that contain slots that the builder understands) are on the roadmap. It sounds like the recommendation is for ISVs to build these with Light DOM, since they are general purpose and just meant to house other content.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's really up to each use case. I think we would need to draft a longer document about the strengths and weaknesses of light DOM versus shadow DOM.

For instance, even a tabset component may want to protect its shadow content from being unintentionally styled by the consumer, so shadow DOM may be an appropriate choice there. Also a tabset will inevitably use slots, meaning that there could be performance or timing differences between shadow and light DOM. (E.g. eager versus lazy – a tabset containing 10 slots that only renders 1 might still pay the cost of rendering the other 9, if slots are eager, as they are in native shadow DOM.)

But yes, in general, I think your intuition is correct. Tabsets and accordions don't themselves contain sensitive content, so they could be good candidates for light DOM.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the issue is broader than builders: any system that generates LWC components needs to be updated if the LWC syntax differs in how slotted content is provided to a Light DOM-enabled component.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other thoughts:

  1. Given the styling/perf differences, we should consider whether some components are configurable by the admin
  2. We could use guardrails in the builder to guide admins towards choosing the correct configuration by a) visually showing the difference in preview and b) predicting perf characteristics

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any system that generates LWC components needs to be updated

Side note: any system that generates LWC components will need to be updated to respect the no-shadow directive anyway.

nolanlawson and others added 2 commits May 5, 2021 13:17
Co-authored-by: Kevin Venkiteswaran <kvenkiteswaran@salesforce.com>
Co-authored-by: Kevin Venkiteswaran <kvenkiteswaran@salesforce.com>
- What are the differences between Shadow DOM and Light DOM
- We need a guide on when to use one or the other

## Open questions

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to think more about the low-code use cases and the admin persona.

  1. How would we protect an admin from using light DOM components that may unknowingly compromise the experience they are building?
  2. Does the component schema itself need to indicate whether it is using light DOM? We have talked about using JSON schema to describe slots, maybe this is part of it
  3. How we teach this needs to also address the admin who is building experiences with a mix of light and shadow DOM components.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Do we require top-level components exposed in LAB to use shadow DOM? I don't think we can, supporting LWC container components like tabset seems like a dealbreaker. Need to think about this more

@nolanlawson
Copy link
Contributor

@caridy I've had a chance to think about your counter-proposal. It has some virtues compared to the current proposal, but I think it also has flaws, which tip the balance toward the current proposal. Here are the main flaws I see:

  1. The use of slot.assign() means we would need to polyfill this for non-Chrome browsers. And this polyfill does not seem trivial or simple to me. Given the headaches we've had with synthetic shadow DOM, it seems unwise to go down this path again.
  2. Lots of other frameworks have a concept of "light DOM components" (Stencil, Lit), or it's just the default model (React, Vue, etc.), and in no case do they use a shadow DOM as a proxy for the light DOM – they just use light DOM. So we would be taking a risky, novel approach rather than using a tried-and-true approach more similar to other frameworks.
  3. The counter-proposal unlocks features that developers aren't really asking for. For instance, targeting :host in CSS is unnecessary, as it can be done with x-mycomponent (in both scoped and unscoped light DOM). Other examples include: targeting :slotted in CSS, being able to querySelector a <slot>, and dynamically slotting content with .appendChild/.innerHTML/etc. I would consider these all to be edge cases, not must-have features. The only feature I can imagine developers might really want is the slotchange event. But even this is somewhat niche, and we can always add some LWC-ism to support it in the future. (For the record, in React/Vue/Svelte, slot changes can be detected essentially as re-renders, but in LWC renderedCallback is not called in this case.)
  4. SSR would become much more complicated, as we cannot just render light DOM as-is; we have to handle the shadow as well.
  5. Less important, but this proposal would lock us into eager (native) slots rather than lazy slots. This has the upside of using the native browser API, but it has the downside of losing the performance benefit we get from lazy slots, so I consider it a less important issue.

The main issue for me personally is # 1 – I can imagine the polyfill to be a bug factory and maintenance burden in the future. There are also the unknown performance implications of this model, with or without the polyfill. (This is part of # 2 – the risk that comes with choosing a novel approach rather than a tried-and-true one.)

Overall, I would prefer to stick with the existing RFC rather than the counter-proposal, but I am open to argument and persuasion from others on the team. 🙂

@caridy
Copy link
Contributor

caridy commented May 20, 2021

inline

  1. The use of slot.assign() means we would need to polyfill this for non-Chrome browsers. And this polyfill does not seem trivial or simple to me. Given the headaches we've had with synthetic shadow DOM, it seems unwise to go down this path again.

We can chat more about this... the RFC is about the direction, and maybe with some flavors about how to get there, but mostly about the end goal for the design. With that in mind, there are things that you can do to accommodate the missing .assign(), e.g.: restrict slot allocation to non-root elements in templates, restriction conditions to non-root elements in templates, that really means you don't need to worry about assign when running native in the first iteration, and no polyfill is needed.

  1. Lots of other frameworks have a concept of "light DOM components" (Stencil, Lit), or it's just the default model (React, Vue, etc.), and in no case do they use a shadow DOM as a proxy for the light DOM – they just use light DOM. So we would be taking a risky, novel approach rather than using a tried-and-true approach more similar to other frameworks.

I'm not concern about this, we have two camps, teachability of my proposal is very high, with prior art, and easy to explain.

  1. The counter-proposal unlocks features that developers aren't really asking for. For instance, targeting :host in CSS is unnecessary, as it can be done with x-mycomponent (in both scoped and unscoped light DOM). Other examples include: targeting :slotted in CSS, being able to querySelector a <slot>, and dynamically slotting content with .appendChild/.innerHTML/etc. I would consider these all to be edge cases, not must-have features. The only feature I can imagine developers might really want is the slotchange event. But even this is somewhat niche, and we can always add some LWC-ism to support it in the future. (For the record, in React/Vue/Svelte, slot changes can be detected essentially as re-renders, but in LWC renderedCallback is not called in this case.)

This is the point that I will push back hard. One of the characteristics of LWC is that you never know what name will be used to instantiate the element you're declaring. Forcing developers to use x-mycomponent automatically make the element non-extensible, if you extend that class, it will not get the proper style anymore, similarly, if you define a custom resolution, you are not going to see it anymore. Forcing developers to have to use a unique classname or an attribute to be able to style themself. This for me represents a loss of generality in the authoring format. We will not know for sure until people start styling their components using light-dom, that's why I prefer to keep that door open.

  1. SSR would become much more complicated, as we cannot just render light DOM as-is; we have to handle the shadow as well.

Can you elaborate more on the SSR issues? I haven't foresee any, so I imagine that I'm missing something important.

  1. Less important, but this proposal would lock us into eager (native) slots rather than lazy slots. This has the upside of using the native browser API, but it has the downside of losing the performance benefit we get from lazy slots, so I consider it a less important issue.

I'm not so sure what you mean but this, I suspect it is about the current allocation of vnodes when using synthetic shadow, something that is not happening with native. Basically, with native enabled, there are no lazy slots anymore, is that it?

text/0000-light-dom.md Outdated Show resolved Hide resolved
@nolanlawson
Copy link
Contributor

@caridy

With that in mind, there are things that you can do to accommodate the missing .assign()

In that case, I would be more comfortable with the counter-proposal if it assumed .assign() doesn't exist, at least for the moment. We could always unblock the restrictions later when more browsers have shipped it.

Forcing developers to use x-mycomponent automatically make the element non-extensible

This is a fair point. Since we own the compiler though, we can always make :host point to the "host" if needed. In #45 we make it throw an error for scoped light DOM styles, but presumably we could just make it point to the root node.

Can you elaborate more on the SSR issues?

In this proposal, we don't rely on declarative shadow DOM, whereas in the counter-proposal, we would. I admit the hydration work is probably going to be substantial either way, though, especially for slots.

Basically, with native enabled, there are no lazy slots anymore, is that it?

Correct, the question is whether, in a pure-native-shadow-DOM world, we still want to have eager (native-like) slots for light DOM components. There are more details about lazy vs eager here, and you can see an argument for lazy slots in this post.

That said, we could still decide to have eager slots with the current proposal; the option is open to us. With the counter-proposal, we don't have the choice.

@pmdartus
Copy link
Member

Can you elaborate more on the SSR issues?

In this proposal, we don't rely on declarative shadow DOM, whereas in the counter-proposal, we would. I admit the hydration work is probably going to be substantial either way, though, especially for slots.

I don't see yet how the counter proposal would easily integrate with server-side rendering. As far as I understand the counter-proposal, relying on manual slot allocation would force server-rendered pages to wait for the framework and the component code to be loaded to slot the appropriate content. Even from the specification perspective, interactions between declarative shadow DOM the manual slot assignment are still unclear (whatwg/dom#831 (comment)).

Since we own the compiler though, we can always make :host point to the "host" if needed.

As this proposal is currently borrowing the <slot> tag to indicate a light DOM slot, I feel that reusing :host would make sense.

@nolanlawson
Copy link
Contributor

During the sync today, we discussed the issues @caridy raised with :host and targeting root nodes. We concluded that we should make :host target the root node, in both scoped and unscoped light DOM styles.

Reasoning:

  • Developers should not have to target x-foo, since as Caridy pointed out, they don't own the tag name.
  • The workaround of manually doing this.classList.add('my-class') and targeting that is ugly.
  • Without a proper solution, developers will be tempted to "guess" the tag name (x-foo), which won't work consistently, e.g. for subclasses.
  • Styling the :host of a shadow parent from within a light child is probably an edge case we don't need to support. Developers can always import CSS within a shadow element if that's what they want.
  • We're already supporting <slot> in light DOM, so supporting :host makes sense from that perspective.
  • We don't need to support :host-context (Safari and Firefox don't seem keen to implement) or ::slotted (hard to implement, too edge case-y).
  • The downside is that we're no longer just including the CSS as-is, but this seems worth it given the upsides.

@nolanlawson
Copy link
Contributor

After thinking about it some more, I think :host should only refer to the light DOM root node in scoped CSS. In unscoped CSS, it's a much simpler mental model for the developer if we just insert the styles as-is, with no transformation. Plus having :host "work" in that case makes some sense, since it's a shadow-DOM-ism and we're already doing shadow-DOM-like style scoping. Also, it gives developers maximum flexibility in making :host do what they want (style shadow parent hosts in one case, style the light root node in the other case).

So if that's the case, there's nothing to change about this RFC – just the scoped style RFC: #50.

@caridy
Copy link
Contributor

caridy commented May 28, 2021

@nolanlawson can you elaborate more?

@nolanlawson
Copy link
Contributor

@caridy Basically my thought process is:

  1. Global styles: acts as if a <style> tag is inserted in situ. This aligns with the behavior of vanilla web components. Moving forward, there is no need for us to maintain transpilers for this system. :host works as-is.
  2. Scoped styles: imitates shadow DOM, but without shadow DOM. Styles are scoped to that component (not children). Since we're already imitating shadow DOM, transpiling :host into a selector for the component root node makes sense here. This option will always need a transpiler.

@pmdartus pmdartus merged commit 914e225 into master Sep 10, 2021
@pmdartus pmdartus deleted the abdulsattar/light-dom branch September 10, 2021 09:29
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.

None yet

9 participants