feat(refs): keeps data of refs when updating the document

this prevents unsubscribing and subscribing again to ref when a document is partially updated. It
works by using old data to populate data with existing values

Author: posva

src/index.js

@@ -1,4 +1,4 @@
-import { createSnapshot, extractRefs, callOnceWithArg, deepGetSplit } from './utils'
+import { createSnapshot, extractRefs, callOnceWithArg, walkGet, walkSet } from './utils'
 
 // NOTE not convinced by the naming of subscribeToRefs and subscribeToDocument
 // first one is calling the other on every ref and subscribeToDocument may call
@@ -7,7 +7,7 @@ function subscribeToRefs ({
   subs,
   refs,
   target,
-  key,
+  path,
   data,
   depth,
   resolve
@@ -30,22 +30,24 @@ function subscribeToRefs ({
     const sub = subs[refKey]
     const ref = refs[refKey]
 
-    // documents are fully replaced so it's necessary to bind things again
-    // TODO it would be nice to prevent unnecessary unbind
-    if (sub) sub.unbind()
+    if (sub) {
+      if (sub.path !== ref.path) sub.unbind()
+      // if has already be bound and as we always walk the objects, it will work
+      else return
+    }
 
     // maybe wrap the unbind function to call unbind on every child
-    const [innerObj, innerKey] = deepGetSplit(target[key], refKey)
-    if (!innerObj) {
-      console.log('=== ERROR ===')
-      console.log(data, refKey, key, innerObj, innerKey)
-      console.log('===')
-    }
+    // const [innerObj, innerKey] = deepGetSplit(target[key], refKey)
+    // if (!innerObj) {
+    //   console.log('=== ERROR ===')
+    //   console.log(data, refKey, key, innerObj, innerKey)
+    //   console.log('===')
+    // }
     subs[refKey] = {
       unbind: subscribeToDocument({
         ref,
-        target: innerObj,
-        key: innerKey,
+        target,
+        path: `${path}.${refKey}`,
         depth,
         resolve
       }),
@@ -62,6 +64,7 @@ function bindCollection ({
   reject
 }) {
   // TODO wait to get all data
+  // XXX support pathes? nested.obj.list
   const array = vm[key] = []
   const originalResolve = resolve
   let isResolved
@@ -82,24 +85,24 @@ function bindCollection ({
         refs,
         subs,
         target: array,
-        key: newIndex,
+        path: newIndex,
         depth: 0,
         resolve: resolve.bind(null, doc)
       })
     },
     modified: ({ oldIndex, newIndex, doc }) => {
       const subs = arraySubs.splice(oldIndex, 1)[0]
       arraySubs.splice(newIndex, 0, subs)
-      array.splice(oldIndex, 1)
+      const oldData = array.splice(oldIndex, 1)[0]
       const snapshot = createSnapshot(doc)
-      const [data, refs] = extractRefs(snapshot)
+      const [data, refs] = extractRefs(snapshot, oldData)
       array.splice(newIndex, 0, data)
       subscribeToRefs({
         data,
         refs,
         subs,
         target: array,
-        key: newIndex,
+        path: newIndex,
         depth: 0,
         resolve
       })
@@ -131,6 +134,7 @@ function bindCollection ({
       resolve = ({ id }) => {
         if (id in validDocs) {
           if (++count >= expectedItems) {
+            // TODO use array instead?
             originalResolve(vm[key])
             // reset resolve to noop
             resolve = _ => {}
@@ -157,21 +161,23 @@ function bindCollection ({
   }
 }
 
-function updateDataFromDocumentSnapshot ({ snapshot, target, key, subs, depth = 0, resolve }) {
-  const [data, refs] = extractRefs(snapshot)
-  target[key] = data
+function updateDataFromDocumentSnapshot ({ snapshot, target, path, subs, depth = 0, resolve }) {
+  const [data, refs] = extractRefs(snapshot, walkGet(target, path))
+  // TODO use walkSet?
+  walkSet(target, path, data)
+  // target[key] = data
   subscribeToRefs({
     data,
     subs,
     refs,
     target,
-    key,
+    path,
     depth,
     resolve
   })
 }
 
-function subscribeToDocument ({ ref, target, key, depth, resolve }) {
+function subscribeToDocument ({ ref, target, path, depth, resolve }) {
   const subs = Object.create(null)
   // console.log('subDoc(1)', key)
   // const lol = key
@@ -181,13 +187,15 @@ function subscribeToDocument ({ ref, target, key, depth, resolve }) {
       updateDataFromDocumentSnapshot({
         snapshot: createSnapshot(doc),
         target,
-        key,
+        path,
         subs,
         depth,
         resolve
       })
     } else {
-      target[key] = null
+      // TODO use deep set
+      walkSet(target, path, null)
+      // target[path] = null
       resolve()
     }
   })
@@ -214,13 +222,14 @@ function bindDocument ({
   const subs = Object.create(null)
   // bind here the function so it can be resolved anywhere
   // this is specially useful for refs
+  // TODO use walkGet?
   resolve = callOnceWithArg(resolve, () => vm[key])
   const unbind = document.onSnapshot(doc => {
     if (doc.exists) {
       updateDataFromDocumentSnapshot({
         snapshot: createSnapshot(doc),
         target: vm,
-        key,
+        path: key,
         subs,
         resolve
       })

src/utils.js

@@ -9,7 +9,9 @@ function isObject (o) {
   return o && typeof o === 'object'
 }
 
-export function extractRefs (doc, path = '', result = [{}, {}]) {
+export function extractRefs (doc, oldDoc, path = '', result = [{}, {}]) {
+  // must be set here because walkGet can return null or undefined
+  oldDoc = oldDoc || {}
   const idDescriptor = Object.getOwnPropertyDescriptor(doc, 'id')
   if (idDescriptor && !idDescriptor.enumerable) {
     Object.defineProperty(result[0], 'id', idDescriptor)
@@ -18,16 +20,16 @@ export function extractRefs (doc, path = '', result = [{}, {}]) {
     const ref = doc[key]
     // if it's a ref
     if (typeof ref.isEqual === 'function') {
-      tot[0][key] = ref.path
+      tot[0][key] = oldDoc[key] || ref.path
       // TODO handle subpathes?
       tot[1][path + key] = ref
     } else if (Array.isArray(ref)) {
       // TODO handle array
       tot[0][key] = Array(ref.length).fill(null)
-      extractRefs(ref, path + key + '.', [tot[0][key], tot[1]])
+      extractRefs(ref, oldDoc[key] || {}, path + key + '.', [tot[0][key], tot[1]])
     } else if (isObject(ref)) {
       tot[0][key] = {}
-      extractRefs(ref, path + key + '.', [tot[0][key], tot[1]])
+      extractRefs(ref, oldDoc[key] || {}, path + key + '.', [tot[0][key], tot[1]])
     } else {
       tot[0][key] = ref
     }
@@ -45,15 +47,14 @@ export function callOnceWithArg (fn, argFn) {
   }
 }
 
-export function deepGetSplit (obj, path) {
-  const keys = path.split('.')
-  // We want the containing obj and the last key
-  // key is the one we're going to bind to
+export function walkGet (obj, path) {
+  return path.split('.').reduce((target, key) => target[key], obj)
+}
+
+export function walkSet (obj, path, value) {
+  // path can be a number
+  const keys = ('' + path).split('.')
   const key = keys.pop()
-  return [
-    keys.reduce((res, key) => {
-      return res[key]
-    }, obj),
-    key
-  ]
+  const target = keys.reduce((target, key) => target[key], obj)
+  return (target[key] = value)
 }

test/refs-collections.spec.js

@@ -138,12 +138,10 @@ test('updates when modifying an item', async () => {
   ])
 })
 
-test('removes the DocumentReference when modifying an item', async () => {
+test('keeps old data of refs when modifying an item', async () => {
   await vm.$bind('items', collection)
   await first.update({ newThing: true })
 
-  expect(typeof vm.items[0].ref).toBe('string')
-  await delay(5)
   expect(vm.items[0]).toEqual({
     ref: { isA: true },
     newThing: true

test/refs-documents.spec.js

@@ -154,6 +154,23 @@ test('unbinds previously bound document when overwriting a bound', async () => {
   spy.mockRestore()
 })
 
+test('does not rebind if it is the same ref', async () => {
+  const c = collection.doc()
+
+  const spy = spyOnSnapshot(c)
+  await c.update({ baz: 'baz' })
+  await d.update({ ref: c })
+  // NOTE see #1
+  await delay(5)
+  expect(spy).toHaveBeenCalledTimes(1)
+
+  await d.update({ ref: c })
+  await delay(5)
+
+  expect(spy).toHaveBeenCalledTimes(1)
+  spy.mockRestore()
+})
+
 test('resolves the promise when refs are resolved in a document', async () => {
   await a.update({ a: true })
   await b.update({ ref: a })