From 4badff8489f653f8ab0d63e5a83ebe2132b472d3 Mon Sep 17 00:00:00 2001 From: Toni-Jan Keith Monserrat Date: Thu, 21 Sep 2017 23:11:31 +0800 Subject: [PATCH 1/5] on-value first predictable before adding other events Signed-off-by: Toni-Jan Keith Monserrat --- firebase-query.html | 89 ++++++++++++++++++++++++--------------------- 1 file changed, 48 insertions(+), 41 deletions(-) diff --git a/firebase-query.html b/firebase-query.html index c4e9cce..eafd434 100644 --- a/firebase-query.html +++ b/firebase-query.html @@ -292,30 +292,47 @@ }); } - if (query) { - if(this._onOnce){ // remove handlers before adding again. Otherwise we get data multiplying - query.off('child_added', this.__onFirebaseChildAdded, this); - query.off('child_removed', this.__onFirebaseChildRemoved, this); - query.off('child_changed', this.__onFirebaseChildChanged, this); - query.off('child_moved', this.__onFirebaseChildMoved, this); + query.once('value').then(function(snapshot) { + if (snapshot.hasChildren()) { + var data = []; + snapshot.forEach(function(childSnapshot) { + var key = childSnapshot.key; + var value = this.__valueWithKey(key, childSnapshot.val()) + this.__map[key] = value; + data.push(value) + }.bind(this)) + + this.set('data', data); } - this._onOnce = true; - - /* We want the value callback to batch load the initial data, - * then let child_added take over for subsequent changes. - */ - this.__initialLoadDone = false; - - /* Don't use query.once() as we need to be able to cancel - * the callback if the query changes - */ - query.on('value', this.__onFirebaseValue, this.__onError, this); query.on('child_added', this.__onFirebaseChildAdded, this.__onError, this); query.on('child_removed', this.__onFirebaseChildRemoved, this.__onError, this); query.on('child_changed', this.__onFirebaseChildChanged, this.__onError, this); query.on('child_moved', this.__onFirebaseChildMoved, this.__onError, this); - } + }.bind(this)) + + + // if (query) { + // if(this._onOnce){ // remove handlers before adding again. Otherwise we get data multiplying + // query.off('child_added', this.__onFirebaseChildAdded, this); + // query.off('child_removed', this.__onFirebaseChildRemoved, this); + // query.off('child_changed', this.__onFirebaseChildChanged, this); + // query.off('child_moved', this.__onFirebaseChildMoved, this); + // } + + // this._onOnce = true; + + // /* We want the value callback to batch load the initial data, + // * then let child_added take over for subsequent changes. + // */ + // this.__initialLoadDone = false; + + // /* Don't use query.once() as we need to be able to cancel + // * the callback if the query changes + // */ + // query.on('value', this.__onFirebaseValue, this.__onError, this); + + // } }, __indexFromKey: function(key) { @@ -329,38 +346,28 @@ return -1; }, - __onFirebaseValue: function(snapshot) { - if (snapshot.hasChildren()) { - var data = []; - snapshot.forEach(function(childSnapshot) { - var key = childSnapshot.key; - var value = this.__valueWithKey(key, childSnapshot.val()) - this.__map[key] = value; - data.push(value) - }.bind(this)) + // __onFirebaseValue: function(snapshot) { - this.set('data', data); - } - /* Now let child_added deal with subsequent changes */ - this.query.off('value', this.__onFirebaseValue, this); - this.__initialLoadDone = true; - }, + // /* Now let child_added deal with subsequent changes */ + // // this.query.off('value', this.__onFirebaseValue, this); + // // this.__initialLoadDone = true; + // }, __onFirebaseChildAdded: function(snapshot, previousChildKey) { var key = snapshot.key; - if (this.__initialLoadDone) { - var value = snapshot.val(); - var previousChildIndex = this.__indexFromKey(previousChildKey); + // if (this.__initialLoadDone) { + var value = snapshot.val(); + var previousChildIndex = this.__indexFromKey(previousChildKey); - this._log('Firebase child_added:', key, value); + this._log('Firebase child_added:', key, value); - value = this.__snapshotToValue(snapshot); + value = this.__snapshotToValue(snapshot); - this.__map[key] = value; - this.splice('data', previousChildIndex + 1, 0, value); - } + this.__map[key] = value; + this.splice('data', previousChildIndex + 1, 0, value); + // } }, __onFirebaseChildRemoved: function(snapshot) { From 50eac1b64226818e6c520cfc361430d8a274f491 Mon Sep 17 00:00:00 2001 From: Toni-Jan Keith Monserrat Date: Fri, 22 Sep 2017 00:09:25 +0800 Subject: [PATCH 2/5] added a fix Signed-off-by: Toni-Jan Keith Monserrat --- firebase-query.html | 85 ++++++++++++++++++++++----------------------- 1 file changed, 41 insertions(+), 44 deletions(-) diff --git a/firebase-query.html b/firebase-query.html index eafd434..d4fe14b 100644 --- a/firebase-query.html +++ b/firebase-query.html @@ -292,47 +292,51 @@ }); } - query.once('value').then(function(snapshot) { - if (snapshot.hasChildren()) { - var data = []; - snapshot.forEach(function(childSnapshot) { - var key = childSnapshot.key; - var value = this.__valueWithKey(key, childSnapshot.val()) - this.__map[key] = value; - data.push(value) - }.bind(this)) - - this.set('data', data); + // this allows us to just call the addition of event listeners only once. + // __queryChanged is being called thrice when firebase-query is created + // 1 - 2. query property computed (null, undefined) + // 3. when attached is called (this.query, this.query) -> this is where we add stuff + + // hopefully if reference changed, this still fire + + if (query && oldQuery) { + if(this._onOnce){ // remove handlers before adding again. Otherwise we get data multiplying + query.off('child_added', this.__onFirebaseChildAdded, this); + query.off('child_removed', this.__onFirebaseChildRemoved, this); + query.off('child_changed', this.__onFirebaseChildChanged, this); + query.off('child_moved', this.__onFirebaseChildMoved, this); } - query.on('child_added', this.__onFirebaseChildAdded, this.__onError, this); - query.on('child_removed', this.__onFirebaseChildRemoved, this.__onError, this); - query.on('child_changed', this.__onFirebaseChildChanged, this.__onError, this); - query.on('child_moved', this.__onFirebaseChildMoved, this.__onError, this); - }.bind(this)) - + this._onOnce = true; - // if (query) { - // if(this._onOnce){ // remove handlers before adding again. Otherwise we get data multiplying - // query.off('child_added', this.__onFirebaseChildAdded, this); - // query.off('child_removed', this.__onFirebaseChildRemoved, this); - // query.off('child_changed', this.__onFirebaseChildChanged, this); - // query.off('child_moved', this.__onFirebaseChildMoved, this); - // } + // does the on-value first + query.once('value') + .then(function(snapshot) { + if (snapshot.hasChildren()) { + var data = []; + snapshot.forEach(function(childSnapshot) { + var key = childSnapshot.key; + var value = this.__valueWithKey(key, childSnapshot.val()) - // this._onOnce = true; + this.__map[key] = value; + data.push(value) + }.bind(this)) - // /* We want the value callback to batch load the initial data, - // * then let child_added take over for subsequent changes. - // */ - // this.__initialLoadDone = false; + this.set('data', data); + } + }.bind(this)) - // /* Don't use query.once() as we need to be able to cancel - // * the callback if the query changes - // */ - // query.on('value', this.__onFirebaseValue, this.__onError, this); + // catches the error above but still sets the listeners below + .catch(this.__onError.bind(this)) - // } + // here comes the per child event listeners + .then(function() { + query.on('child_added', this.__onFirebaseChildAdded, this.__onError, this); + query.on('child_removed', this.__onFirebaseChildRemoved, this.__onError, this); + query.on('child_changed', this.__onFirebaseChildChanged, this.__onError, this); + query.on('child_moved', this.__onFirebaseChildMoved, this.__onError, this); + }.bind(this)) + } }, __indexFromKey: function(key) { @@ -346,18 +350,11 @@ return -1; }, - // __onFirebaseValue: function(snapshot) { - - - // /* Now let child_added deal with subsequent changes */ - // // this.query.off('value', this.__onFirebaseValue, this); - // // this.__initialLoadDone = true; - // }, - __onFirebaseChildAdded: function(snapshot, previousChildKey) { var key = snapshot.key; - // if (this.__initialLoadDone) { + if (this.__map[key]) return + var value = snapshot.val(); var previousChildIndex = this.__indexFromKey(previousChildKey); @@ -367,10 +364,10 @@ this.__map[key] = value; this.splice('data', previousChildIndex + 1, 0, value); - // } }, __onFirebaseChildRemoved: function(snapshot) { + var key = snapshot.key; var value = this.__map[key]; From 5f4df9f99dc4ebe470425ad8921428fd9fe4cf69 Mon Sep 17 00:00:00 2001 From: Toni-Jan Keith Monserrat Date: Fri, 22 Sep 2017 00:12:47 +0800 Subject: [PATCH 3/5] added some comments Signed-off-by: Toni-Jan Keith Monserrat --- firebase-query.html | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/firebase-query.html b/firebase-query.html index d4fe14b..9d954d6 100644 --- a/firebase-query.html +++ b/firebase-query.html @@ -353,7 +353,8 @@ __onFirebaseChildAdded: function(snapshot, previousChildKey) { var key = snapshot.key; - if (this.__map[key]) return + // check if the key-value pair already exists + if (this.__indexFromKey(key) < 0) return var value = snapshot.val(); var previousChildIndex = this.__indexFromKey(previousChildKey); @@ -377,7 +378,11 @@ this.__map[key] = null; this.async(function() { this.syncToMemory(function() { - this.splice('data', this.__indexFromKey(key), 1); + // this only catches already deleted keys (which will return -1) + // at least it will not delete the last element from the array (this.splice('data', -1, 1)) + if (this.__indexFromKey(key) >= 0) { + this.splice('data', this.__indexFromKey(key), 1); + } }); }); } From 80ee04a32397270b1fbaa78291dd338c6438413e Mon Sep 17 00:00:00 2001 From: Toni-Jan Keith Monserrat Date: Fri, 22 Sep 2017 21:24:07 +0800 Subject: [PATCH 4/5] fixed bug 1 Signed-off-by: Toni-Jan Keith Monserrat --- firebase-query.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/firebase-query.html b/firebase-query.html index 9d954d6..cc53924 100644 --- a/firebase-query.html +++ b/firebase-query.html @@ -354,7 +354,7 @@ var key = snapshot.key; // check if the key-value pair already exists - if (this.__indexFromKey(key) < 0) return + if (this.__indexFromKey(key) >= 0) return var value = snapshot.val(); var previousChildIndex = this.__indexFromKey(previousChildKey); From 4535ddd1878d27b1c06ad12923ea968d0d00170a Mon Sep 17 00:00:00 2001 From: Toni-Jan Keith Monserrat Date: Sat, 23 Sep 2017 10:03:57 +0800 Subject: [PATCH 5/5] made on-value as an on event again Signed-off-by: Toni-Jan Keith Monserrat --- firebase-query.html | 66 ++++++++++++++++++++++++--------------------- 1 file changed, 36 insertions(+), 30 deletions(-) diff --git a/firebase-query.html b/firebase-query.html index cc53924..d503f83 100644 --- a/firebase-query.html +++ b/firebase-query.html @@ -295,11 +295,10 @@ // this allows us to just call the addition of event listeners only once. // __queryChanged is being called thrice when firebase-query is created // 1 - 2. query property computed (null, undefined) - // 3. when attached is called (this.query, this.query) -> this is where we add stuff + // 3. when attached is called (this.query, this.query) + // need help to fix this so that this function is only called once - // hopefully if reference changed, this still fire - - if (query && oldQuery) { + if (query) { if(this._onOnce){ // remove handlers before adding again. Otherwise we get data multiplying query.off('child_added', this.__onFirebaseChildAdded, this); query.off('child_removed', this.__onFirebaseChildRemoved, this); @@ -308,34 +307,11 @@ } this._onOnce = true; + this._query = query // does the on-value first - query.once('value') - .then(function(snapshot) { - if (snapshot.hasChildren()) { - var data = []; - snapshot.forEach(function(childSnapshot) { - var key = childSnapshot.key; - var value = this.__valueWithKey(key, childSnapshot.val()) - - this.__map[key] = value; - data.push(value) - }.bind(this)) - - this.set('data', data); - } - }.bind(this)) - - // catches the error above but still sets the listeners below - .catch(this.__onError.bind(this)) - - // here comes the per child event listeners - .then(function() { - query.on('child_added', this.__onFirebaseChildAdded, this.__onError, this); - query.on('child_removed', this.__onFirebaseChildRemoved, this.__onError, this); - query.on('child_changed', this.__onFirebaseChildChanged, this.__onError, this); - query.on('child_moved', this.__onFirebaseChildMoved, this.__onError, this); - }.bind(this)) + query.off('value', this.__onFirebaseValue, this) + query.on('value', this.__onFirebaseValue, this.__onError, this) } }, @@ -350,6 +326,36 @@ return -1; }, + __onFirebaseValue: function(snapshot) { + if (snapshot.hasChildren()) { + var data = []; + snapshot.forEach(function(childSnapshot) { + var key = childSnapshot.key; + var value = this.__valueWithKey(key, childSnapshot.val()) + + this.__map[key] = value; + data.push(value) + }.bind(this)) + + this.set('data', data); + } + + const query = this.query + + query.off('value', this.__onFirebaseValue, this) + + // ensures that all events are called once + query.off('child_added', this.__onFirebaseChildAdded, this); + query.off('child_removed', this.__onFirebaseChildRemoved, this); + query.off('child_changed', this.__onFirebaseChildChanged, this); + query.off('child_moved', this.__onFirebaseChildMoved, this); + + query.on('child_added', this.__onFirebaseChildAdded, this.__onError, this); + query.on('child_removed', this.__onFirebaseChildRemoved, this.__onError, this); + query.on('child_changed', this.__onFirebaseChildChanged, this.__onError, this); + query.on('child_moved', this.__onFirebaseChildMoved, this.__onError, this); + }, + __onFirebaseChildAdded: function(snapshot, previousChildKey) { var key = snapshot.key;