feature request: async derived properties

[joeybaker]

It would be great if it was possible to derive properties in an async way. The fn could accept a callback that called if the function returns undefined.

Use cases: debouncing derived properties, ajax calls, etc…

[HenrikJoreteg]

Can you elaborate a bit more? Perhaps include some code for how you wish it worked?

[joeybaker]

You bet:

…
derived: {
  // current behavior
  mySyncProperty: {
    deps: ['aProp']
    , fn: function deriveMySyncProperty(){
      return this.aProp + 1 // mySyncProperty now set
    }
  }
  // proposed async behavior
  , myAsyncProperty: {
    deps: ['aProp']
    , fn: _.debounce(function deriveMyAsyncProperty(done){
      expensiveComputation(this.aProp, function(err, res){
        if (err) done(err)
        else done(null, res) // myAsyncProperty now set to res
      })
    }, 500)
  }
}
…

[lukekarrys]

To chime in, I've found myself wanting to this for ajax calls:

drivingDistance: {
    deps: ['latitude', 'longitude'],
    fn: function (done) {
        fetchDrivingDistance(this.latitude + ',' + this.longitude, function (err, result) {
            // done could be error first
            done(err, result);
            // or just accept a value
            done(err ? 'Could not get driving distance' : 'Driving distance is ' + result);
        });
    }
}

This isn't too hard to achieve otherwise with some listeners and a session property, but I do like the idea of the syntax above.

[HenrikJoreteg]

Typically, with normal derived properties, everything happens synchronously. So we fire a change:property then when all the changes are done firing, including derived once, we fire a single change event. How would you expect this to event?

You could fire a late change:derivedAsync but would it also fire a change again at that time?

[joeybaker]

I would vote "yes" for a second change event. As you say, it's expected behavior that every time the state changes, we get a "change" event. That wouldn't be a problem for me.

[lukekarrys]

I think they should fire change events in order to have consistent behavior with other derived properties, but then you'll have an extra change event for each async derived prop which might lead to degrading perf.

I like the idea and the proposed syntax, but I'm closer to +0 on this since it could have some quirks like that.

[HenrikJoreteg]

You can achieve this same type of behavior by setting the result of your async function as a session property.

In some ways I think doing that would be preferable, albeit a bit more verbose. But doing so means we can also cleanly handle how to deal with the fact that derived properties can derive from each other.

It also allows you to easily manage your own cache of the results, since the result is simply set as session property and control how often your expensive operation occurs.

I'm hesitant to introduce this additional complexity to derived properties, they're not so simple as is :)

I'd say something like this would suffice:

session: {
    drivingDistance: 'string'
},
calculateDrivingDistance: function () {
    var self = this;
    fetchDrivingDistance(this.latitude + ',' + this.longitude, function (err, result) {
            self.drivingDistance = result;
    });
}

[HenrikJoreteg]

i think i'm going to close this for now. Thanks for the suggestion, though @joeybaker. Re-open if you disagree.

[joeybaker]

@HenrikJoreteg the only downside with the current approach is that you have to manually add change even listeners to run the calculateDrivingDistance method. The nice part about derived properties is that all of that happens for you. I think the benefit of the API I laid out is that it's totally backward compatible and solves a problem that at least two of us have had in a simple way. IMHO

[HenrikJoreteg]

Yeah, that's fair. I'll leave it open for discussion.

[HenrikJoreteg]

How would it deal with the fact that you'd still get model.drivingDistance === undefined until the callback was done. Because we have to return something synchronously when referenced as a property.

[joeybaker]

IMHO, leaving model.drivingDistance as undefined is okay. If we were to hook things up manually, that's what would happen.

[chesles]

Could you combine the session & derived approaches to avoid manually setting up listeners? e.g.

session: {
    drivingDistance: 'string'
},
derived: {
    _drivingDistance: {
        deps: ['latitude', 'longitude'],
        fn: function () {
            var self = this
            fetchDrivingDistance(this.latitude + ',' + this.longitude, function (err, result) {
                if (result) {
                  self.drivingDistance = result;
                }
            });
        }
    }
}

Yes, it's kindof ugly that there's an extra _drivingDistance derived property in there that's not really used, and you have to specify the session property as well, but something like this would work, right?

Assuming it does, it would be cool to see an async option for derived properties, where passing async: true along with deps and fn sets up a session property and listeners automatically, so the example would become just:

derived: {
    drivingDistance: {
        deps: ['latitude', 'longitude'],
        async: true,
        fn: function () {
            var self = this
            fetchDrivingDistance(this.latitude + ',' + this.longitude, function (err, result) {
                // or maybe use the done(result) callback approach to make sure the right property is set
                if (result) {
                  self.drivingDistance = result;
                }
            });
        }
    }
}

Or maybe async properties like this could be their own thing, similar to derived properties but labeled async to avoid confusion?

[joeybaker]

I like @chesles suggestion of a async: true option. That would make the async derived properties explicit.

[chesles]

This is actually pretty trivial to support: https://github.com/chesles/ampersand-state/compare/117-async-derived-properties

[HenrikJoreteg]

I think fetching additional or related data is a common enough use case that might simplify doing model relationships. My main concern here is the edges surrounding this. For example every read of the derived prop before successful completion would cause a re-running of that async function running unless we also did that book keeping.

[chesles]

You're right - had a few more edges to cover here to do this right. The initial implementation would call the async function multiple times without the done function on uncached reads - only changes to the dependencies worked correctly.

Did a little refactor so there's an _updateDerivedProperty function that handles all uncached reads - async or not - and does the necessary bookkeeping. The way the bookkeeping works means that if done isn't called the async function won't get called again- a potential gotcha.

[chesles]

Added a few more tests and tweaked the bookkeeping so that:

• uncached reads won't initiate more than 1 simultaneous async call
• changes that trigger an update to an async property can happen inside a change event handler

Accessing the property while the async function runs just returns the previous value (or undefined), which I don't think is a problem. We could set the value to undefined at the beginning of an update - but I don't see any advantage to doing that.

Check out my branch if you're interested in this feature and let me know if you see any issues. If you like it I can submit a PR.

[flipside]

This looks good and seems fairly straight forward to merge with my PR for dataTypes and other enhancements to derived properties in #116 (type, default, and init).

Also just to share a really hacky way of solving this problem using custom events:

derived: {
    user: {
        deps: ['userId','fetchedUser'],
        fn: function () {
            var user = app.users.get(this.userId);
            if (!user) app.users.fetch(this.userId, this.trigger.bind(this, 'change:fetchedUser'));
            else return user;
        }
    }
}

[ben-eb]

I'd like this feature too. 👍

[ianwremmel]

I'd really like to see this feature, but I've got a concern about the implementation in https://github.com/chesles/ampersand-state/commit/1688650daa164e9328b0220c999ed829fcdb74ae. By making sure the update function is only called once, don't you end up with a result that might not match its trigger?

In other words, let's say I change a property weightInPounds which triggers an update of the dependent property weightInKilograms to fire its async method superIneffecientWeightConverter. Then, if I change weightInPounds again before weightInKilograms has been updated, weightInKilograms will be based on the first value.

[timwis]

Hey guys, did anything come of this? I was faced with this issue yesterday and ended up returning a promise from the derived fn, leaving two options for interacting with them via bindings. Overall it feels a bit hacky though and I'd love some guidance.

[wraithgar]

I don't think we're ready to throw promises into the mix here. So far nobody has come up w/ a method that they feel is ready enough to make a PR for, it appears.

[cdaringe]

agreed.

How would it deal with the fact that you'd still get model.drivingDistance === undefined until the callback was done

a key question that would still require bottoming out on. not that it's pertinent, but i can see folks abusing this, embedding application logic within the state obj itself. im not saying it's so, but there's something fishy feeling (to me*) about tying your application and model logic into the state container itself. just food for thought.

[DimitryDushkin]

So? What about async derived props? Maybe use promises as returns?