WebReflection / hyperHTML-Element

An extensible class to define hyperHTML based Custom Elements.
ISC License
202 stars 25 forks source link

Proposal: automatically add getter and setter for not primitive values #53

Closed jiayihu closed 5 years ago

jiayihu commented 5 years ago

IMO, for a Web Component is really common having to deal with object attributes and I find writing a getter/setter for each of them to be annoying. Besides the source code is not very concise and I don't like my object properties not listed in observedAttributes, making them "special" compared to primitive properties. Is there any way to improve the situation?

My first idea is to create getter/setter in the library, as follows for instance:

    const observedAttributes = Class.observedAttributes || [];
    observedAttributes.forEach(name => {
      // it is possible to redefine the behavior at any time
      // simply overwriting get prop() and set prop(value)
      if (!(name in proto))
        defineProperty(proto, camel(name), {
          configurable: true,
          get() {
            return this.getAttribute(name) || this[`_${name}`];
          },
          set(value) {
            if (typeof value === 'object' && value !== null) {
              this[`_${name}`] = value;
            } else {
              if (value == null) this.removeAttribute(name);
              else this.setAttribute(name, value);
            }
          },
        });
    });

This would allow me to just return the property name in observedAttributes like any other.

WebReflection commented 5 years ago

This is a class, you can extend it however you like through either its constructor or the created method.

jiayihu commented 5 years ago

Okay, just for feedback and for future readers, this is a first attempt to it:

import HyperHTMLElement from 'hyperhtml-element';

export class MicroReadElement<State = {}> extends HyperHTMLElement<State> {
  static define(name: string, options?: ElementDefinitionOptions) {
    HyperHTMLElement.define.call(this, name, options);

    const observedAttributes = this.observedAttributes || [];
    const proto = this.prototype;

    observedAttributes.forEach(name => {
      Object.defineProperty(proto, name, {
        configurable: true,
        get() {
          return this[`_${name}`] || super[name];
        },
        set(value) {
          if (typeof value === 'object' && value !== null) {
            this[`_${name}`] = value;
          } else {
            super[name] = value;
          }
        },
      });
    });
  }
}
sourcegr commented 5 years ago

FWIW, I believe this would be a very nice addition to the core. It would allow more flexibility for devs out of the box.

WebReflection commented 5 years ago

hyper-element-base is free in npm, feel free to add that.

We use this in production already so we're not planning to change current class with something kinda ambiguous (attributes ... maybe? what if it's a primitive then an object?)

If it was less magic, I might have thought of it, but right now observedBooleans plus observeAttributes give us already all we need to do tons of crazy stuff 🎉