Closed jods4 closed 8 years ago
+1 :) (btw, is there no other way to create model listeners (watchers in angular terms) in the controllers?)
Well and another thing: if we write paths or expressions in @computedFrom, there's no error message, it simply doesn't do anyting. Neither does the documentation mention exactly what can you write in there.
+1
+1
+1
++1
+1 my own ticket along other people, just to attract attention here @EisenbergEffect
We've used Aurelia for 6 months in a complex project and there is something crucial missing; this might be part of the solution.
In UI you commonly have dependencies/computations between various elements. Some simple examples: list elements are filtered by other inputs (commonly just a textbox), you display the total of a shopping cart, you display shipping options only when the user did not choose "pickup" and is not in a foreign country, and so on.
A modern application needs to be able to express such values declaratively, as a computation from their dependencies:
get filteredList() { return this.list.filter(x => x.name.startsWith(x.search)) ; }
get grandTotal() { return this.list.reduce((total, x) => total + x.lineTotal); }
get showShippingOptions() { return this.shipping !== 'pickup' && this.country === 'US'; }
I've used properties for all these examples, although there can be other representations.
Aurelia needs a good story for those use-cases.
It could be some good integration with Reactive programming patterns and/or libraries, which are suited for those kind of things (and more!). I think Rx is great for many advanced scenarios but maybe a little overboard for something as simple as the motivating examples I've shown here.
Coming from Knockout, the examples above would 'just work', thanks to the inherent capability of KO to automatically discover and watch the dependencies of any computations (method, property or otherwise).
To be honest, the examples above would also 'just work' in Aurelia because they would trigger dirty checking. It's just that we need to be careful with dirty checking. Overusing it can lead to perf, battery or animation issues. Examples like the grand total can quickly become expensive to evaluate repeatedly. So we need to have another option.
@computedFrom
could be that tool but it only works for simple properties. That's too limiting and if it is the only solution to this problem it needs to beef up: at least start with sub-properties and arrays support.
I personally believe that this theme is very important to Aurelia and it's really overdue.
@jods4
I have a blog post coming out tomorrow on this topic, but:
get filteredList() { return this.list.filter(x => x.name.startsWith(x.search)) ; }
get grandTotal() { return this.list.reduce((total, x) => total + x.lineTotal); }
Are best handled using a valueConverter
. For example, filteredList
would be:
export class FilterValueConverter {
toView(array, search) {
return array.filter((item) => x.name.startsWith(search));
}
}
<require from="components/filterValueConverter"></require>
<select value.bind="selection">
<option repeat.for="item of array | filter: search" model.bind="item">
${item.name}
</option>
</select>
However, showShippingOptions
is best handled with computedFrom
, and you can do this today.
@computedFrom('shipping','country')
get showShippingOptions() { return this.shipping !== 'pickup' && this.country === 'US'; }
However, I'm with you and everyone else here. There is a legitimate use case for pathed arguments in computedFrom
. This is on our radar, but the +1's do help us assess the value of this feature.
@jods4 We've had a computed plugin for many months that does some of this. Maybe @jdanyow can provide some feedback on that...maybe it's time to roll it into the core?
@davismj You are nitpicking on the examples I gave. They were all made-up to illustrate and I agree some have alternate solutions. I have much more complex examples from real applications.
I agree the first two were very well suited to value converters. Especially because they are so re-usable across your app. Just imagine they're not ;)
The last one works... until I tweak my model to:
get showShippingOptions() { return this.shippingMethod !== 'pickup' && this.shippingAddress.country === 'US'; }
And now it does not.
Looking forward to your blog post BTW!
@EisenbergEffect I saw this plugin when it was fresh. I'm a bit wary of it. It's leaning toward too much magic for my taste:
return
expressions;In the end, if you're only going to use it for a few trivial computed properties, you might as well slap a computedFrom
on them and be done. At least that's how I feel vs the added complexity/bloat.
Mind you, I love the idea and actually was asking for some automatic dependency tracking when I first got into Aurelia. Coming from KO, this was a very sweet feature. I have realized that this is not easy to build into the platform (not without read detection) and accepted that we might have to use manual dependency declaration instead.
@jods4 Using @computedFrom
to solve the list filter problem is a common pitfall in Aurelia, and I wanted to make sure I pointed it out. As noted at the bottom of my comment, I am still in favor of making this an out of the box feature because there is a real use case for it.
@EisenbergEffect @jdanyow I'm all in favor of getting this feature into the core sooner than later. Without pathed arguments, @computedFrom
loses all of its value except in very special use cases.
@jods4 The automatic dependency checking is not possible to build without you explicitly declaring something. If you have some ideas on how this might work, please share. You could even built it as a plugin to prototype if you want. The system is extensible enough for that.
@EisenbergEffect At this point I gave up on automatic dependency checking (not for technical reasons, though. Can't say I won't ever look back).
FWIW my former ideas in this space were some kind of @dependencySource
annotation that would turn on setter instrumentation similar to the write detection in getters; and another @dependencyAuto
that would trigger watching those reads to establish a list of dependencies for arbitrary code/property.
@davismj thanks a lot, that is a very complete and flexible filter! As filtering is one of the most commonly performed operation, this is an awesome value converter to have. Suggestion: put it in the standard aurelia libraries! :sparkles:
That said I hope it's not "instead" of this ticket. There are other use cases where a value converter is not a good mental model for this. I don't think we should see value converters as the solution to declare dependencies for any kind computations.
If it were the case, we wouldn't need @computedFrom
and we probably would call them multi-binding
rather than valueConverter
.
@davismj how about Array.prototype.reduce
? I am using it like this:
// Return the sum of ranks of all Masteries
get rank(): number {
return this.masteries.reduce((previous, current) => previous + current.rank, 0);
}
Where
this.masteries: Array<Mastery>;
and Mastery
is
interface Mastery {
...
rank: number;
...
}
In this case the property rank
of all the Mastery
inside this.masteries
should be observed. There is no way to express that with @computedFrom
and aurelia-computed
fallbacks to dirty-checking.
@jods4 definitely not a replacement, just a tool I built that I hope will help the community.
I agree, @Gheoan.
@Gheoan there is a better alternative, I did it for a shopping cart I've implemented, and is using the bindingEngine. You need to observe both the this.masteries
array and the rank
field of each item.
Every time one of them change, I re-run my reduce function to calculate the total items in the cart.
_observeItems(bindingEngine) {
let subscriptions = [];
let observer;
let observeItemsAmountField = () => {
while (subscriptions.length > 0) {
subscriptions.pop().dispose();
}
this.items.forEach(item => {
observer = bindingEngine.propertyObserver(item, 'amount');
subscriptions.push(observer.subscribe(() => this._calculateTotalItems()));
});
};
observeItemsAmountField();
bindingEngine.collectionObserver(this.items).subscribe(() => {
this._calculateTotalItems();
observeItemsAmountField();
});
}
_calculateTotalItems() {
this.totalItems = this.items
.map(x => parseInt(x.amount, 10))
.reduce((a, b) => a + b, 0);
}
Of course this is more complex solution, but it works like a charm and it doesn't use dirty checking. Hope it helps :)
@nomack84 Thank you. In my case I don't need to observe the array (masteries
`items`) because its length is static.
@nomack84 that's the kind of things I find myself doing as well when I want to compute something that depends on others that @computeFrom
can't observe.
It seems evident to me that we need to find a way to hide this plumbing behind some Aurelia primitive. I'm fine with @computedFrom
but it needs to accept a more useful syntax than just a direct property.
We could imagine something like @computedFrom('items[].amount')
to encapsulate the code you've presented.
I remember mentioning the computedFrom
use-case when discussing the BindingEngine
API in another ticket, but it didn't get much traction unfortunately.
All- I wanted to share some of my thoughts/opinions on computed from- let me know what you think...
get fullName() { return this.user.firstName + ' ' + this.user.lastName; }
Why not dirty-check?@computedFrom('items[].amount')
, there's a point at which the cost of observing the amount property on each item in the array exceeds the cost of periodically calculating the sum. Dirty checking the "total" getter is a perfectly good approach. Alternatively you could also setup your binding such that dirty-checking isn't necessary, for example:
<input repeat.for="item of items" value.one-way="item.amount" change.delegate="updateItemAmountAndTotal(item, $event.target.value)">
In my experience there's usually a much better way to achieve the same result without observing tons of properties/expressions on arrays of objects. Making @computedFrom
more advanced won't make apps more performant and instead might make it easier for people to shoot themselves in the foot.
All that said, it should be pretty easy to enhance computedFrom to the point where it can understand the same expressions Aurelia supports in bindings, I'll review the existing PR for that and merge or suggest changes as-needed.
Be sure to use the upcoming "improved" computedFrom with caution...
@jdanyow I already saw the "sometimes dirty checking is better" argument. Most of the time I don't buy it and when I do I turn to alternative solutions.
Dirty checking is a dangerous rabbit hole. It's very easy to be unaware how many properties are dirty-checked. "A ton" can be a lot less than you'd expect before you have perf issues.
Even if you have a reasonable amount a quickly computed properties that are dirty checked, that periodic process (several times per second) can have negative impacts. It's not good for your battery life. It may interfere with your smooth, 60fps animations.
Refactoring things to observe less is good when you can. At the same time, change observation is the foundation of frameworks such as Aurelia. You do it a lot and it's often the most elegant way. It has to provide all the bells and whistles to be efficiently supported.
For scenarios like
get fullName() { return this.user.firstName + ' ' + this.user.lastName; }
Why not dirty-check?
Because that's exactly the kind of scenario where precise observation is more efficient and easy. Why dirty-check?
Some properties that are dirty checked may create a non-trivial amount of work. Also a non-trivial GC pressure, which is also important perf-wise. Especially examples that involve arrays... filtering then reducing a 1'000 elements array 10 times per second is not that cheap. If you do that often, your dirty checking costs can go up quickly. No need for tons of watched properties.
there's a point at which the cost of observing the amount property on each item in the array exceeds the cost of periodically calculating the sum.
Two comments: first it's comparing apples to oranges, because the costs are spread very differently. In one case the cost is additional memory + dependencies set-up at each change (better yet: offer a "one-time" mode for cases where we know the dependencies won't change after evaluation), in the other it's constant CPU usage (+ possible memory). So the comparison very much depends on how frequently data changes. Typically in front-ends (user driven): not very much. If you have optimized code when dependencies don't change, it's only a one-time fee + some memory, that's actually hard to beat.
Second, this is how it was typically done with KO and my practical experience is that it worked very nicely, quite a long way. Actually I never encountered a perf problem with this in my projects -- granted, I never did silly things such as watch a 10'000 items array.
Making
@computedFrom
more advanced won't make apps more performant and instead might make it easier for people to shoot themselves in the foot.
I'd argue that dirty checking is very much an easy way to shoot yourself in the foot. In the end, if it's so much better, why did Aurelia not use it for all its observables -- like Angular did? What is special in the core framework that it prefers instrumenting (lots of properties obviously) over plain dirty checking? I think this goes to show which one of the two scales best.
In my experience there's usually a much better way to achieve the same result without observing tons of properties/expressions on arrays of objects.
Well yes. If I need to observe huge things like a 1000 elements array, I'll probably rather use a signal. Or update things incrementally when I can. But neither a computedFrom
nor dirty checking. I think it was said support for signals was coming and this is a good news.
I don't quite like your option with change.delegate
because it puts core logic in the UI. It means any change that is triggered by some business logic is lost. Which is IMHO not a good design and against proper MVVM.
@jdanyow
aurelia-computed
does that).@computedFrom
.amount
property is already observed? I'm not sure how the observer-subscribers relation works, but there should be only one observer and one/many subscribers. If that's right the cost of adding one more subscriber (for each item
) should be lower than dirty checking the sum. Nobody wants an inefficient solution, but if we have a choice between being unable to detect changes and do it sub-optimally, I'll got the sub-optimal approach until someone finds a way of optimising the process.
I currently have to bodge dependent variables as dummy params into bindings to force re-evaluation of functions that take any parameters (i.e. can't be made a get accessor), meaning I'm exposing my functional dependencies into the View.
Likewise with personChanged()
not firing on @bindable person
if I modify person.firstName
. This is fundamental stuff.
Are we obsessing with optimisation to the point that we're making development difficult? How about a convention of "make it possible now, and improve performance later"?
I have simple fix to get dot notation supported in computedFrom annotation: https://github.com/aurelia/binding/pull/345 That fix will also enable property name syntax validation and aurelia will throw an error if syntax is not correct.
Good stuff @jdanyow !
Hi,
Can you provide an example of the use of computedFrom with dot property ?
I tried computedFrom("MyObject.property") but this does not work.
@ggrimbert this is the same for me config.computedFrom(['baseContent.ValidFromTime', 'baseContent.ValidToDate', 'baseContent.ValidToTime'])
FYI, aurelia bindings can't bind normally to Date objects. In my case I have to use and bind to value property of Date object:
if (!Date.prototype.value) {
Object.defineProperty(Date.prototype, 'value', {
get: function() {
return this.valueOf()
}
});
}
But that stills fall back to dirty checking to value property of Date object.
PS: I developed aurelia-stats to track digest loop for aurelia, will release soon.
@ClareBear85, if you have Date objects as properties and you would like to bind them with @computedFrom()
then I would like to suggest you https://github.com/sormy/aurelia-date-observer
Is there a way to make this work with dynamic values?
If I use
get changed() {
return this.initialValue == this.model.data[this.model.key];
}
The value is dirty checked, however if I use $.initialValue == model.data[model.key]; ? 'bar' : 'foo'}
, the value is not dirty checked.
${changed ? 'bar' : 'foo'}
Are you trying to avoid this? If you want to avoid dirty checking you need to intercept every dependency's property setter. That was the original idea behind allowing @computedFrom on functions.
${changed ? 'bar' : 'foo'}
is what I want but this results in dirty checking. I can't use @computedFrom because my values used to compute are dynamic.
@Nepoxx Did you try this with aurelia-computed?
@Nepoxx Also, see this issue: https://github.com/aurelia/binding/issues/147
What about @computedFrom('initialValue', 'model.data', 'model.key')
?
@khenderick That works but it'll be triggered whenever model.data
changes, not when model.data[model.key]
changes. It's not a perfect solution but it's definitely an improvement :)
It is my understanding and my experience that
computedFrom
does not work with paths, like:@computedFrom('user.firstName')
.I think that this is a very hard limitation that should be lifted:
computedFrom
.My use case is a computed property that creates an array from complex computations and lookups from other arrays. Definitively not something you want to poll and it is too complex for the auto-observation plugin. On the other hand its dependency is trivial to express as it is a single, nested property.