fix the temp record workflow

When you call new Todo(), a temp record is created.  The temp record contains a temporary `__id` (note the double underscore) and a `__isTemp` attribute.  (These attributes can be discarded in a before hook so they don’t get sent to the server.)

To assist in Vue reactivity, this temp record is updated with the API response.  The record will keep the local `__id` for building components that recognize the change from temp to permanent record.  Records will no longer have the `__isTemp` attribute.

To keep this from being a breaking change, a few new mutation were created.  They can be found in the `addOrUpdate` service action:

- `remove__isTemp` does just what it says.
- `replaceItemWithTemp` Adds or updates an item. If a matching temp record is found in the store, the temp record will completely replace the existingItem. This is to work around the common scenario where the realtime `created` event arrives before the `create` response returns to create the record. The reference to the original temporary record must be maintained in order to preserve reactivity.

Author: marshallswain

src/service-module/service-module.actions.ts

@@ -106,10 +106,14 @@ export default function makeServiceActions(service) {
             })
           } else {
             const id = getId(response, idField)
+            const tempId = tempIds[0]
 
-            await dispatch('addOrUpdate', response)
+            if (id != null && tempId != null) {
+              commit('updateTemp', { id, tempId })
+            }
+            response = await dispatch('addOrUpdate', response)
 
-            response = state.keyedById[id]
+            // response = state.keyedById[id]
           }
           commit('unsetPending', 'create')
           commit('removeTemps', tempIds)
@@ -204,7 +208,10 @@ export default function makeServiceActions(service) {
      *         Feathers client.  The client modifies the params object.
      *   @param response
      */
-    async handleFindResponse({ state, commit, dispatch }, { params, response }) {
+    async handleFindResponse(
+      { state, commit, dispatch },
+      { params, response }
+    ) {
       const { qid = 'default', query } = params
       const { idField } = state
 
@@ -289,24 +296,40 @@ export default function makeServiceActions(service) {
       return response
     },
 
+    /**
+     * Adds or updates an item. If a matching temp record is found in the store,
+     * the temp record will completely replace the existingItem. This is to work
+     * around the common scenario where the realtime `created` event arrives before
+     * the `create` response returns to create the record. The reference to the
+     * original temporary record must be maintained in order to preserve reactivity.
+     */
     async addOrUpdate({ state, commit }, item) {
       const { idField } = state
       let id = getId(item, idField)
       let existingItem = state.keyedById[id]
 
       const isIdOk = id !== null && id !== undefined
 
-      if (
-        service.FeathersVuexModel &&
-        !item.isFeathersVuexInstance
-      ) {
+      if (service.FeathersVuexModel && !item.isFeathersVuexInstance) {
         item = new service.FeathersVuexModel(item)
       }
 
+      // If the item has a matching temp, update the temp and provide it as the new item.
+      const temp = state.tempsByNewId[id]
+      if (temp) {
+        commit('merge', { dest: temp, source: item })
+        commit('remove__isTemp', temp)
+      }
       if (isIdOk) {
-        existingItem ? commit('updateItem', item) : commit('addItem', item)
+        if (existingItem && temp) {
+          commit('replaceItemWithTemp', { item, temp })
+        } else {
+          existingItem
+            ? commit('updateItem', temp || item)
+            : commit('addItem', temp || item)
+        }
       }
-      return item
+      return temp || item
     }
   }
   /**

src/service-module/service-module.mutations.ts

@@ -143,6 +143,32 @@ export default function makeServiceMutations() {
       updateItems(state, items)
     },
 
+    // Adds an _id to a temp record so that that the addOrUpdate action
+    // can migrate the temp to the keyedById state.
+    updateTemp(state, { id, tempId }) {
+      const temp = state.tempsById[tempId]
+      if (state.tempsById) {
+        temp[state.idField] = id
+        state.tempsByNewId[id] = temp
+      }
+    },
+
+    /**
+     * Overwrites the item with matching id with the temp record.
+     * This is to preserve reactivity for temp records.
+     */
+    replaceItemWithTemp(state, { item, temp }) {
+      const id = item[state.idField]
+      if (state.keyedById[id]) {
+        state.keyedById[id] = temp
+        Vue.delete(state.keyedById[id], '__isTemp')
+      }
+    },
+
+    remove__isTemp(state, temp) {
+      Vue.delete(temp, '__isTemp')
+    },
+
     removeItem(state, item) {
       const { idField } = state
       const idToBeRemoved = _isObject(item) ? getId(item, idField) : item
@@ -155,8 +181,18 @@ export default function makeServiceMutations() {
       }
     },
 
-    // Removes temp records
+    // Removes temp records. Also cleans up tempsByNewId
     removeTemps(state, tempIds) {
+      const ids = tempIds.reduce((ids, id) => {
+        const temp = state.tempsById[id]
+        if (temp && temp[state.idField]) {
+          delete temp.__isTemp
+          Vue.delete(temp, '__isTemp')
+          ids.push(temp[state.idField])
+        }
+        return ids
+      }, [])
+      state.tempsByNewId = _omit(state.tempsByNewId, ids)
       state.tempsById = _omit(state.tempsById, tempIds)
     },
 

src/service-module/service-module.state.ts

@@ -23,7 +23,8 @@ export default function makeDefaultState(servicePath, options) {
     ids: [],
     keyedById: {},
     copiesById: {},
-    tempsById: {},
+    tempsById: {}, // Really should be called tempsByTempId
+    tempsByNewId: {}, // temporary storage for temps while getting transferred from tempsById to keyedById
     pagination: {
       defaultLimit: null,
       defaultSkip: null

test/service-module/make-service-plugin.test.ts

@@ -87,6 +87,7 @@ describe('makeServicePlugin', function() {
       servicePath: 'todos',
       skipRequestIfExists: false,
       tempsById: {},
+      tempsByNewId: {},
       whitelist: []
     }
 

test/service-module/model-temp-ids.test.ts

@@ -15,6 +15,7 @@ import ObjectID from 'bson-objectid'
 
 interface RootState {
   transactions: ServiceState
+  things: ServiceState
 }
 
 class ComicService extends MemoryService {
@@ -50,6 +51,7 @@ function makeContext() {
     }
   }
   const store = new Vuex.Store({
+    strict: true,
     plugins: [
       makeServicePlugin({
         Model: Comic,
@@ -147,7 +149,71 @@ describe('Models - Temp Ids', function() {
     assert(store.state.transactions.tempsById[txn.__id], 'it is in the store')
   })
 
-  it('clones into Model.copiesById', function () {
+  it('maintains reference to temp item after save', function() {
+    const { makeServicePlugin, BaseModel } = feathersVuex(feathersClient, {
+      idField: '_id',
+      serverAlias: 'temp-ids'
+    })
+    class Thing extends BaseModel {
+      public static modelName = 'Thing'
+      public constructor(data?, options?) {
+        super(data, options)
+      }
+    }
+    const store = new Vuex.Store<RootState>({
+      plugins: [
+        makeServicePlugin({
+          Model: Thing,
+          service: feathersClient.service('things')
+        })
+      ]
+    })
+
+    // Manually set the result in a hook to simulate the server request.
+    feathersClient.service('things').hooks({
+      before: {
+        create: [
+          // Testing removing the __id and __isTemp so they're not sent to the server.
+          context => {
+            delete context.data.__id
+            delete context.data.__isTemp
+          },
+          context => {
+            assert(!context.data.__id, '__id was not sent to API server')
+            assert(!context.data.__id, '__isTemp was not sent to API server')
+            context.result = {
+              id: 1,
+              description: 'Robb Wolf - the Paleo Solution',
+              website:
+                'https://robbwolf.com/shop-old/products/the-paleo-solution-the-original-human-diet/',
+              amount: 1.99
+            }
+            return context
+          }
+        ]
+      }
+    })
+
+    const thing = new Thing({
+      description: 'Robb Wolf - the Paleo Solution',
+      website:
+        'https://robbwolf.com/shop-old/products/the-paleo-solution-the-original-human-diet/',
+      amount: 1.99
+    })
+
+    assert(store.state.things.tempsById[thing.__id], 'item is in the tempsById')
+
+    return thing.save().then(response => {
+      assert(response._id === 1)
+      assert(response.__id, 'the temp id is still intact')
+      assert(!store.state.things.tempsById[response.__id])
+      //@ts-ignore
+      assert(!Object.keys(store.state.things.tempsByNewId).length)
+      assert(response === thing, 'maintained the reference')
+    })
+  })
+
+  it('clones into Model.copiesById', function() {
     const { makeServicePlugin, BaseModel } = feathersVuex(feathersClient, {
       idField: '_id',
       serverAlias: 'temp-ids'

test/service-module/service-module.test.ts

@@ -637,6 +637,7 @@ describe('Service Module', function() {
         servicePath: 'service-todos',
         tempIdField: '__id',
         tempsById: {},
+        tempsByNewId: {},
         pagination: {
           defaultLimit: null,
           defaultSkip: null