Let sub classes easily override the SkateJS accessors

[trusktr]

• Bug
• Feature

Requirements

• Read the contribution guidelines.
• Wrote tests.
• Updated docs and upgrade instructions, if necessary.

Rationale

Before this change, it was not possible to do this:

import {withUpdate, props} from 'skatejs'

class Base extends withUpdate(HTMLElement) { ... }

class MyEl extends Base {
  static props = {
    foo: props.number,
    bar: props.boolean,
  }

  get foo() {
    return super.foo // delegate to SkateJS getter
  }
  set foo(val) {
    if (typeof val === 'function')
      Motor.addRenderTask('') // skip SkateJS setter function values
    else
      super.foo = val // otherwise delegate to SkateJS setter
  }
}

Implementation

This change makes it possible in the simplest way: instead of caching props stuff on the leaf-most classes, this caches stuff on the class that is generated by the withUpdate mixin.

Before, it was not possible to define your own setters/getters because you would then be forfeting SkateJS functionality (or having to re-implement it).

So this change makes it so that subclasses can call SkateJS setters/getters by taking advantage of super.

Open questions

In this implementation, if two or more subclasses define a prop with the same name, then they'll both use the same setter/getter on the withUpdate base class. I think this is fine, unless I overlooked something.

Basically, the class returned from withUpdate will contain all the setters/getters for all the props of all classes extending from it, but I think this has no impact on end users making the sub classes, they won't think about it.

The getters/setters will anyways always be called with this set to the relevant instance, and the logic is always the same.

---

So in fact, this should improve memory usage by some degree because now there won't be duplicate getters/setters per leaf class, but rather shared in the base class.

---

I'm not sure how this would apply in v6 yet, but this is what I had to do so that I could stick withUpdate on one of my base classes, then I was able to selectively trigger withUpdate logic in certain cases (like my above example).

---

In my use case, I need to be able to do this:

el.position = [x, y, z] // sets the position value, triggers withUpdate reactivity

or

// animated position, side-steps SkateJS withUpdate logic this time,
// and then it will trigger reactivity at 60fps.
el.position = (x, y, z, time) => [x, 400*Math.sin(time/800), z]

[treshugart]

v6 is getting a get() and a set() on the prop definition. I think a more forward-compatible implementation would just implement that in v5.

[trusktr]

I think a more forward-compatible implementation would just implement that in v5.

If you keep mixins in v6, I'd be happy just moving to v6 and going from there!

[treshugart]

If you keep mixins in v6, I'd be happy just moving to v6 and going from there!

Mixins are just component() now. I've commented on other issues that discuss pulling out some imperative parts to give you props on non-DOM-elements. Can discuss over there.

[treshugart]

I think this should definitely be done, but I think it needs to be reintegrated with the latest updates.