computedPromise has issues with proper state management

[foxnewsnetwork]

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 https://github.com/cibernox/ember-cpm/pull/132#issuecomment-170152050

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.

[kellyselden]

@foxnewsnetwork Can you try v3.0.0? computedPromise got a major refactor, and may have fixed your issue in the process.