From ed9c8b591eaef11f856b7d641991851e093b7493 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natan=20S=C4=85gol?= Date: Mon, 23 Oct 2017 11:20:55 +0000 Subject: [PATCH 01/13] Increase efficiency of Array updates made by FirestoreMixin. Currently, when one document in a collection changes, an entirely new array is assigned to a property. It does not work well with dom-repeats, iron-lists etc. This change lays the groundwork for a more efficient solution. --- firebase-firestore-mixin.html | 90 +++++++++++++++++++++++++---------- 1 file changed, 64 insertions(+), 26 deletions(-) diff --git a/firebase-firestore-mixin.html b/firebase-firestore-mixin.html index 370a1e5..3e51ddf 100644 --- a/firebase-firestore-mixin.html +++ b/firebase-firestore-mixin.html @@ -6,10 +6,6 @@ (context => { const MATCH_RE = /\{[a-z0-9_]+\}/gi; - const TRANSFORMS = { - doc: function(snap) { return iDoc(snap); }, - collection: function(snap) { return snap.empty ? [] : snap.docs.map(doc => iDoc(doc)) } - } function parsePath(path) { let result; @@ -27,14 +23,14 @@ return whole; } - function collect(what, which) { - let res = {}; - while (what) { - res = Object.assign({}, what[which], res); // Respect prototype priority - what = Object.getPrototypeOf(what); - } - return res; - }; + function collect(what, which) { + let res = {}; + while (what) { + res = Object.assign({}, what[which], res); // Respect prototype priority + what = Object.getPrototypeOf(what); + } + return res; + }; function iDoc(snap) { if (snap.exists) { @@ -153,18 +149,17 @@ this._firestoreProps[name] = config; - // Create a method observer that will be called every time a templatized or observed property changes - let args = config.props.concat(config.observes).join(','); - if (args.length) { args = ',' + args; } - this._createMethodObserver(`_firestoreUpdateBinding('${type}','${name}'${args})`); - if (!config.props.length && !config.observes.length) { - this._firestoreUpdateBinding(type,name); + this._firestoreUpdateBinding(name, type); + } else { + // Create a method observer that will be called every time a templatized or observed property changes + let args = config.props.concat(config.observes).join(','); + this._createMethodObserver(`_firestoreUpdateBinding('${name}', '${type}', ${args})`); } } - _firestoreUpdateBinding(type, name) { - this._firestoreUnlisten(name); + _firestoreUpdateBinding(name, type) { + this._firestoreUnlisten(name, type); const config = this._firestoreProps[name]; const propArgs = Array.prototype.slice.call(arguments, 2, config.props.length + 2).filter(arg => arg); @@ -178,10 +173,7 @@ } const collPath = stitch(config.parts, propArgs); - const assigner = snap => { - this[name] = TRANSFORMS[type](snap); - this[name + 'Ready'] = true; - } + const assigner = this._firestoreAssigner(name, type); let ref = this.db[type](collPath); this[name + 'Ref'] = ref; @@ -199,13 +191,59 @@ } } - _firestoreUnlisten(name) { + _firestoreUnlisten(name, type) { if (this._firestoreListeners[name]) { this._firestoreListeners[name](); delete this._firestoreListeners[name]; } + if (type === 'collection') { + this[name] = []; + } + } + + _firestoreAssigner(name, type) { + if (type === 'doc') { + return (snap) => this._firestoreAssignDocument(name, snap); + } else if (type === 'collection') { + return (snap) => this._firestoreAssignCollection(name, snap); + } else { + throw new Error('Unknown listener type.') + } + } + + _firestoreAssignDocument(name, snap) { + this[name] = iDoc(snap); + this[name + 'Ready'] = true; + } + + _firestoreAssignCollection(name, snap) { + const allDocumentsChanged = snap.docs.length === snap.docChanges.length; + if (Array.isArray(this[name]) && !allDocumentsChanged) { + snap.docChanges.forEach((change) => { + switch (change.type) { + case 'added': + this.splice(name, change.newIndex, 0, iDoc(change.doc)); + break; + case 'removed': + this.splice(name, change.oldIndex, 1); + break; + case 'modified': + if (change.oldIndex === change.newIndex) { + this.splice(name, change.oldIndex, 1, iDoc(change.doc)); + } else { + this.splice(name, change.oldIndex, 1); + this.splice(name, change.newIndex, 0, iDoc(change.doc)); + } + break; + default: + throw new Error(`Unhandled document change: ${change.type}.`) + } + }); + } else { + this[name] = snap.docs.map(iDoc); + } } } } })(window); - \ No newline at end of file + From d0a4870d98e9861c0e89192174d3c5c928e2c4c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natan=20S=C4=85gol?= Date: Wed, 25 Oct 2017 16:57:03 +0000 Subject: [PATCH 02/13] Fix an issue with initail binding in FirestoreMixin. This change fixes an issue which occurs when observed/bound properties are defined prior to a call of `connectedCallback`. Since _firestoreUpdateBinding method handles null and undefined values correctly, the `if` statement can be safely removed. --- firebase-firestore-mixin.html | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/firebase-firestore-mixin.html b/firebase-firestore-mixin.html index b4e9a19..423aaac 100644 --- a/firebase-firestore-mixin.html +++ b/firebase-firestore-mixin.html @@ -162,9 +162,7 @@ if (args.length) { args = ',' + args; } this._createMethodObserver(`_firestoreUpdateBinding('${type}','${name}'${args})`); - if (!config.props.length && !config.observes.length) { - this._firestoreUpdateBinding(type,name); - } + this._firestoreUpdateBinding(type,name); } _firestoreUpdateBinding(type, name) { From e284cc42bd31e2a0adcb95c824708600a9cee702 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natan=20S=C4=85gol?= Date: Wed, 25 Oct 2017 17:13:25 +0000 Subject: [PATCH 03/13] FirestoreMixin: throw an error on binding to properties with invalid type. --- firebase-firestore-mixin.html | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/firebase-firestore-mixin.html b/firebase-firestore-mixin.html index b4e9a19..893d3a8 100644 --- a/firebase-firestore-mixin.html +++ b/firebase-firestore-mixin.html @@ -131,6 +131,15 @@ */ Polymer.FirestoreMixin = parent => { return class extends parent { + static _ensurePropertyTypeCorrectness(prop) { + if (prop.doc !== undefined && prop.type !== Object) { + throw new Error(`FirestoreMixin's \`doc\` can only be used with properties of type \`Object\`.`); + } + if (prop.collection !== undefined && prop.type !== Array) { + throw new Error(`FirestoreMixin's \`collection\` can only be used with properties of type \`Array\`.`); + } + } + constructor() { super(); this._firestoreProps = {}; @@ -140,6 +149,8 @@ connectedCallback() { const props = collect(this.constructor, 'properties'); + Object.values(props).forEach(this.constructor._ensurePropertyTypeCorrectness); + for (let name in props) { if (props[name].doc) { this._firestoreBind('doc', props[name].doc, name, props[name].live, props[name].observes); From 28210c0da8070e667695aa49cc6d267530c98bee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natan=20S=C4=85gol?= Date: Wed, 25 Oct 2017 17:23:01 +0000 Subject: [PATCH 04/13] FirestoreMixin: ensure that it was not previously applied to the same class. --- firebase-firestore-mixin.html | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/firebase-firestore-mixin.html b/firebase-firestore-mixin.html index b4e9a19..1cc0b3c 100644 --- a/firebase-firestore-mixin.html +++ b/firebase-firestore-mixin.html @@ -5,6 +5,7 @@ } { + const TOKEN = Symbol('polymerfire-firestore-mixin'); const PROPERTY_BINDING_REGEXP = /{([^{]+)}/g; const TRANSFORMS = { doc: function(snap) { return iDoc(snap); }, @@ -133,6 +134,12 @@ return class extends parent { constructor() { super(); + + if (this[TOKEN]) { + throw new Error(`FirestoreMixin must not be applied more than once to the same class.`); + } + + this[TOKEN] = true; this._firestoreProps = {}; this._firestoreListeners = {}; this.db = this.constructor.db || firebase.firestore(); From 3644a853916e566b5db5ca6ba2a0423bb2e664e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natan=20S=C4=85gol?= Date: Wed, 25 Oct 2017 17:24:58 +0000 Subject: [PATCH 05/13] FirestoreMixin: add missing semicolons. --- firebase-firestore-mixin.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/firebase-firestore-mixin.html b/firebase-firestore-mixin.html index 645c8b4..4039c8f 100644 --- a/firebase-firestore-mixin.html +++ b/firebase-firestore-mixin.html @@ -211,7 +211,7 @@ } else if (type === 'collection') { return (snap) => this._firestoreAssignCollection(name, snap); } else { - throw new Error('Unknown listener type.') + throw new Error('Unknown listener type.'); } } @@ -240,7 +240,7 @@ } break; default: - throw new Error(`Unhandled document change: ${change.type}.`) + throw new Error(`Unhandled document change: ${change.type}.`); } }); } else { From 5982940df56c120ed380930f3b2836005215baf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natan=20S=C4=85gol?= Date: Thu, 26 Oct 2017 16:55:06 +0000 Subject: [PATCH 06/13] Add query property to _firestoreProps in FirestoreMixin. --- firebase-firestore-mixin.html | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/firebase-firestore-mixin.html b/firebase-firestore-mixin.html index ac0258c..01ba8fe 100644 --- a/firebase-firestore-mixin.html +++ b/firebase-firestore-mixin.html @@ -141,38 +141,42 @@ connectedCallback() { const props = collect(this.constructor, 'properties'); for (let name in props) { - if (props[name].doc) { - this._firestoreBind('doc', props[name].doc, name, props[name].live, props[name].observes); - } else if (props[name].collection) { - this._firestoreBind('collection', props[name].collection, name, props[name].live, props[name].observes); + const options = props[name]; + if (options.doc || options.collection) { + this._firestoreBind(name, options); } } super.connectedCallback(); } - _firestoreBind(type, path, name, live = false, observes = []) { - const config = parsePath(path); - config.observes = observes; - config.live = live; + _firestoreBind(name, options) { + const defaults = { + live: false, + observes: [], + } + const parsedPath = parsePath(options.doc || options.collection); + const config = Object.assign({}, defaults, options, parsedPath); + config.type = config.doc ? 'doc' : 'collection'; this._firestoreProps[name] = config; // Create a method observer that will be called every time a templatized or observed property changes let args = config.props.concat(config.observes).join(','); if (args.length) { args = ',' + args; } - this._createMethodObserver(`_firestoreUpdateBinding('${type}','${name}'${args})`); + this._createMethodObserver(`_firestoreUpdateBinding('${name}'${args})`); if (!config.props.length && !config.observes.length) { - this._firestoreUpdateBinding(type,name); + this._firestoreUpdateBinding(name); } } - _firestoreUpdateBinding(type, name) { + _firestoreUpdateBinding(name, ...args) { this._firestoreUnlisten(name); const config = this._firestoreProps[name]; - const propArgs = Array.prototype.slice.call(arguments, 2, config.props.length + 2).filter(arg => arg); - const observesArgs = Array.prototype.slice.call(arguments, config.props.length + 2).filter(arg => arg); + const isDefined = (x) => x !== undefined; + const propArgs = args.slice(0, config.props.length).filter(isDefined); + const observesArgs = args.slice(config.props.length).filter(isDefined); if (propArgs.length < config.props.length || observesArgs.length < config.observes.length) { this[name] = null; @@ -183,11 +187,11 @@ const collPath = stitch(config.literals, propArgs); const assigner = snap => { - this[name] = TRANSFORMS[type](snap); + this[name] = TRANSFORMS[config.type](snap); this[name + 'Ready'] = true; } - let ref = this.db[type](collPath); + let ref = this.db[config.type](collPath); this[name + 'Ref'] = ref; this[name + 'Ready'] = false; From 18ac794ad4b05b31ae6e5abd9616f00dc6514857 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natan=20S=C4=85gol?= Date: Wed, 3 Jan 2018 15:25:49 +0000 Subject: [PATCH 07/13] Assign a null value to a document when it's listener is being deleted. --- firebase-firestore-mixin.html | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/firebase-firestore-mixin.html b/firebase-firestore-mixin.html index 4039c8f..a5476fd 100644 --- a/firebase-firestore-mixin.html +++ b/firebase-firestore-mixin.html @@ -200,9 +200,8 @@ this._firestoreListeners[name](); delete this._firestoreListeners[name]; } - if (type === 'collection') { - this[name] = []; - } + + this[name] = type === 'collection' ? [] : null; } _firestoreAssigner(name, type) { From ef9c2d022cc9e9c183477e4af2f6ee4915224cd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natan=20S=C4=85gol?= Date: Wed, 3 Jan 2018 15:29:18 +0000 Subject: [PATCH 08/13] Strict & more readable comparison in _firestoreAssignCollection method in FirestoreMixin. --- firebase-firestore-mixin.html | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/firebase-firestore-mixin.html b/firebase-firestore-mixin.html index a5476fd..8036f33 100644 --- a/firebase-firestore-mixin.html +++ b/firebase-firestore-mixin.html @@ -220,8 +220,9 @@ } _firestoreAssignCollection(name, snap) { + const propertyValueIsArray = Array.isArray(this[name]) const allDocumentsChanged = snap.docs.length === snap.docChanges.length; - if (Array.isArray(this[name]) && !allDocumentsChanged) { + if (propertyValueIsArray && allDocumentsChanged === false) { snap.docChanges.forEach((change) => { switch (change.type) { case 'added': From 35caf3a0ccb0fe8c23434d1583a876ea2245591e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natan=20S=C4=85gol?= Date: Wed, 3 Jan 2018 15:54:44 +0000 Subject: [PATCH 09/13] Apply DRY rule and improve a name of _ensurePropertyTypeCorrectness in FirestoreMixin. --- firebase-firestore-mixin.html | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/firebase-firestore-mixin.html b/firebase-firestore-mixin.html index 893d3a8..08a7819 100644 --- a/firebase-firestore-mixin.html +++ b/firebase-firestore-mixin.html @@ -131,13 +131,18 @@ */ Polymer.FirestoreMixin = parent => { return class extends parent { - static _ensurePropertyTypeCorrectness(prop) { - if (prop.doc !== undefined && prop.type !== Object) { - throw new Error(`FirestoreMixin's \`doc\` can only be used with properties of type \`Object\`.`); - } - if (prop.collection !== undefined && prop.type !== Array) { - throw new Error(`FirestoreMixin's \`collection\` can only be used with properties of type \`Array\`.`); + static _assertPropertyTypeCorrectness(prop) { + const errorMessage = (listenerType, propertyType) => + `FirestoreMixin's ${listenerType} can only be used with properties ` + + `of type ${propertyType}.`; + const assert = (listenerType, propertyType) => { + if (prop[listenerType] !== undefined && prop.type !== propertyType) { + throw new Error(errorMessage(listenerType, propertyType.name)); + } } + + assert('doc', Object); + assert('collection', Array); } constructor() { @@ -149,7 +154,7 @@ connectedCallback() { const props = collect(this.constructor, 'properties'); - Object.values(props).forEach(this.constructor._ensurePropertyTypeCorrectness); + Object.values(props).forEach(this.constructor._assertPropertyTypeCorrectness); for (let name in props) { if (props[name].doc) { From 5e5245edc3276e5500a091d66f0be280d65c0efd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natan=20S=C4=85gol?= Date: Wed, 3 Jan 2018 16:07:15 +0000 Subject: [PATCH 10/13] Handle applying FirestoreMixin multiple times to the same element. --- firebase-firestore-mixin.html | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/firebase-firestore-mixin.html b/firebase-firestore-mixin.html index 1cc0b3c..97c8b89 100644 --- a/firebase-firestore-mixin.html +++ b/firebase-firestore-mixin.html @@ -5,7 +5,9 @@ } { - const TOKEN = Symbol('polymerfire-firestore-mixin'); + const CONSTRUCTOR_TOKEN = Symbol('polymerfire-firestore-mixin-constructor'); + const CONNECTED_CALLBACK_TOKEN = + Symbol('polymerfire-firestore-mixin-connected-callback'); const PROPERTY_BINDING_REGEXP = /{([^{]+)}/g; const TRANSFORMS = { doc: function(snap) { return iDoc(snap); }, @@ -135,17 +137,22 @@ constructor() { super(); - if (this[TOKEN]) { - throw new Error(`FirestoreMixin must not be applied more than once to the same class.`); + if (this[CONSTRUCTOR_TOKEN] === true) { + return; } + this[CONSTRUCTOR_TOKEN] = true; - this[TOKEN] = true; this._firestoreProps = {}; this._firestoreListeners = {}; this.db = this.constructor.db || firebase.firestore(); } connectedCallback() { + if (this[CONNECTED_CALLBACK_TOKEN] === true) { + return; + } + this[CONNECTED_CALLBACK_TOKEN] = true; + const props = collect(this.constructor, 'properties'); for (let name in props) { if (props[name].doc) { From fd12a0da1c1e3d270cd70ce0c49361eb28a2d09b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natan=20S=C4=85gol?= Date: Wed, 3 Jan 2018 16:09:12 +0000 Subject: [PATCH 11/13] Simplify syntax in TRANSFORMS in FirestoreMixin. --- firebase-firestore-mixin.html | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/firebase-firestore-mixin.html b/firebase-firestore-mixin.html index 97c8b89..bc58a46 100644 --- a/firebase-firestore-mixin.html +++ b/firebase-firestore-mixin.html @@ -9,10 +9,6 @@ const CONNECTED_CALLBACK_TOKEN = Symbol('polymerfire-firestore-mixin-connected-callback'); const PROPERTY_BINDING_REGEXP = /{([^{]+)}/g; - const TRANSFORMS = { - doc: function(snap) { return iDoc(snap); }, - collection: function(snap) { return snap.empty ? [] : snap.docs.map(doc => iDoc(doc)) } - } const isOdd = (x) => x & 1 === 1; @@ -51,6 +47,11 @@ } } + const TRANSFORMS = { + doc: iDoc, + collection: (snap) => snap.empty ? [] : snap.docs.map(iDoc), + } + /** * This mixin provides bindings to documents and collections in a * Cloud Firestore database through special property declarations. From 28f3e7da9324d7aecf79856ddc298fa33e0bb5e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natan=20S=C4=85gol?= Date: Thu, 4 Jan 2018 17:14:05 +0000 Subject: [PATCH 12/13] Fix an issue with initial binding of properties in FirestoreMixin. --- firebase-firestore-mixin.html | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/firebase-firestore-mixin.html b/firebase-firestore-mixin.html index 1ef21a9..5d39589 100644 --- a/firebase-firestore-mixin.html +++ b/firebase-firestore-mixin.html @@ -169,7 +169,9 @@ this[CONNECTED_CALLBACK_TOKEN] = true; const props = collect(this.constructor, 'properties'); - Object.values(props).forEach(this.constructor._assertPropertyTypeCorrectness); + Object + .values(props) + .forEach(this.constructor._assertPropertyTypeCorrectness); for (let name in props) { const options = props[name]; @@ -192,16 +194,16 @@ this._firestoreProps[name] = config; - if (config.props.length > 0 || config.observes.length > 0) { + const args = config.props.concat(config.observes); + if (args.length > 0) { // Create a method observer that will be called every time // a templatized or observed property changes - const args = config.props.concat(config.observes).join(','); const observer = - `_firestoreUpdateBinding('${name}', ${args})` + `_firestoreUpdateBinding('${name}', ${args.join(',')})` this._createMethodObserver(observer); } - this._firestoreUpdateBinding(name); + this._firestoreUpdateBinding(name, ...args.map(x => this[x])); } _firestoreUpdateBinding(name, ...args) { From cbd73709b053e2d3db47337114efb0f6f20e19df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natan=20S=C4=85gol?= Date: Mon, 8 Jan 2018 13:34:00 +0000 Subject: [PATCH 13/13] Fix 'propertyReady' in FirestoreMixin. --- firebase-firestore-mixin.html | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/firebase-firestore-mixin.html b/firebase-firestore-mixin.html index 5d39589..3b58764 100644 --- a/firebase-firestore-mixin.html +++ b/firebase-firestore-mixin.html @@ -252,15 +252,17 @@ } _firestoreAssigner(name, type) { + const makeAssigner = (assigner) => (snap) => { + assigner.call(this, name, snap); + this[name + 'Ready'] = true; + } if (type === 'doc') { - return (snap) => this._firestoreAssignDocument(name, snap); + return makeAssigner(this._firestoreAssignDocument); } else if (type === 'collection') { - return (snap) => this._firestoreAssignCollection(name, snap); + return makeAssigner(this._firestoreAssignCollection); } else { throw new Error('Unknown listener type.'); } - - this[name + 'Ready'] = true; } _firestoreAssignDocument(name, snap) {