Speed up loading the initial data, by using the 'value' callback.#213
Speed up loading the initial data, by using the 'value' callback.#213dickp wants to merge 4 commits intoFirebaseExtended:masterfrom
Conversation
When 'value' has finished, then start listening for 'child_added'; in the 'child_added' callback, ignore any additions that we already have.
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
|
I signed it! |
| if (query) { | ||
| query.on('child_added', this.__onFirebaseChildAdded, this.__onError, this); | ||
| /* The 'value' event doesn't order the data, so we have to | ||
| * revert to the slow path if ordering is requested |
There was a problem hiding this comment.
The DataSnapshot provided to the 'value' event is in fact ordered. But .val() returns it as a plain JS Object, which is not ordered. To preserve ordering, you can iterate the DataSnapshot using the .forEach() method.
| } | ||
|
|
||
| /* Now start listening for changes */ | ||
| this.query.on('child_added', this.__onFirebaseChildAdded, this.__onError, this); |
There was a problem hiding this comment.
You can attach this right away but ignore the events you get before the first 'value' event.
There was a problem hiding this comment.
@mikelehen I forked @dickp 's branch and asked for a PR on his side that is connected to here, using what you have suggested
Signed-off-by: Toni-Jan Keith Monserrat <tonijanmonserrat@gmail.com>
Signed-off-by: Toni-Jan Keith Monserrat <tonijanmonserrat@gmail.com>
|
I signed it! Again! |
There was a problem hiding this comment.
This looks good to me.
Personally, I'd attach the child_added event right away (with the rest of the events) and add a gotInitialEvents flag, which is set to true on the first 'value' event... and onFirebaseChildAdded would just ignore any events until the flag was set to true. (and then you wouldn't need the __map[key] check in onFirebaseChildAdded anymore...) But there's probably no material difference.
…value callback. Implements suggestion made by @mikelehen on FirebaseExtended#212
|
Changes look good to me (thanks)! |
|
I think @ecgtheow is the one that needs to sign the CLA. |
|
@justweb1 I have been trying to sign the CLA as @ecgtheow, but someone in the CLA office keeps deciding I don't have the authority and voiding the signature. I've pointed them at the UK government's companies registration site, but it's like talking to a brick wall. I'll try signing it yet again now. |
|
Aaaand they 'voided' it again. I give up. |
|
@dickp would you be willing to re-open this in a new PR as a single commit from just your personal account? That should make the CLA bot happy (I hope). |
|
I don't think it's the CLA bot. Every time I try to sign it, I get an email back that says: "Google Contributor License Agreement for Ecgtheow Ltd has been voided for the following reason: Please have this document signed by an authorized executive. ..." No "please show us some documentation" (I tried to), not that that would help as emails to cla-submissions@google.com seem to go straight to /dev/null anyway. It's a bank holiday weekend here and I'm making it a long one. I might have some time next week to look at this. |
|
@dickp if you squash all if the changes into one commit using your username instead if the company you will be able to submit a new OR with no issue. |
|
Hey, |
|
Supplanted by new PR with functioning CLA. |
7 participants
When 'value' has finished, then start listening for 'child_added'; in
the 'child_added' callback, ignore any additions that we already have.
All polymerfire tests pass.
Fixes #212