Alternative API for style modules?

[arthurevans]

Currently in 3.x, to create a style module (that you can then<style include="name">, you need to create a dom-module imperatively, inject the style, and call register on the style module.

Kinda like here:

http://plnkr.co/edit/tfODOF?p=preview

This is not a lot of code, but it doesn't seem like a great developer experience. Compared to, say,

import {registerStyleModule} from '/node-modules/@polymer/polymer/polymer-something.js';

registerStyleModule('my-shared-styles',
    `:host { 
      color: red;
      border: 2px solid blue;
     }`
);

(Or something.)

[Westbrook]

When moving to string templates and JS imports would we be able to get a style module my-shared-styles.js like:

export const mySharedStyles = `:host { 
      color: red;
      border: 2px solid blue;
     }`;

And then in your component start doing:

import mySharedStyles from `my-shared-styles.js`;
...
get static template() {
    return `<style>
            ${mySharedStyles}
            h2.styled-locally {
                ... etc ....
            }
        </style>
        <h2>My Element</h2>`;
}

Not sure which I'd say is better, but, if I understand correctly, shared style modules are duplicated into all of the components where they are applied anyways, so this might reduce some boot up time?

[kevinpschaaf]

@Westbrook Per #4937 the static template getter now needs to return an HTMLTemplateElement rather than a string (most easily via the new html tag function), and per the the pending change in #5023 we are going to disallow string interpolation via the html tag to ward off xss vulnerabilities, and instead require interpolation of other templates only. This just means that your export would need to be an actual template, but otherwise I think we will be promoting a similar pattern to the one you proposed:

export const mySharedStyles = html`
  <style>
    :host { 
      color: red;
      border: 2px solid blue;
    };
  </style>`

import mySharedStyles from `my-shared-styles.js`;
...
class MyElement extends PolymerElement {
  static get template() {
    return html`
      ${mySharedStyles}
      <style>
        h2.styled-locally {
          ... etc ....
        }
      </style>
      <h2>My Element</h2>`;
  }
}

[kevinpschaaf]

@arthurevans While I think the above will likely be what we promote for new development, we still need to decide how to migrate existing code. Had a discussion with @justinfagnani and @sorvell about this recently and we were debating the following options:

• update modulizer to perform the above transformation for style modules (more complexity for modulizer)
• leave the modulizer output as is, which inlines code that innerHTML's the dom-module into the body (ugly, but impresses the point that it is code that should be upgraded to the new pattern rather than maintained as-is)
• add a registerStyleModule type API to make maintaining existing code more ergonomic, but one-off for style modules
• add another tag like Polymer.upgradeHTML, which would do the innerHTML of existing <dom-module> code into the main document

Forget what we landed on, but hopefully this will spark discussion.

[arthurevans]

Given that we added htmlLiteral for interpolating strings, we could now use the original pattern suggested by @Westbrook, by wrapping the shared styles in htmlLiteral, right @kevinpschaaf?

That is,

export const mySharedStyles = htmlLiteral`:host { 
      color: red;
      border: 2px solid blue;
}`

Do you have any thoughts about whether this pattern would be preferred, versus using a full template?

[equinusocio]

@arthurevans Can you explain a more complicated use case, for example if we want to import and external css file..maybe after a postcss processing. How that can be achievable with your example?

BTW, as css developer, I personally don't a like a style definition inside a js file, mainly because if we want to pre/post process the style we need a way to import the css without manually injecting it. I think that css should remain pure css and not a string inside a js file, or maybe not at the definition level. Users should write pure css and be able to integrate others tools. Starting from a string inside a js file does not make easy integrate eg. postcss or any other css parsing.

Right now, with Polymer Skeleton and webpack we just import css files a normal moduleß (using the postcss-loader obviously) and interpolate them inside the template string:

my-element.js

import {Element as PolymerElement} from '@polymer/polymer/polymer-element';
import './../../dumbs/sk-menu';

import cssHueRotate from '../../common-styles/keyframes/hue-rotate.pcss';
import css from './style.pcss';
import template from './template.html';

export default class SkApp extends PolymerElement {


  static get template() {
    return `
      <style>${cssHueRotate} ${css}</style> ${template}`;
  }
}

window.customElements.define('sk-app', SkApp);

hue-rotate.pcss

@keyframes HueRotate {
  0% {
    filter: none;
  }

  50% {
    filter: hue-rotate(360deg);
  }

  100% {
    filter: none;
  }
}

Is that something achievable?

[mdwragg]

👀 Yeah, I put one of our components (from predix-ui.com) through the modulizer and got scared...!

I don't have a proposal in mind but just outlining how we do things on our large polymer-based design system.

• Each component has a sass file e.g.:
  https://github.com/PredixDev/px-toggle/blob/master/sass/px-toggle.scss

• Which is built into a style module:
  https://github.com/PredixDev/px-toggle/blob/master/css/px-toggle-styles.html

• Which is included inside the template of the consuming component:
  https://github.com/PredixDev/px-toggle/blob/master/px-toggle.html#L26

Our design system is also completely theme-able the presence of a <custom-style> and a whole bunch of css variables allows us to tune all colours, typography, and more via a theme at the top of the application. Whatever mechanism allows us to keep on the path we've followed in Polymer 1 and 2 will make us happy with the outcome in 3.

Cheers!

[kevinpschaaf]

@equinusocio I think your use case is worth solving one way or another. The only reason your example wouldn't work (once updating to use the html tag function, which will be required in Polymer 3) is because the html tag function prevents interpolating non-literal text to prevent XSS holes by default. Options are to expose an ${unsafeHtmlValue(css)} function for the author to explicitly take responsibility for the trustworthiness of the content and allow html to interpolate it. Another is to make an extension (or configuration?) of postcss-loader that does the htmlLiteral wrapping as part of the webpack build. Which of these seems like a better fit?

[kevinpschaaf]

@mdwragg I think the idiomatic code for what you described going forward in the Polymer 3 module world would look like this:

1. px-toggle.scss is built into px-toggle-styles.js instead of px-toggle-styles.html
2. Rather than defining a dom-module with the shared style, px-toggle-styles.js would export const pxToggleStyles = html`<style>/* contents of scss here */</style>`;
3. Usage of the shared style in px-toggle.html's template would change from <link rel="import" href="css/px-toggle-styles.html"> and <style include="..."> to import pxToggleStyles from 'css/px-toggle-styles.js'; and ${pxToggleStyles} in the template.

Unfortunately, modulizer doesn't currently do this transformation because dom-module is more general and can be used for a lot more purposes than shared styles, so the modulizer transformation keeps all of the original HTML, and just makes the dom-module upgrade by injecting it into the main document, and then the normal machinery from Polymer 2 continues working.

If possible I'd recommend the manual migration I described in the bullets above since it's more idiomatic and maintainable going forward. An alternative is that we could provide an upgradeThisDom`...` tag that would do what modulizer does in a slightly more ergonomic way so that the usage sites for the shared styles require fewer changes. Which one of those appeals more?

[mdwragg]

Hi @kevinpschaaf, thanks for getting back to me.

I'd have to try it, but the upgradeThisDom sounds like it would work. If you get anything going, we'd be happy to try it. We owe you guys a visit at some point too, it's been ages...

[kevinpschaaf]

@mdwragg modulizer currently outputs something like the following for dom-modules not associated with an element definition (style module case):

const $_documentContainer = document.createElement('div');
$_documentContainer.setAttribute('style', 'display: none;');
$_documentContainer.innerHTML = `<dom-module>...</dom-module>`;
document.head.appendChild($_documentContainer);

So the "upgradeThisDom" function would basically just be that, parameterized:

function upgradeHtml(html) {
  const $_documentContainer = document.createElement('div');
  $_documentContainer.setAttribute('style', 'display: none;');
  $_documentContainer.innerHTML = html;
  document.head.appendChild($_documentContainer);
}

If modulizer put that function in scope and used it instead of inlining the code main-document injection code, then your modulized source for e.g. px-toggle-styles.html would look like this (less weird looking, and and your sass->js pipeline would just wrap the css in this cleaner boilerplate going forward):

upgradeHtml(`
  <dom-module id="px-toggle-styles">
    <template>
      <style>...</style>
    </template>
  </dom-module>
`)

We've discussed this internally, and the only reason we didn't just do this in modulizer is that the above code is still an extremely roundabout way of getting the shared style text content to the desired spot if you're not parsing anything natively in HTML anymore (vs. where this made more sense with HTML Imports). Rather than just importing the string and interpolating it into the template where needed (step 3 above, basically), instead you're creating a div, innerHTML'ing a custom element into it, which then boots up and registers its content in the dom-module map, and then when the custom element boots up and parses its template, it sees the style include, goes and finds the dom from the dom-module map, finds the styles in it, and clones them into the template. We're just uncertain we want to provide sugar to allow people to keep doing the roundabout thing rather than biting the bullet and doing the more idiomatic thing.

OTOH the one advantage is that you don't have to touch any of the usage sites (<style include="...">), and if that's a big enough deal for your codebase then you could have your sass pipeline just wrap the css directly into a .js file using the main-document injection boilerplate above, without any help from modulizer, since you're generating those files anyway. If that makes sense.

[jouni]

This is slightly off topic perhaps, but I’ve been wondering and I’m curious how the idiomatic way affects performance, if at all?

I mean, I kind of assume that the browser will have to do the style parsing for each instance of the element when it’s concatenated as a string to the template. But I also assume this is such an obvious issue, that browsers have optimizations in place for this, and it doesn’t matter how many element instances you have on the page (1..n), the cost for the styles is always the same.

With the Polymer 2 way, using dom-modules, my assumption has been that the style element, even though it’s inside a template tag, is only parsed once and under the hood, the browser just references a single style tree/document when it’s included/stamped for in an element instance.

Seeing the same styles repeated over and over again inside each individual element instance in dev tools makes me a little uneasy 😅

[mdwragg]

@kevinpschaaf - yeah, I think I need a lie down in a darkened room and 2 days to think about it...

True, we're going to be automagically generating this stuff from Polymer 2 components anyways so not sure it matters how grokkable the end result is. We'll be coding up in sass anyways and generating the style module for Polymer 2 'style js' for Polymer 3...

I'll chin stroke some more... Thanks for the thoughts though, all good intel.

[kevinpschaaf]

@jouni Using the pattern above, the style string concatenation into the template is done once ever for the template; from there identical <style> tags are stamped from the template into shadow roots, and browsers do indeed have optimizations to minimize the cost of identical stylesheets by reusing internal data structures for them.

We are also helping push ConstructableStylesheets and an imperative API to add these to custom elements and/or shadowRoots, so that the "identicalness" heuristic is even more explicit to the browser, and would also avoid the need for the <style> element in each shadow root.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[laddng]

@jouni Using the pattern above, the style string concatenation into the template is done once ever for the template; from there identical <style> tags are stamped from the template into shadow roots, and browsers do indeed have optimizations to minimize the cost of identical stylesheets by reusing internal data structures for them.

We are also helping push ConstructableStylesheets and an imperative API to add these to custom elements and/or shadowRoots, so that the "identicalness" heuristic is even more explicit to the browser, and would also avoid the need for the <style> element in each shadow root.

@kevinpschaaf I'm following up on this comment made a while back - I'm hoping to understand how to best apply common shared styles across 140 Polymer 3 components without stamping them into every Shadow DOM template with style modules - is there a recommended best practice now or a new API? It feels like this issue wasn't really addressed besides using the dom-module trick mentioned in the Polymer 3 documentation. Any leads or hints would be great - thank you!

[justinfagnani]

@laddng Polymer 3 doesn't support Constructible StyleSheets, so even the Polymer-specific style sharing with <dom-module> does stamp out <style> tags in each shadow root. While Constructible StyleSheets are a bit faster, browsers have really optimized having many identical stylesheets, Firefox being the latest to add this optimization. I wouldn't worry much about the duplication, especially if you're using a shared style via <dom-module>, because like @kevinpschaaf says, the style text is calculated once for every template, then cloned. Cloning <style> helps trigger the deduplication optimization that browsers implement.

So in Polymer 3 the best recommendation is this pattern:

const styleMod = document.createElement('dom-module');
styleMod.innerHTML = `
  <template>
    <style>
      :host {
        color: red;
        border: 2px solid blue;
      }
    </style>
  </template>
`;

styleMod.register('my-shared');

[laddng]

@justinfagnani Thank you for the fast response! This is very helpful and makes me more comfortable using the <dom-module> method with regards to the performance of shared styles.

[kevinpschaaf]

Thanks for the feedback. Will go ahead and close this issue, since there's an idiomatic, module-based pattern for sharing styles using html-tagged templates containing <styles> that can be interpolated into element templates (#4940 (comment)), and a reasonably concise migration pattern that @justinfagnani described (#4940 (comment)), and we're unlikely to improve on the API for this going forward.