lit / rfcs

RFCs for changes to Lit
BSD 3-Clause "New" or "Revised" License
15 stars 10 forks source link

[RRFC] Add ability to set dynamic styles programatically after render #13

Open andy-blum opened 1 year ago

andy-blum commented 1 year ago

Motivation

In the documentation for styles, there is a brief section dynamic styles and classes. In this section it provides the below example (modified for clarity):

constructor() {
    super();
    this.styles = {color: 'lightgreen', fontFamily: 'Roboto'};
  }
  render() {
    return html`
      <div style=${styleMap(this.styles)}>
        Some content
      </div>
    `;
  }

The stylemap directive simply iterates over Object.entries(this.styles) and uses Array.prototype.reduce() to form a string that can be rendered as part of the component's own lifecycle. Styles added this way, however, are blocked when using a CSP with the style-src directive (MDN)

From the MDN page above:

Inline style attributes are also blocked:

<div style="display:none">Foo</div>

As well as styles that are applied in JavaScript by setting the style attribute directly, or by setting cssText:

document.querySelector('div').setAttribute('style', 'display:none;');
document.querySelector('div').style.cssText = 'display:none;';

However, styles properties that are set directly on the element's style property will not be blocked, allowing users to safely manipulate styles via JavaScript:

document.querySelector('div').style.display = 'none';

Example

On a page with the CSP header Content-Security-Policy: style-src https://example.com/

This code adds the styles to the markup, but the browser does not apply those styles, blocking them per the content security policy.

constructor() {
    super();
    this.styles = {color: 'lightgreen', fontFamily: 'Roboto'};
  }
  render() {
    return html`
      <div style=${styleMap(this.styles)}>
        Some content
      </div>
    `;
  }

How

I propose adding a new @ expression or similar, as we would for attaching event listeners, to programatically attach stylemap-ready objects post-render:

constructor() {
    super();
    this.styles = {color: 'lightgreen', fontFamily: 'Roboto'};
  }
  render() {
    return html`
      <div @style=${this.styles}>
        Some content
      </div>
    `;
  }

In this situation, the html tagged literal wouldn't render style="color: lightgreen; font-family: 'Roboto';", but instead render the div without a style element and then post-render run:

div.style.color = lightgreen;
div.style.fontFamily = 'Roboto';

References

justinfagnani commented 1 year ago

The core of lit-html has no knowledge of classes and styles, they're just attributes, and classMap() and styleMap() are just directives. We don't want to add code to lit-html's core that deals directly in these concepts, so if we want to change the behavior of styleMap() we have to do it in that directive.

One way to do this would be to have styleMap() take a second options argument to tell it to skip the string rendering and go directly to updating the style object, which would happen around here: https://github.com/lit/lit/blob/main/packages/lit-html/src/directives/style-map.ts#L72

We'd need a name for this option. I'll call it directUpdate for now. It would look like:

constructor() {
    super();
    this.styles = {color: 'lightgreen', fontFamily: 'Roboto'};
  }
  render() {
    return html`
      <div style=${styleMap(this.styles, {directUpdate: true})}>
        Some content
      </div>
    `;
  }
andy-blum commented 1 year ago

How would that integrate with the html tagged string? Doesn't styleMap() currently evaluate before render() does?

I was thinking the stylemap stuff would need evaluated here, along side .property, ?boolean-attribute, and @event.

Similar to how lit-html doesn't have knowledge of the event's name, callback, or options, but still adds it:

    if (shouldAddListener) {
      this.element.addEventListener(
        this.name,
        this,
        newListener as EventListenerWithOptions
      );
    }

Functionality could be added to apply styles to an element:

    if (shouldAddStyles) {
      Object.entries(styleMap).forEach((property, value) => {
        this.element.style[property] = value;
      }
    }
justinfagnani commented 1 year ago

@andy-blum styleMap() is a directive, not just a function, so it's render() and update() methods are called by lit-html at the right times in the rendering pass.

The only thing that needs to be done here is to put a condition around the few lines that prime the previousStyleProperties set and return the result of render(). Something like:

    if (this._previousStyleProperties === undefined) {
      this._previousStyleProperties = new Set();
      if (this.directUpdate !== false) {
        for (const name in styleInfo) {
          this._previousStyleProperties.add(name);
        }
        return this.render(styleInfo);
      }
    }

You should be able to try this out by commenting out lines 69-72 locally and seeing that it works with CSP style-src.

andy-blum commented 1 year ago

I've been poking at this again, and it seems that styleMap handles inline styles under a CSP just fine - except for the initial render.

See this codepen: https://codepen.io/andy-blum/pen/JjZQrwj

Notice how the styles property sets the background, padding, and fontsize.

If you remove the requestUpdate in firstUpdated, it'll fail as the styles added in a way the CSP blocks.

justinfagnani commented 1 year ago

@kevinpschaaf I think this may be a reason to have update() not call render(), at least as an option.

kevinpschaaf commented 1 year ago

Given that this is a pretty core directive with CSP implications, we could just bite the bullet and make a node build for styleMap where the string render is only done under the node condition. Then it'd just work with no DX implications.