Binding should expose an `observable` decorator

[plwalters]

Should be a subset of the bindable functionality that can be used on any class instance field -

import {observable} from 'aurelia-binding';

export class MyViewModel {
  user;
  constructor() {
    this.user = new User();
  }
}

class User {
  @observable name = '';
  nameChanged(newValue) {
    console.log(newValue);
  }
}

[jdanyow]

I like this idea!

I think this will do the trick

function observable(target, propertyName, descriptor) {
  // use a convention to compute the inner property name and the callback 
  // function name.
  let innerPropertyName = `_${propertyName}`;
  let callbackName = `${propertyName}Changed`;

  // set the initial value of the property.
  target[innerPropertyName] = descriptor.initializer();

  // we're adding a getter and setter which means the property descriptor
  // cannot have a "value" or "writable" attribute
  delete descriptor.writable;
  delete descriptor.initializer;

  // add the getter and setter to the property descriptor.
  descriptor.get = function() { return this[innerPropertyName]; };
  descriptor.set = function(newValue) {
      let oldValue = this[innerPropertyName];
      this[innerPropertyName] = newValue;
      if (this[callbackName]) {
        this[callbackName](newValue, oldValue);
      }
    };

  // make sure Aurelia doesn't use dirty-checking by declaring the property's
  // dependencies. This is the equivalent of "@computedFrom(...)".
  descriptor.get.dependencies = [innerPropertyName];
}

[StrahilKazlachev]

What about an option to explicitly specify the changed handler?

[jdanyow]

That would be cool- also- this decorator doesn't seem to work in TypeScript.

[plwalters]

@jdanyow do you think it would work somehow to break this out somehow to work from multiple decorators? That way both @bindable and @observable could use the same code to implemented the changeHandler?

[EisenbergEffect]

@pwkad That's a tricky question. There are parts of that code that are very behavior-specific, such as the options check, for example.

[plwalters]

@jdanyow I just got around to testing what you put in and it seems to work perfect on a class instance field. If I add some tests to binding to support this as a class decorator and a class instance field decorator think we can get this in the next release?

[jdanyow]

Sounds good to me- some outstanding questions/concerns:

1. does it work in typescript
2. what repo does it go in?
3. maybe the name isn't quite right. Maybe something like @notifyChanged or @notifies is more accurate?
4. option to specify the handler function name

@EisenbergEffect thoughts?

[jdanyow]

I think this will preserve the default value, even in typescript:

function observable(target, propertyName, descriptor) {
  // use a convention to compute the inner property name and the callback 
  // function name.
  let innerPropertyName = `_${propertyName}`;
  let callbackName = `${propertyName}Changed`;

  // typescript or babel?
  let babel = arguments.length === 3;

  if (babel) {
    // babel passes in the property descriptor with a method to get the initial value.
    // set the initial value of the property.
    target[innerPropertyName] = descriptor.initializer();
  } else {
    // typescript doesn't pass in the property descriptor.
    descriptor = {};
  }

  // we're adding a getter and setter which means the property descriptor
  // cannot have a "value" or "writable" attribute
  delete descriptor.writable;
  delete descriptor.initializer;

  // add the getter and setter to the property descriptor.
  descriptor.get = function() { return this[innerPropertyName]; };
  descriptor.set = function(newValue) {
      let oldValue = this[innerPropertyName];
      this[innerPropertyName] = newValue;
      if (this[callbackName]) {
        this[callbackName](newValue, oldValue);
      }
    };

  // make sure Aurelia doesn't use dirty-checking by declaring the property's
  // dependencies. This is the equivalent of "@computedFrom(...)".
  descriptor.get.dependencies = [innerPropertyName];

  if (!babel) {
    Object.defineProperty(target, propertyName, descriptor);
  }
}

[Scapal]

This is a modified version accepting a changeHandler to be specified, both @observable value = 10;and @observable({changeHandler: ‘myHandler’}) value=10;are working.

 function observable(targetOrConfig, key, descriptor) {
    console.trace('observable');
    let deco = function(target, key2, descriptor2) {
        // use a convention to compute the inner property name and the callback 
        // function name.
        let innerPropertyName = `_${key2}`;
        let callbackName = (targetOrConfig && targetOrConfig.changeHandler) || `${key2}Changed`;

        // set the initial value of the property.
        target[innerPropertyName] = descriptor2.initializer();

        // we're adding a getter and setter which means the property descriptor
        // cannot have a "value" or "writable" attribute
        delete descriptor2.writable;
        delete descriptor2.initializer;

        // add the getter and setter to the property descriptor.
        descriptor2.get = function() { return this[innerPropertyName]; };
        descriptor2.set = function(newValue) {
            let oldValue = this[innerPropertyName];
            this[innerPropertyName] = newValue;
            if (this[callbackName]) {
                this[callbackName](newValue, oldValue);
            }
        };

        // make sure Aurelia doesn't use dirty-checking by declaring the property's
        // dependencies. This is the equivalent of "@computedFrom(...)".
        descriptor2.get.dependencies = [innerPropertyName];
    }

    if(key) {
        let target = targetOrConfig;
        targetOrConfig = null;
        return deco(target, key, descriptor);
    }

    return deco;
}

[EisenbergEffect]

@jdanyow Should we put this in the box? In the binding repo or elsewhwere? Of note on this implementation is that the callback timing will be different than other scenarios.

[jdanyow]

yes, I think so. Do you think the name @observable is appropriate or should it be something else? @notifies or @notifyChanged maybe?

@Scapal would you be interested in submitting a PR to the binding repo for this (with tests, etc)? (if not, no worries- we're just always looking for new contributers 🚀)

[EisenbergEffect]

I'm not sure what the best name is. I don't particularly like "notifies" or "notifyChanged" though...but I'm not sure if "observable" is the best name either...though I like that the best, mostly because of how it reads when combined with the property name.

[plwalters]

Yes @jdanyow and @Scapal have done a great job today of getting a working concept in place.

Personal opinion - @observable seems to be most common but does slightly conflict with other concepts. @notifies or anything similar doesn't feel right.

Here's a user story to back it up, maybe this helps (or maybe it makes me look crazy) -

As a property of a class,
When my value changes,
I want to conventionally call a changed handler method (or a supplied override).

@observable
@watcher
@watch
@subscribe

[Scapal]

Does this sound enough for a unit test?

import {observable} from '../../src/utils/observable-decorator.js';

describe('observable decorator', () => {
    it('changeHandler valueChanged should be called when changing the decorated property', () => {
        class Test  {
            @observable value = '';
            valueChanged () {
                this.test = this.value;
            }
        };

        var test = new Test();

        expect(test.test).not.toBeDefined();
        test.value = 'hello';
        expect(test.test).toBeDefined();
        expect(test.test).toEqual('hello');

        test.value = 'world';
        expect(test.test).toEqual('world');
    });

    it('changeHandler should be called when changing the decorated property', () => {
        class Test  {
            @observable({changeHandler: 'customHandler'}) value = '';
            valueChanged () {
                this.test = 'fail';
            }
            customHandler () {
                this.test = this.value + ' world';
            }
        };

        var test = new Test();

        expect(test.test).not.toBeDefined();
        test.value = 'hello';
        expect(test.test).toBeDefined();
        expect(test.test).toEqual('hello world');
    }); 
});