feat(computedFrom): support expressions

[jdanyow]

fixes #149

[EisenbergEffect]

Very interesting stuff. With this in place, what do you think about adding the auto-dependency checking stuff that you had in that other plugin? Is it easy to add on top of this now that it's there? Or is that completely different?

[jdanyow]

Definitely a step towards integrating that. If we do add the aurelia-computed capability here it would probably need to be opt-in via a decorator right? The explicit nature of the decorator would allow us to throw errors when we are not able to parse the expression(s) in the getter function's toString result.

The other thing related to this I'm investigating is support for #147 by adding logic to @computedFrom that would rewrite this:

@computedFrom('some', 'stuff')
myClassMethod(myArg) {
  ...
}

to the equivalent of something like this:

_myClassMethod(myArg) {
  ...
}

@computedFrom('some', 'stuff')
get myClassMethod() {
  return _myClassMethod.bind(this);
}

Then it should "just work":tm: provided this line is added here.

What do you think?

[EisenbergEffect]

Sounds pretty good to me.

[EisenbergEffect]

Should we rename computedFrom to computed so that if you use it without any args it will auto-compute the props? (We can keep computedFrom and just create an alias to computed and deprecate the old one.)

[jdanyow]

computed sounds good to me. Will create a PR for that and the aurelia-computed logic integration.

Update on #147: my plan didn't quite work. The changes to the decorator:

export function computedFrom(...rest) {
  return function(target, key, descriptor) {
+    if (!descriptor.get) {
+      let innerKey = '__' + key;
+      Object.defineProperty(target, innerKey, { enumerable: false, configurable: true, value: descriptor.value });
+      descriptor.get = function() { return this[innerKey]; };
+      descriptor.set = function(newValue) { this[innerKey] = newValue; };
+      delete descriptor.value;
+      delete descriptor.writable;
+    }

    descriptor.get.dependencies = rest;
    return descriptor;
  }
}

Basically say "when the dependencies change, notify the binding system that the decorated class method changed". This doesn't work. The class method doesn't ever change. The result of invoking the class method potentially has changed. This check (which is a valid check) prevents the change event from bubbling up and causing the binding to reevaluate.

Will need to think about #147 a bit more while working on the rest of this stuff.

OK to merge what I have so far?

[jdanyow]

this was merged but rebasing the branch prevented the PR from closing automatically. Closing this manually...

[sormy]

Does it work well? It looks like even previously working syntax without expressions stop to work well.

I got master, build it and use method like that:

  @computedFrom('offset', 'topOffset', 'hourHeight')
  get top() {
    console.log('event get top()')
    return this.topOffset + this.offset * this.hourHeight / 60;
  }

and the code fall back to dirty checking and eating all cpu.