diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 0000000..66864db --- /dev/null +++ b/.travis.yml @@ -0,0 +1,28 @@ +language: node_js +sudo: false +matrix: + include: + - node_js: stable + script: xvfb-run -a wct --simpleOutput -l firefox -l chrome + addons: + firefox: latest + apt: + sources: + - google-chrome + packages: + - google-chrome-stable + - node_js: node + script: + - | + if [ "${TRAVIS_PULL_REQUEST}" = "false" ]; then + wct --simpleOutput -s 'Windows 10/microsoftedge' -s 'Windows 8.1/internet explorer@11' -s 'Windows 7/internet explorer@10' -s 'OS X 10.10/safari@8' -s 'OS X 10.9/safari@7' + fi +before_script: +- npm install bower +- npm install web-component-tester +- export PATH=$PWD/node_modules/.bin:$PATH +- bower install +env: + global: + - secure: dlr2l5btcMEnaJI/VzBDIKhcJTUgnrQ6AN/S+qbPFqsckokV5SCt3k0f0Z22CBhBXJi2qVrLDX05+wZ4FkVkLLdGOf6zR0sSNvXubzvUu8oKaZczo2B8EAs+OjsuvaZWPYTabAOGvyAwuBzfZaYDlXbk6Dlb/51hjzSl2D5/WbY= + - secure: dFreaGtRTAwcFtSZ57ktyiDejTkJ7vI9TVbJZ0Yd9Adp/4mINOOgHDFiVDl5kDCy9xx21vCRel+IrGkOdRllk+OWxO8Ga1OQF4EzchvUsxzngDwi3I0P1+uNuzjn8MyEvE4HYPwDZ0mDYzMLJS9b1GlcpMTvJvE2Sg3ly8h4wYc= diff --git a/firebase-collection.html b/firebase-collection.html index 64de6f6..612fc83 100644 --- a/firebase-collection.html +++ b/firebase-collection.html @@ -233,6 +233,11 @@

[[dinosaur.__firebaseKey__]]

'firebase-child-moved': '_onFirebaseChildMoved', }, + created: function() { + this._pendingSplices = []; + this._lastLocallyAddedIndex = null; + }, + /** * Add an item to the document referenced at `location`. A key associated * with the item will be created by Firebase, and can be accessed via the @@ -261,7 +266,10 @@

[[dinosaur.__firebaseKey__]]

*/ remove: function(data) { if (data == null || data.__firebaseKey__ == null) { - this._error('Failed to remove unknown value:', data); + // NOTE(cdata): This might be better as an error message in the + // console, but `Polymer.Base._error` throws, which we don't want + // to happen in this case. + this._warn('Refusing to remove unknown value:', data); return; } @@ -287,49 +295,82 @@

[[dinosaur.__firebaseKey__]]

* located at `location`. */ removeByKey: function(key) { + if (!this.query) { + this._error('Cannot remove items before query has been initialized.'); + return; + } + this.query.ref().child(key).remove(); }, - _applyLocalDataChanges: function(change) { + _localDataChanged: function(change) { var pathParts = change.path.split('.'); - var firebaseKey; - var key; - var value; + // We don't care about self-changes, and we don't respond directly to + // length changes: if (pathParts.length < 2 || pathParts[1] === 'length') { return; } - if (pathParts[1] === 'splices') { - this._applySplicesToRemoteData(change.value.indexSplices); + // Handle splices via the adoption process. `indexSplices` is known to + // sometimes be null, so guard against that. + if (pathParts[1] === 'splices' && change.value.indexSplices != null) { + this._adoptSplices(change.value.indexSplices); return; } - key = pathParts[1]; - value = Polymer.Collection.get(change.base).getItem(key); + // Otherwise, the change is happening to a sub-path of the array. + this._applySubPathChange(change); + }, + + _applySubPathChange: function(change) { + var key = change.path.split('.')[1]; + var value = Polymer.Collection.get(change.base).getItem(key); + var firebaseKey = value.__firebaseKey__; - // Temporarily remove the client-only `__firebaseKey__` property: - firebaseKey = value.__firebaseKey__; + // We don't want to accidentally reflect `__firebaseKey__` in the + // remote data, so we remove it temporarily. `null` values will be + // discarded by Firebase, so `delete` is not necessary: value.__firebaseKey__ = null; - this.query.ref().child(firebaseKey).set(value); - value.__firebaseKey__ = firebaseKey; }, - _applySplicesToRemoteData: function(splices) { - this._log('splices', splices); - splices.forEach(function(splice) { - var added = splice.object.slice(splice.index, splice.index + splice.addedCount); + _adoptSplices: function(splices) { + this._pendingSplices = this._pendingSplices.concat(splices); - splice.removed.forEach(function(removed) { - this.remove(removed); + // We can afford apply removals synchronously, so we do that first + // and save the `added` operations for the `debounce` below. + this._applyLocalDataChange(function() { + splices.forEach(function(splice) { + splice.removed.forEach(function(removed) { + this.remove(removed); + }, this); }, this); + }); - added.forEach(function(added) { - this.add(added); - }, this); - }, this); + // We async until the next turn. The reason we want to do this is + // that splicing within a splice handler will break the data binding + // system in some places. This is referred to as the "re-entrancy" + // problem. See polymer/polymer#2491. + this.debounce('_adoptSplices', function() { + this._applyLocalDataChange(function() { + var splices = this._pendingSplices; + + this._pendingSplices = []; + + splices.forEach(function(splice) { + var added = splice.object.slice(splice.index, splice.index + splice.addedCount); + + added.forEach(function(added, index) { + this._lastLocallyAddedIndex = splice.index + index; + this.add(added); + }, this); + }, this); + + this._lastLocallyAddedIndex = null; + }); + }); }, _computeQuery: function(location, limitToFirst, limitToLast, orderByMethodName, startAt, endAt, equalTo) { @@ -339,6 +380,8 @@

[[dinosaur.__firebaseKey__]]

return; } + this._log('Recomputing query.', arguments); + query = new Firebase(location); if (orderByMethodName) { @@ -460,17 +503,56 @@

[[dinosaur.__firebaseKey__]]

_onFirebaseChildAdded: function(event) { this._applyRemoteDataChange(function() { var value = this._valueFromSnapshot(event.detail.childSnapshot); + var key = value.__firebaseKey__; var previousValueKey = event.detail.previousChildName; var index = previousValueKey != null ? this.data.indexOf(this._valueMap[previousValueKey]) + 1 : 0; + var lastLocallyAddedValue; this._valueMap[value.__firebaseKey__] = value; - this.splice('data', index, 0, value); + // NOTE(cdata): The rationale for this conditional dance around the + // last locally added index (since you will inevitably be wondering + // why we do it): + // There may be a "locally" added value which was spliced in. If + // there is, it may be in the "wrong" place in the array. This is + // due to the fact that Firebase may be applying a sort to the + // data, so we won't know the correct index for the locally added + // value until the `child_added` event is fired. + // Once we get the `child_added` event, we can check to see if the + // locally added value is in the right place. If it is, we just + // `set` it to the Firebase-provided value. If it is not, then + // we grab the original value, splice in the Firebase-provided + // value in the right place, and then (importantly: at the end) + // find the locally-added value again (since its index may have + // changed by splicing-in Firebase's value) and splice it out of the + // array. + if (this._lastLocallyAddedIndex === index) { + this.set(['data', index], value); + } else { + if (this._lastLocallyAddedIndex != null) { + lastLocallyAddedValue = this.data[this._lastLocallyAddedIndex]; + } + + this.splice('data', index, 0, value); + + if (this._lastLocallyAddedIndex != null) { + this.splice('data', this.data.indexOf(lastLocallyAddedValue), 1); + } + } }); }, _onFirebaseChildRemoved: function(event) { + if (this._receivingLocalChanges) { + this._valueMap[event.detail.oldChildSnapshot.key()] = null; + // NOTE(cdata): If we are receiving local changes, that means that + // the splices have already been performed and items are already + // removed from the local data representation. No need to remove + // them again. + return; + } + this._applyRemoteDataChange(function() { var key = event.detail.oldChildSnapshot.key(); var value = this._valueMap[key]; diff --git a/firebase-document.html b/firebase-document.html index de4739f..ff2010d 100644 --- a/firebase-document.html +++ b/firebase-document.html @@ -64,7 +64,7 @@ 'firebase-value': '_onFirebaseValue' }, - _applyLocalDataChanges: function(change) { + _localDataChanged: function(change) { var pathFragments = change.path.split('.'); if (pathFragments.length === 1) { @@ -98,8 +98,10 @@ }, _setRemoteDocumentChild: function(key, value) { - this._log('Setting child "' + key + '" to', value); - this.query.child(key).set(value); + this.debounce('set-' + key, function() { + this._log('Setting child "' + key + '" to', value); + this.query.child(key).set(value); + }); }, _removeRemoteDocumentChild: function(key) { diff --git a/firebase-query-behavior.html b/firebase-query-behavior.html index bfba352..7c67243 100644 --- a/firebase-query-behavior.html +++ b/firebase-query-behavior.html @@ -40,11 +40,6 @@ type: Boolean, value: false, reflectToAttribute: true - }, - - _receivingRemoteChanges: { - type: Boolean, - value: false } }, @@ -52,6 +47,11 @@ '_dataChanged(data.*)' ], + created: function() { + this._receivingLocalChanges = false; + this._receivingRemoteChanges = false; + }, + get dataAsObject() { if (Array.isArray(this.data)) { return this.data.reduce(function(object, value, index) { @@ -69,27 +69,29 @@ this.location = ''; }, - _applyLocalDataChanges: function(changes) { + _localDataChanged: function(changes) { // Virtual.. }, + _applyLocalDataChange: function(applyChange) { + this._receivingLocalChanges = true; + applyChange.call(this); + this._receivingLocalChanges = false; + }, + _applyRemoteDataChange: function(applyChange) { - if (this._applyingLocalDataChanges) { - return; - } this._receivingRemoteChanges = true; applyChange.call(this); this._receivingRemoteChanges = false; }, _dataChanged: function(changes) { - if (this._receivingRemoteChanges) { + if (this._receivingRemoteChanges || + this._receivingLocalChanges) { return; } - this._applyingLocalDataChanges = true; - this._applyLocalDataChanges(changes); - this._applyingLocalDataChanges = false; + this._localDataChanged(changes); }, _queryChanged: function(query, oldQuery) { @@ -181,9 +183,15 @@ } }, + _warn: function() { + if (this.log) { + Polymer.Base._warn(this._logf.apply(this, arguments)); + } + }, + _error: function() { if (this.log) { - console.error.apply(console, arguments); + Polymer.Base._error(this._logf.apply(this, arguments)); } } }; diff --git a/test/firebase-collection.html b/test/firebase-collection.html index 8aecae4..3866864 100644 --- a/test/firebase-collection.html +++ b/test/firebase-collection.html @@ -17,7 +17,6 @@ - @@ -25,42 +24,7 @@ - - - - - - - - - - @@ -75,7 +39,6 @@
[[item.value]]
@@ -85,217 +48,228 @@ diff --git a/test/test-helpers.html b/test/test-helpers.html index bfd28b0..735137b 100644 --- a/test/test-helpers.html +++ b/test/test-helpers.html @@ -1,14 +1,82 @@ +