webtides / element-js

Simple and lightweight base classes for web components with a beautiful API
MIT License
26 stars 3 forks source link

enhance propertyOptions #51

Closed quarkus closed 2 years ago

quarkus commented 2 years ago

We should discuss how to add more option to the propertyOptions getter.

I recently came across a bug where an attribute text="1E8" was parsed to a property text = 100000000 . Thats make total sense as 1E8 is a number to JS but in my case (trying to highlight a substr) this was leading to a bug.

My first Idea was to add sth like:

propertyOptions() { return { text: { parse: false, }, }; }

Lit does it by explicitly adding types ... what do we prefer ?

static get properties() { return { prop1: { type: String, reflect: true }, };}

@lukas-schardt @eddyloewen Opinions ?

eddyloewen commented 2 years ago

I think I like it!

Lit also has the concept of "converters":

prop1: {
  converter: {
    fromAttribute: (value, type) => {
      // `value` is a string
      // Convert it to a value of type `type` and return it
    },
    toAttribute: (value, type) => {
      // `value` is of type `type`
      // Convert it to a string and return it
    }
  }
}

But I think that is bit to complicated. We probably don't need that much complexity here.

With just "parse" and "reflect" everything should be possible. We could also make it a bit more flexible by not only allowing for boolean values but also callbacks to manually parse/reflect the properties/attributes.

prop1: {
  parse: false,
  reflect: true,
},
prop2: {
  parse: (value) => {
    return value.toString();
  },
  reflect: (value) => {
    return JSON.stringify(value);
  },
}
quarkus commented 2 years ago

I think i like the second approach! We´ll end up with a pretty simple interface that will be fine for 98% of our use cases.

And by also allowing functions it can be arbitrarily complex if you want too.