@bindable should support getter/setter

[istrau2]

I'm submitting a feature request

Current behavior:
The @bindable decorator overrides virtual getters/setters

Expected/desired behavior:
The @bindable decorator should respect virtual getters/setters (i.e. it should wrap any already existing getter/setter with its own getter or setter).

Using getters/setters makes for some really beautiful code in Aurelia. When combining them with the @computedFrom decorator as well as some custom decorators that I have:

• @connected (for a property that is connected to a redux store)
• @awaitedBindable (for an asynchronous value)

I end up writing almost no proper functions. Instead my viewModels end up looking like the glue that the y are supposed to be, just a bunch of getters/setters.

The one big problem that I am encountering is the @bindable decorator. Because I can't define virtual getters and setters, I am forced to use the propertyChanged convention. This ends up uglier, more confusing and I end up running into a bunch of infinite loops.

[Alexander-Taran]

Would you like to provide a pull request for it @istrau2 ?

[istrau2]

@Alexander-Taran

I would like to yes, question is if I will have the time. It touches a pretty sensitive area in Aurelia. I'll try to submit something in the next week or two.

[Alexander-Taran]

Probably won't get into au 1.x
so no rush

[Alexander-Taran]

thing is.. @bindable already makes a getter/setter out of a propery

[istrau2]

@Alexander-Taran Right, that is why in my OP I said:

it should wrap any already existing getter/setter with its own getter or setter

something like:

//inside decorator
const originalGetter = descriptor.get;
descriptor.get = function bindableFunction() {
     // do stuff
     if (originalGetter) {
          originalGetter.call(this);
     }
}

[Alexander-Taran]

Exactly what I thought (-:

[obedm503]

Been having the same problem. If the use-case is simple, it can be solved with a guard and early return on the Changed callback. but if the case is slightly more complex, it very quickly and easily becomes spaghetti. this would be a very nice addition

[thomas-darling]

Yeah, we really need this.
I'm trying to bind to a domain model, which exposes its state as readonly properties, while mutations are done by calling methods on it. The propertyChanged callbacks are no help here, as I need the getter to expose the state of the model - otherwise I'd get into some nasty manual observation to keep the view model properties in sync with the domain model.

Example use case:

@computedFrom("_model.branch")
public get branch(): Branch
{
    return this._model.branch;
}

public set branch(value: Branch)
{
    this._model.setBranch(value.name, true);
}

Being able to do this, would be really awesome.
If we then also use something like aurelia-computed, so the computedFrom decorator can be omitted, then this starts to look pretty damn close to a perfect view model!

@Alexander-Taran Why do you say this would probably not make it into Aurelia 1.x?

[EisenbergEffect]

I believe we have something that will address this already implemented for our vNext. I was hoping to backport it to vCurrent. One reason I'm waiting is because there's a bit of an issue with tc39 and how the proposed private fields will work that will cause potential runtime errors when used with proxies. Some of the experimental code for dependency tracking that we have leverages proxies. Before we push that out anywhere we want to have a better feel for what the caveats will be with respect to future JS features.

Having said all that, take a look here https://github.com/aurelia/experiment/blob/master/src/runtime/binding/computed-observer.ts This is the new mechanism we've been experimenting with for properties that have a getter or a getter/setter. Here's where we determine which observer to instantiate based on getter/setter patterns: https://github.com/aurelia/experiment/blob/master/src/runtime/binding/computed-observer.ts#L22 @thomas-darling If I understand you correctly, your scenario would just be a simple wrapper around a getter/setter, which is this implementation https://github.com/aurelia/experiment/blob/master/src/runtime/binding/computed-observer.ts#L51

Let us know if you think this will meet your needs. There is some work to backport this, but it's not too difficult. You could actually turn this into a plugin that is registered as a custom adapter today probably.

[istrau2]

@EisenbergEffect Sounds like you are planning big changes in the implementation of computed-observer. Wondering how these changes would affect these two plugins:

https://github.com/stellarport/aurelia-async-bindable-bluebird
https://github.com/stellarport/aurelia-redux-connect

[EisenbergEffect]

If you have a getter or setter today, aurelia has to do dirty checkig, based on a timer, which isn't great. This would upgrade that to a mechanism that would attempt to detect dependencies. I believe that it should work with plugins that create getter/setters as along as those plugins respect the existing getters and setters by properly wrapping them. I haven't tested that out though. If we bring this to vCurrent then there will probably be a setting to opt-in so it doesn't unintentionally break things that are somehow dependent on the previous behavior.

[thomas-darling]

Yeah, I assume this is the one you're referring to:
tc39/proposal-class-fields#106

That's an unfortunate situation they are creating there.
That said though, proxies are probably the only good way to solve this, so we'll just have to fight it - and looking over the vNext binding code, it does indeed seem to handle my use case nicely :-)

A side note
In vCurrent, there seem to be some inconsistencies in the behaviour of properties with @bindable, properties with @observable, and properties that are bound to from the view - including whether getters and setters work, and the timing of when the set/changed method is called. I'd really like to see more consistency around this in vNext, as it's currently a bit confusing.

[EisenbergEffect]

@thomas-darling We're definitely working to improve those exact things for vNext.

[thomas-darling]

Awesome - much appreciated! 👍