lit / lit-element

LEGACY REPO. This repository is for maintenance of the legacy LitElement library. The LitElement base class is now part of the Lit library, which is developed in the lit monorepo.
https://lit-element.polymer-project.org
BSD 3-Clause "New" or "Revised" License
4.49k stars 318 forks source link

CSS injection based on property cascades to unset elements #1108

Closed blackfalcon closed 3 years ago

blackfalcon commented 3 years ago

Description (bug)

The following scenario is this. For a custom element that may have optional or additional CSS added to an element based on a boolean property, it appears that if the property is set, it not only updates the current element, but also any elements that appear after.

The component

import { LitElement, html } from "lit-element";

class MyElement extends LitElement {
  static get properties() {
    return {
      fixed: { type: Boolean }
    };
  }
  withStyle() {
    if (this.fixed) {
      return html`
        <style>
          :host {
            color: red;
          }
        </style>
      `;
    }
    return null;
  }
  render() {
    return html`
      ${this.withStyle()}
      <p>Hello world! From my-element</p>
    `;
  }
}

When used with the following HTML, all elements will be red

<my-element fixed></my-element>
<my-element></my-element>
<my-element></my-element>

But when used with this HTML, only the one element with the fixed property is colored red

<my-element></my-element>
<my-element fixed></my-element>
<my-element></my-element>

Acceptance criteria

When a user of an element adds a property that injects CSS, this is injected to only the element applied.

kevinpschaaf commented 3 years ago

Can you please provide a reproduction case (e.g. using JSBin), and be more specific about your environment?

I see no reason why this wouldn't work as expected with native Shadow DOM, but note that the Shady CSS polyfill does not support dynamically injected <style> at instance time. If you need your code to run on browsers requiring the polyfill, you must specify all styles using static styles, or else <style> tags must be unconditionally rendered in your render() method.

blackfalcon commented 3 years ago

You are right, it 'should' work. Here is a link to a test that @Westbrook put together. https://stackblitz.com/edit/j9k3ga?file=index.html

In the first view, it appears to work. But move the fixed prop to the first CE in the HTML and then you will see the cascade I discussed.

sorvell commented 3 years ago

Thanks for the reproduction. The behavior here makes sense and is an artifact of the limitations of polyfilling styling with ShadyCSS, let me explain...

Currently, support for polyfill based ShadyDOM/ShadyCSS rendering is baked into LitElement. Even when native Shadow DOM is used, ShadyCSS is still invoked to be able to support the deprecated @apply feature. Whenever ShadyCSS is used, it always defines styles once per element class based on the styles provided in the first rendering of the element. Therefore, if your first rendering has the "fixed" styling, all elements get that styling; however, if you manually put that style into subsequent renderings, only they will get the styling.

There are a few ways to workaround this issue:

  1. If you don't care about IE11, you can just not load the webcomponents polyfills.
  2. If you're ok with different behavior on polyfilled browsers, you can use the webcomponents-loader.js to optionally load the polyfills as is shown in this working example.
  3. You could create an initial dummy version of the element without the styling, add and remove it from the DOM and then subsequent versions will work (in native but not polyfilled Shadow DOM).

Hope that helps.