Netflix / x-element

A dead simple starting point for custom elements.
Apache License 2.0
28 stars 12 forks source link

Bug: issue with using property shadowing element.id #51

Closed klebba closed 3 years ago

klebba commented 4 years ago

Sample case:

import XElement from 'https://import.dog/@netflix/x-element/x-element.js';

export default class Demo extends XElement {
  static get properties() {
    return {
      id: {
        type: Number,
      },
      value: {
        type: Number,
      },
    };
  }

  static template(html) {
    return ({ id, value }) => html`
      <style>
        :host {
          display: block;
          width: 5em;
          height: 5em;
          background-color: yellow;
        }
      </style>
      <div>${id}</div>
      <div>${value}</div>
    `;
  }
}

customElements.define('id-number-bug', Demo);

If we inspect the id-number-bug in the DOM the id property will be 0 instead of undefined which is unexpected. If we inspect the value property, it will be undefined. It appears that shadowing id property of an element is a special case, though it's not obvious why

theengineear commented 3 years ago

A couple notes on this one:

  1. Since id shadows the HTMLElement prototype, the current version of x-element will now warn about that issue.
  2. Since we no longer do type coercion, you would actually get an ERROR in this case if/when browser internals were setting it to something like "" or if an integrating component was setting "id".

In general, we probably don't want to shadow "id" though.

I'm going to close this down since I believe this is a non-issue with the most recent release.