Description
The Problem
Suppose you are writing code for some DS.Model using computedPromise
IssueThread = DS.Model.extend
comments: DS.hasMany "comments", async: true
okToMerge: computedPromise "comments.[]", ->
@get("comments")
.then (comments) ->
comments.find (comment) ->
comment.get("speaker") is "rwjblue" &&
comment.get("comment") is "lgtm"Aside from the infinite loop notification problem as mentioned in #132 (comment)
There is another issue regarding different instances of an object sharing the same result state.
The Reason
From the code at:
https://github.com/cibernox/ember-cpm/blob/master/addon/macros/computed-promise.js#L41
We see that both pendingPromise and result are declared outside the computed property, which means if you have something like:
issue1 = @store.find "issue-thread", 1
issue2 = @store.find "issue-thread", 2
issue1.get("okToMerge") # Call to start the promise
waitUntilResolve()
issue2.get("okToMerge") === issue1.get("okToMerge")That is, even though we have two distinct instances of the IssueThread, they both share the result state... which is definitely a bug
My Humble Suggestion
Please consider just putting pendingPromise and result directly into the object instance, i.e.:
return computed(...dependentKeys, function(key) {
if (!this[key + "PendingPromise"]) {
const promise = fn.call(this);
this[key + "PendingPromise"] = true;
Ember.RSVP.resolve(promise)
.then((promiseResult) => {
this[key + "Result"] = promiseResult;
pendingPromise = false;
this.notifyPropertyChange(key);
});
}
return this[key + "Result"];
});Yes, this feels dirty, but this way, state is properly tied to each object instance, not to some closure shared by all instances of a factory. And we could always put in some assert to ensure the additional property names aren't colliding, and tell off the user if they are.