Patch to nested object is changing object content order

[trevorgeise]

I have some nested data coming in from Firestore, so that my store looks like this

 pages: {
  pageid1: {
    id: 'pageid1',
    title: 'My page title',
    notes: {
        noteId1: {
          id: noteId1,
          text: "My note!",
          order: 1,
         },
         noteId2: {
          id: noteId2,
          text: "My second note!",
          order: 1,
         }
       }
    }

I'm running into two problems with this, and I think they are related.

Problem 1

When I update the text on a note, using patch, the order of my notes (the position of the key/value pair, nothing to do with the order field) inside my notes object changes, which seems to trigger reactivity on the whole object, causing everything connected to page.notes to rerun ( all notes). This happens instantly based on the changes made in the store, not from data coming back from Firestore.

I've verified this by both logging out the object at each point, but also by having a v-for of notes fed by the object. When the patch happens, the updated note jumps to the bottom of the list in the ui.

If, instead of going through vuex-easy-firestore I update the store myself with a mutation, either by using Vue.set(state[pageid].notes[noteid], 'text', newText) or with state[pageid].notes[noteid].text = newText, all reactivity happens as expected. The store gets updated, the object doesn't change order, and only the computed properties touching the note text get re-run.

Problem 2

Second problem is after every patch serverChange gets fired. I'm assuming this is because the nested object in the store is a weird order, but the object coming back from firebase is in the original order, so it fails the diff. However, serverChange seems to get triggered even if I update a non-nested field, like page.title.

Any thoughts would be greatly appreciated!

[mesqueeb]

@trevorgeise Thanks for your issue.
I really want to help you but, I'm just not 100% understanding exactly what you mean.

Problem 1:
Can you give me the following:

• your vuex-easy-firestore module config
• What action you dispatch (please provide code snippet of the exact action you call with what data)
• what do you expect to happen
• and what really happens

Problem 2:
I have no idea why serverChange fires after every change. It should not happen.
I'll need to see exactly what action you dispatch, and also see any hooks if you have them, to understand why this might happen.

[trevorgeise]

Absolutely! This is how my module looks (slimmed down for clarity)

moduleConfig = {
  firestorePath: 'pages',
  firestoreRefType: 'collection',
  moduleName: 'pages',
  statePropName: 'pages',
  namespaced: true,
  sync: {
    patchHook: function(updateStore, doc) {
      console.log('patch hook', doc);
      updateStore(doc);
    },

    patchHookBeforeSync: function(updateFirestore, doc) {
      console.log('patch hook before sync', doc);
      updateFirestore(doc);
    }
  },
  serverChange: {
    modifiedHook: function(updateStore, doc) {
      console.log('server change', doc);
      updateStore(doc);
    }
  },
  state: {},
  actions: {
    updatePageTitle({ dispatch }, { pageId, title }) {
     // both pageId and titleId are strings
      dispatch('patch', { id: pageId, title });
    },
    updateNoteText({ dispatch }, { pageId, noteId, text }) {
      // pageId, noteId and text and strings
      const notes = {};
      notes[noteId] = { text };

      //update vuex and database with vuex-easy-firestore
      dispatch('patch', { id: pageId, notes });
    },
  }
}

what do you expect to happen

I expect the field changed to be updated in Vuex, which will trigger all computed properties that reference that field to re-run, but not computed properties referencing sibling fields. Then I expect just that field to be updated to Firestore. Then I expect no serverChange to trigger.

and what really happens

The field in Vuex gets updated, but the object the field lives in (page.notes) gets re-ordered. e.g. if the object pulled down from firebase looked like

    notes:  {
        noteId1: {
          id: noteId1,
          text: "My note!",
          order: 1,
         },
         noteId2: {
          id: noteId2,
          text: "My second note!",
          order: 1,
         }
       }

if I updated the note1 text with the above action vuex looks like this:

{
       noteId2: {
          id: noteId2,
          text: "My second note!",
          order: 1,
         }
        noteId1: {
          id: noteId1,
          text: "My UPDATED note!",
          order: 1,
         },        
       }

At this point any computed property referencing any of the fields inside notes gets re-run, as if the entire object got recreated.

Then a beat later a serverChange gets fired and the document reverts back to the proper order, with the updated field. Then all computed properties run again.

{
        noteId1: {
          id: noteId1,
          text: "My UPDATED note!",
          order: 1,
         },   
       noteId2: {
          id: noteId2,
          text: "My second note!",
          order: 1,
         }
       }

[mesqueeb]

I'm still not sure why serverChange gets triggered, it shouldn't.

For your first problem. It seems to me that everything works as expected, all but the computed properties.

In JavaScript you can never assume order in object props will always be the same, this might even differ between browsers as well.
You'll need to create a new computer property that returns an ordered array based on that object.

As for reactivity of computed properties based on certain ids, this is always very fragile in Vue 2 but it is a problem of how computed properties work, not my library. With some experience you'll learn what is possible and what is not reactivity-wise when it comes to computed properties.

If you share the code of the computed properties you are referring to, maybe I can give extra advice. 😄

_{--
Vuex Easy Firestore was made with ♥ by Luca Ban.
If you use this library in your projects, you can support the maintenance of this library by a small contribution via Github 💜.
You can also reach out on twitter if you want a one-on-one coding review/lesson. 🦜}

[louisameline]

Actually, when iterating on an object, in modern browsers, the order of keys is supposed to stay in the same order as the order in which they were created, except for integer keys and symbol keys. I recently was suprised too to see in my app that I had an issue with order, but I didn't go to the bottom of it to know what was responsible. I'll check it shortly to know if it could come from the library.

[trevorgeise]

Yes, that's my understanding too @louisameline .
I fear I am doing a terrible job explaining the issue I'm seeing.

I think in simplest terms my issue is this:
If I use vuex-easy-firestore to update Vuex with patch a nested field, like 'notes.noteid.text', it causes the entire object (notes) to change, as if it is replacing the entire thing, instead of just the updated field. It triggers reactivity as if it was a whole new object.

But, If I use a mutation to make the change in Vuex, it works exactly as expected. Only the updated field triggers an update. Everything doesn't get re-calculated.

I just tried hijacking the patch hook so instead of calling updateStore(doc), I mutate the store myself with Vue.set. This works great from a Vuex perspective. Everything updates properly, nothing is reactive that shouldn't be.

Later today I will try to recreate the issue in a test repo.

[trevorgeise]

Alright, I think I found the issue.

I kind of suck at digging through code, but I think when you are calling this._vm.$set(ref, key, newVal) inside your PATCH_DOC mutation, the object you are updating is just the root document object. You seem to be merging the new change with the rest of the object, but in this case that is forcing the entire object to be treated as updated, rather than just the nested field.

I think the reason that is happening is because when that merge happens, you may be doing something in merge_anything that causes the object to think the updated property is in fact a new property, and therefore, as @louisameline pointed out, the order changes because the creation date changes.

Whatever the reason, this makes Vue see the whole thing as new, and triggers reactivity on the whole thing, rather than just the updated field.

That causes everything touching the notes object to re-render, it thinks the whole notes object has been replaced and so everything needs to be updated. I think that is also why serverChange gets called. I'm guessing this merge thing isn't happening on the update to firstore. So in firestore only the field is getting updated, not the whole object. So when that comes back down the pipe, it sees a difference and has to reconcile.

Instead of merging, here is what I did inside the patchHook to update just the target field.

patchHook: function(updateStore, doc, store) {
      // function to turn the 'doc' into dot notation
      function dotNotate(obj, target, prefix) {
        (target = target || {}), (prefix = prefix || '');
        Object.keys(obj).forEach(function(key) {
          // if we are either at a final value or a firestore date object, move on
          if (typeof obj[key] === 'object' && obj[key].toDate != 'function') {
            dotNotate(obj[key], target, prefix + key + '.');
          } else {
            return (target[prefix + key] = obj[key]);
          }
        });
        return target;
      }
      const updates = dotNotate(doc);
      const updateKeys = Object.keys(updates);
      const docId = updates.id;

      updateKeys.forEach((k, n) => {
        if (n == 0) {
          // first key is page id, ignore
          return;
        }
        const pathArray = k.split('.');
        const key = pathArray.pop();
        // use lodash get to get the nested object in store we want to update
        const updateObject = get(store.state.pages.pages[docId], pathArray);
       Vue.set(updateObject, key, updates[k]);
}

That works great. Only text gets updated, the rest of the object is left alone.
I don't know how to reconcile that with the helpers you implement for incrementing and what not. Or if this is just weird edge case.
I also don't know if I can implement this, skip the plugin from updating vuex, but have it handle writing to firestore.

[mesqueeb]

@trevorgeise Ah... I understand the problem now.
So, yeah, basically I make a copy of the original object, merge changes onto it, then put it back in place in Vuex.

I never realised this would trigger Vue's reactivity to see the entire object as new, but I guess it makes sense...

I'm not sure how to get around this though. I feel like messing with this might break some things.

I will definitely keep this in mind for version 2 though! I'm already making good progress on this. 😉

As for the serverChange hook triggered, maybe we can do a VSCode Live session so it's easy to try and debug on my end. Can you add me on discord?
Luca Ban [Mesqueeb]#4774

[mesqueeb]

@trevorgeise I implemented your fix for Problem 2 above.

For problem 1 I kind of want to wait for v2... (your fix on your branch was really a lot of lines and added new dependencies)

I'm working on v2.0 now, and I'll make sure to have a test that covers and prevents this case!!!

[dsl101]

I have exactly the same issue at the moment—a component bound to one part of the data is being triggered to re-render when other unrelated data is changed. I also tried vuex-firestore, but have the same problem there. Replacing whole Vuex objects is definitely going to lead to performance issues in Vue reactivity, so I'm really glad to hear this fix is on the cards. Do you have any idea of a timeline yet?

[trevorgeise]

I fixed it in this branch, https://github.com/trevorgeise/vuex-easy-firestore , if you want to see a solution.
Instead of replacing the whole object it now only updates the changed field, so it gets triggered properly. It's not the most elegant, but it works an it's obviously a huge performance improvement for nested stuff.

[dsl101]

Thanks—I will definitely take a look. Are you planning to maintain a fork, or is @mesqueeb going to roll it back in?

[trevorgeise]

I'm not sure yet. Probably as my needs require. But @mesqueeb is planning on implementing a proper fix for 2.0, so hopefully I can just switch to that.

[mesqueeb]

Thanks, based on @trevorgeise's version, I wrote a fix, but when I tried adding the fix, it broke a bunch of tests, so I have some more troubleshooting to do. I ran out of time today, but will try to continue working on this over the next days.

[mesqueeb]

@trevorgeise @dsl101
I have just deployed a fix in v1.35.9, please update to the latest version and let me know how it goes.

@trevorgeise Both your problems should now be fixed!🎉

_{--
Vuex Easy Firestore was made with ♥ by Luca Ban.
If you use this library in your projects, you can support the maintenance of this library by a small contribution via Github 💜.
You can also reach out on twitter if you want a one-on-one coding review/lesson. 🦜}

[dsl101]

I have a case where there's still an unwanted update / re-render in Vue. I have an object bound to a set of users in firestore at /users/<userId>/, and each user has a profile such as:

profile: {
  name: 'Bob',
  value: 42,
  access: 'something'
}

In my code, I bind a component to, say, all the user names. If I change that field in firestore console, I see my component update, and also see the beforeUpdate() and updated() Vue lifecycle hooks fire.

If I change value to, say, 43, I see no updates in the app, and no lifecycle hooks fired (this is definitely fixed from the previous version, where I would see those events regardless of what I changed in the user profile).

But, if I delete the access key in the profile, I do see both the Vue lifecycle hooks fire, which re-renders the component (in the real app, I sometimes have hundreds or even thousands of components here). In the mutation history, I see users/DELETE_PROP with the payload B8Wdhgd3x5JkU8i8Ljlt.profile.access (which looks spot on), and then users/PATCH_DOC with the full object as a payload. I'm guessing the second mutation fails the comparison somehow and resets the whole vuex object?

[mesqueeb]

@dsl101 i will do extra troubleshooting for deleting props. But you can just set the prop to null as well and make sure your frontend handles that the same as not existing.

This workaround would solve your issues I think.

[dsl101]

Not sure I understand the workaround—changes are coming in to my app from elsewhere via firestore... Where would I intercept the update?

…

On Thu, 23 Apr 2020 at 21:26, Luca Ban ***@***.***> wrote: Reopened #300 <#300> . — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#300 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAG33PRV6WXMHDCUIRRVJYDROCP7XANCNFSM4LCP7O2A> .

[mesqueeb]

@dsl101 wherever you're deleting the property. Can't you set it to null instead of deleting it?

[dsl101]

Ah, I see. Yes, I'm converting this app from firebase to firestore, and setting the key to null in firebase is the same as deleting it. So, in fact, most of it should work fine!

It would obviously be great if a delete really did only delete the key, but much less of an issue in the short term now. Many thanks for the updates!

[dsl101]

Just an update on this—adding properties is also triggering an update on the bound Vue component. So whilst setting unwanted props to null instead of deleting them is fine, it's not obvious how to work around this one. Would you expect adding props to also recreate the entire object? Note subsequently changing the value of the newly added prop does not trigger an update.

[mesqueeb]

@dsl101 I'll have to double check my logic for adding props that were not already there, and for deleting props as well.
Sorry haven't gotten around to it yet.

in the meantime, if you want to try your hand, it's this logic that would need updating:
https://github.com/mesqueeb/vuex-easy-firestore/blob/master/src/module/mutations.ts#L117

Anything inside this PATCH_DOC mutation. 😉
the delete prop one is DELETE_PROP

[dsl101]

OK. I've been able to reproduce the 'error' (if we can call it that for now—also likely my misunderstanding of Vuex :) in a pure Vuex store, so I'm going to try submitting an issue to Vuex and see if this is expected behaviour. The premise is this:

State contains a single object with 1 property (prop1). That is accessed via a getter and a mutation to increment it, and its current value is displayed in a Vue component.

There are also 3 other mutations to create, increment, and delete a second prop (prop2) on the state object. The create and delete mutations cause the Vue component bound to the prop1 getter to update, but the call to increment prop2 does not. This seems at least inconsistent.