Derived props not calculated before prop change events are fired?

[callumlocke]

Say I've got a prop foo, and a derived prop bar that depends on foo.

If I listen to change:foo, and inside the listener I read the value of bar, I find that the value of bar is old, i.e. it has not updated to reflect the new foo value. But if I use setTimeout(..., 0) inside the listener to delay checking the value of bar till the next tick, then the value of bar is now up to date.

Is this expected behaviour? I have a vague memory that ampersand-state is designed so that this shouldn't happen - that it always works out all derived props before firing any listeners. But my recent experience contradicts this. Or is it more likely something wrong with my code?

[latentflip]

Hmm, looking at the relevant code (https://github.com/AmpersandJS/ampersand-state/blob/master/ampersand-state.js#L405-L438) it looks like the order is that prop change events will fire before their related derived changes. Which kind of makes sense since all change events are synchronously fired, and we'd have to have the props themselves changing before we run the derived property updates.

I'm not exactly sure how we could fix this from a quick look at the code :/

[callumlocke]

Well this is weird... you say that, but I just tried making a minimal test case and it seems the derived prop DOES get recalculated before the prop listener fires:

var State = require('ampersand-state');

var Person = State.extend({
  props: {
    firstName: 'string',
    lastName: 'string',
  },

  derived: {
    fullName: {
      deps: ['firstName', 'lastName'],
      fn: function () {
        return this.firstName + ' ' + this.lastName;
      }
    }
  }
});


var me = new Person({firstName: 'John', lastName: 'Smith'});

me.on('change:firstName', function () {
  console.log('full name is now ' + me.fullName);
});

me.firstName = 'James'; // "full name is now James Smith"

(The above works on requirebin.com)

But I've had bugs where this is not the case, and where I've had to use setTimeout to wait for the next tick. Weird.

[latentflip]

Ah, okay, I see the issue.

The reason it's not failing in that situation is because the derived prop isn't in the cache, so when you call it for the first time from in that callback, it's running the function on the fly because it's not cached.

If you add console.log('full name before', me.fullName); before me.firstName = 'James' that'll force it to cache it before the change, and then you'll see that it doesn't get updated :/

[callumlocke]

Ah ok that makes sense.

So do you agree it's a problem?

Is there any way the behaviour could be changed, by 'settling' all props (normal/session/derived) and THEN checking which ones have changed and firing all the change events?

[latentflip]

I sort of agree it's a problem :) I guess the first question is: if you want to read a derived prop from a change callback, why don't you listen to the derived prop instead?

It's conceivable that we could try to batch updates like that, though my gut feeling is that it would be a pretty significant change: currently derived props are effectively just callbacks bound to the change events of their deps, so that would all have to change somehow to allow derived props to still work properly. I'm not sure of the implications their either for child model events etc

Philip Roberts
&yet

On 10 Jul 2015, at 13:54, Callum Locke notifications@github.com wrote:

Ah ok that makes sense.

So do you agree it's a problem?

Is there any way the behaviour could be changed, by 'settling' all props (normal/session/derived) and THEN checking which ones have changed and firing all the change events?

—
Reply to this email directly or view it on GitHub.

[callumlocke]

if you want to read a derived prop from a change callback, why don't you listen to the derived prop instead?

yep I see what you mean, I'll try to give a convincing example...

Say you've got a prop weight, which is stored as a number of grams. But then you've got a couple of derived props too, weightImperial and weightMetric, which get you formatted strings like '45 kg' or '100 lb'. And you have a view that displays the weight (in both kilograms and pounds), and you want to re-render the whole view whenever the weight changes. My instinct is to listen to change:weight. It just happens that in rendering the view, I'm going to be reading the derived props.

I guess you could argue that I should be watching only the exact properties I'm going to actually use (e.g. view.listenTo(model, 'change:weightKilograms change:weightPounds', function () {...}) but I would say in this situation it's not as intuitive as simply listening to change:weight, because conceptually the change of weight is the reason why I need to re-render the view, and the rest is formatting details.

It feels weird that there's ever a time when I can read the derived props and they're temporarily 'wrong'.

currently derived props are effectively just callbacks bound to the change events of their deps

Could this maybe work if there were two types of changes: unsettledChange for internal use (derived props can listen to this to monitor their deps), and another change which works in the new way I described (fired only after cascade of prop changes has actually settled, for all props that have ended up with a different value than what they had at the beginning of the cascade)...?

[callumlocke]

I keep coming up against this problem in different places, I'm surprised it hasn't been raised before. Do you think this behaviour should be changed, given my extended example above? Or am I approaching derived props in the wrong way?

[latentflip]

@HenrikJoreteg thoughts? I'm not against the idea in principle, just unsure of the scale of work required/who has time to do it

[HenrikJoreteg]

Not against this in principle either. I seem to recall there was some challenge that led it to be implemented this way. But I could certainly be wrong.