FirebaseExtended / angularfire

AngularJS bindings for Firebase
MIT License
2.73k stars 631 forks source link

$firebaseArray.$extend - wait for all infos before return #647

Closed JiiBay closed 9 years ago

JiiBay commented 9 years ago

Hello guys ! And thanks for this awesome library :)

I'm working on an app and made a heavy use of extending firebaseArray's $$added method to load more informations when a record is loaded (for exemple, for a message, load the author's name and avatar URL from users object).

Something like

$$added: function (snap) {
    // call the super method
    var record = $firebaseArray.prototype.$$added.call(this, snap);

    $firebaseObject(userIdRef).$loaded().then(function (userData) {
        record.userData = userData;
    });

    // return the modified record
    return record;
}

My issue is, the messages list shows up and there is a slight delay before the user's name and avatar appears. I perfectly understand why - these informations are loaded and until the promise returns they are not available, while the main query is already returned.

I find it being an awful user experience, and I want to fix this. Looking for an answer, I found this : https://github.com/firebase/firebase-util/blob/master/src/NormalizedCollection/README.md

But it's apparently not recommended to use in production, and my app is about to go live soon.

So, any idea ?

Cheers !

jwngr commented 9 years ago

You can actually make $$added() return a promise which will get rid of that delay by making the query wait until the promise is fulfilled. This feature was added in AngularFire 1.1.0. You code would look like this (excuse typos, I have not tested this code):

$$added: function (snap) {
  // call the super method
  var record = $firebaseArray.prototype.$$added.call(this, snap);

  return new Promise(function(resolve, reject) {
    $firebaseObject(userIdRef).$loaded().then(function (userData) {
      record.userData = userData;
      resolve(record);
    }).catch(reject);
  });
}

Hope that helps!

JiiBay commented 8 years ago

Hello @jwngr ! Thank you so much for that, it worked like a charm ! I'm using it happily ever since.

Sadly, I ran into a big issue two days ago and can't get over it - and I'm not sure what I'm doing wrong. Narrowed it down to this :

var extendData = Sync.fbObjExt({
    $$updated: function (snap) {
        var record = this;
        // call the super
        Sync.fbObjSimple().prototype.$$updated.apply(this, arguments);
        return new Promise(function(resolve, reject) {
            // Call any promise and resolve after it returned data
            manageMediaObj.getUserData(mediaId).then(function () {
                record.$test = 22222;
                resolve(record);
            });
        });
    }
});

var mediaData = new extendData(fbRef("medias/" + mediaId));
mediaData.$loaded().then(function () {
    // Added data, like $test, will return "undefined" but are available immediately after
    console.log(mediaData, mediaData.$test);
});

Basically, the data added in the promise before the resolve is not immediately available. It feels like the basic data of the query, located at the original location, is immediately available, and the data added using promises is coming after that - negating the effect of the used promise to wait for full data before returning !

Anything you could do to help me solve this ?

Thank you very much again.

katowulf commented 8 years ago

What is Sync.fbObjExt()? We can't help without knowing what your code is doing. Please start by simplifying your examples to the minimal code needed to repro, and all the code needed to repro.

Note that all of these could be simplified by using constructs like the following (no need for new Promise() and the resolve()/reject() logic since you can just chain and return the new value):

    $$updated: function (snap) {
        var record = this;
        // call the super
        Sync.fbObjSimple().prototype.$$updated.apply(this, arguments);
        return manageMediaObj.getUserData(mediaId).then(function () {
           record.$test = 22222;
           return record;
       });
    }
JiiBay commented 8 years ago

Sorry, forgot that. Sync.fbObjExt() is simply $firebaseObject.$extend(). Same, Sync.fbObjSimple() is for firebaseObject().

I should have simplified like you did, sorry for the lack of mcve. It's a try I did in the middle of my madness, in the last two days.

Simplified like youy did, the output is exactly the same.

JiiBay commented 8 years ago

Hello again @katowulf , @jwngr !

I made a codepen to demonstrate the issue. I can't be sure if I discovered a bug or if I'm simply messing with my head again, but there you go :

http://codepen.io/anon/pen/dYZmMz

Simply open the console. As you can see, the data.$test shows "undefined", that means the console.log occurs before this variable is available. But if you open the object "data", logged just before, you can see $test with its value - since the console shows the object at the time its expanded, not printed.

I tried to make it as much mcve as possible this time. If you need anything more please ask.

JiiBay commented 8 years ago

Since this one is closed, should I open a new issue to report this bug ?

katowulf commented 8 years ago

I've answered this question somewhere else, but I can't find the response. $loaded does not depend on or reference $$updated. If you think this through, it's fairly clear why: $loaded is a one time promise resolved on initial data load. $$updated is a recurring event triggered each time data changes.

If your goal is to log the data, stop trying to guess when it will show up and just view it in the DOM with something like {{data | json}}, letting AngularFire and Angular work out the timing issues.

If your goal is to manipulate the data, do so in the $$updated method or the constructor.

JiiBay commented 8 years ago

Interesting. Usually you're right, that's enough. But what if I need to manipulate data of the view's $scope (that has to be done in the controller) using data from the extended function ? I need to wait for a promise to return on the controller side before manipulating. The only promise I see here that seems usable is the $loaded, but as you explained it's not the solution (I understand why now, thank you).

One solution would be to use a custom promise that resolves inside of the extended function, when all data is available. I did that, it works great. Another one I see would be to $watchCollection the data set, and when the wanted data is available, use it as needed and unbind the watcher. If you can confirm me that this is the best options I have, that would be great - and I hope this topic can help people, someday :)

Have a nice day !

katowulf commented 8 years ago

Without knowing the relationship between the components, I can't make any recommendations. Generally, depending on internal state of another class/object is bad. There are always exceptions.