Complications from shadyCSS @apply shim and lit polyfill support

steverep commented 3 weeks ago

Checklist

[X] I have updated to the latest available Home Assistant version.
[X] I have cleared the cache of my browser.
[X] I have tried a different browser to see if it is related to my browser.
[X] I have tried reproducing the issue in safe mode to rule out problems with unsupported custom resources.

Describe the issue you are experiencing

Users have been reporting that some custom cards stopped working after being pushed to the legacy build (mistakenly for the iOS companion app). As @piitaya pointed out in https://github.com/home-assistant/iOS/issues/2908#issuecomment-2293615805, this is the result of those cards using dynamic styles in their templates, which are not supported by the shadyCSS polyfill. However, we only load that polyfill when shadow DOM is not supported, so it shouldn't be an issue. I decided to look into what's going on here.

What we do load unconditionally in the legacy build is lit/polyfill-support, which is needed whenever the web components polyfills are active. That support adds some functions for Lit with a short-circuit that looks like:

  if (
    window.ShadyCSS === undefined ||
    (window.ShadyCSS.nativeShadow && !window.ShadyCSS.ApplyShim)
  ) {
    return;
  }

It turns out that this short circuit doesn't work because we still use some <paper-*> components, which always load the shim for CSS @apply (via @polymer/polymer/polymer-legacy.js). So the end result is that the lit/polyfill-support effects are always active on the legacy build. Whether @apply is used by the cards or not, Lit's polyfill support ends up also forcing those shadyCSS limitations.

Describe the behavior you expected

Fortunately, there's a number of ways to address these complications:

[ ] Custom cards should stop using dynamic styles in their templates. Aside from these complications, they are terribly inefficient in lit, and will also present issues if a CSP is being enforced. Where CSS alone cannot do the job, lit's classMap or styleMap directives should be used. @piitaya I think you suggested adding some notes to the docs on this and perhaps a developer blog post - can you take care of that?
[ ] We don't load lit/polyfill-support on the modern build at all, so clearly we don't need it just for the @apply shim (probably because no lit elements actually use it, just the paper-* polymer elements). I'll modify this so we only load it when shadyDOM is active.
[ ] Remove remaining uses of <paper-*> elements, and thus the @apply shim will stop polluting our bundles along with plenty more polymer stuff. Thanks to @silamon, these have mostly been eradicated and there's only a few locations left (sidebar and dashboard tabs).
[ ] Lastly, we have a few locations where we use @apply with legacy paper styles. It's time to get rid of those, especially since the @apply proposal was abandoned years ago as a bad idea.

Steps to reproduce the issue

Can verify why the lit polyfill support is active just by looking at window.shadyCSS in the browser console.

What version of Home Assistant Core has the issue?

dev

What was the last working version of Home Assistant Core?

No response

In which browser are you experiencing the issue with?

n/a

Which operating system are you using to run this browser?

n/a

State of relevant entities

No response

Problem-relevant frontend configuration

No response

Javascript errors shown in your browser console/inspector

No response

Additional information

No response

steverep commented 3 weeks ago

@bramkragten or @piitaya, I removed some orphan @apply uses in #21751, but it looks like we actually have a few uses in lit elements in the developer tools:

3 results - 2 files

src/panels/developer-tools/event/developer-tools-event.ts:
  162:           @apply --paper-font-body1;
  183:           @apply --paper-font-title;

src/panels/developer-tools/template/developer-tools-template.ts:
  314:           @apply --paper-font-code1;

Can we get rid of those? I suspect further investigation might show those amount to very minor bugs/differences between modern and legacy for those elements because of this issue.

silamon commented 3 weeks ago

There are more usage of the apply in the codebase but they are polymer related.

There is not really much more left to remove polymer completely.

Dashboard tabs and developer tools tabs are pretty much done, but there's a prelimerary pr open at the moment before I open this one. The arrows are lost unfortunately, but the desktop scrolling works with some additional code.
Sidebar, there's been a few closed prs already to replace it. The new m3 list doesn't have a selected property, so we need to migrate it to mwc list without doing a refactoring since it may end up closed as well.
Remove polymer and the css variables. The hardest part is the fact that it is still being used all over the pages, we might need to introduce specific ha css variables that follow the material design so we are less dependant to changes to the material libraries.

silamon commented 3 weeks ago

@bramkragten or @piitaya, I removed some orphan @apply uses in #21751, but it looks like we actually have a few uses in lit elements in the developer tools:
3 results - 2 files

src/panels/developer-tools/event/developer-tools-event.ts:
  162:           @apply --paper-font-body1;
  183:           @apply --paper-font-title;

src/panels/developer-tools/template/developer-tools-template.ts:
  314:           @apply --paper-font-code1;
Can we get rid of those? I suspect further investigation might show those amount to very minor bugs/differences between modern and legacy for those elements because of this issue.

You can deduplicate them. That is look up what properties they set and copy them over and use css variables. But removing those will still leave the apply in the ha-style and I think it needs removal their too.

steverep commented 3 weeks ago

You can deduplicate them. That is look up what properties they set and copy them over and use css variables. But removing those will still leave the apply in the ha-style and I think it needs removal their too.

At least for this issue, I left out the ones in ha-style as they are appended to the document rather than a lit element, so they shouldn't cause any modern/legacy differences. Keeping them just means we need to keep the shim.

home-assistant / frontend