webcomponents / polyfills

Web Components Polyfills
BSD 3-Clause "New" or "Revised" License
1.14k stars 165 forks source link

[scoped-custom-elements-registry] Setting attributes doesnt work as expected when polyfill enabled #483

Closed thepassle closed 2 years ago

thepassle commented 2 years ago

Description

Using lit-element@2.4.0 and @open-wc/scoped-elements@1.3.4 we're noticing some unexpected behavior when setting boolean attributes. The following code:

return html`<scope-me ?fooBar=${true}></scope-me>`;

Behavior without loading the polyfill: Sets a foobar attribute on <scope-me> and it also sets the fooBar property to true

Behavior with loading the polyfill: Sets a foobar attribute on <scope-me>, but the fooBar property is undefined

Example

/**
 * Specific version:
 *
 * @webcomponents/scoped-custom-element-registry@0.0.3
 * @open-wc/scoped-elements@1.3.4
 * lit-element@2.4.0
 */

// Disabling this import will make `ScopeMe` render ✅
import '@webcomponents/scoped-custom-element-registry';

import { LitElement, html } from 'lit-element';
import { ScopedElementsMixin } from '@open-wc/scoped-elements';

class ScopeMe extends LitElement {
  static get properties() {
    return {
      fooBar: { type: Boolean },
    };
  }

  render() {
    return html`hello ${this.fooBar ? '✅' : '❌'}`;
  }
}

class MyEl extends ScopedElementsMixin(LitElement) {
  static get scopedElements() {
    return {
      'scope-me': ScopeMe,
    };
  }

  render() {
    return html`<scope-me ?fooBar=${true}></scope-me>`;
  }
}

customElements.define('my-el', MyEl);

Stackblitz repro

Version

0.0.3

Browsers affected

LarsDenBakker commented 2 years ago

@kevinpschaaf @justinfagnani I guess browsers lowercase the attribute, I'm not sure if that's done before setAttribute is called or before attributeChangedCallback is called. What do you think would be the best behavior for the polyfill to follow?

justinfagnani commented 2 years ago

setAttribute is called from userland, so it may receive mixed case attribute names. attributeChangedCallback is called by the browser and probably only received lower case names. Looks like the patch for setAttribute needs to lowercase the attribute before calling attributeChangedCallback.

thepassle commented 2 years ago

Looks like the patch for setAttribute needs to lowercase the attribute before calling attributeChangedCallback.

That does indeed seem to lead to the expected behavior: https://stackblitz.com/edit/js-mnczsr?file=polyfill.js image