vuex-pathify icon indicating copy to clipboard operation
vuex-pathify copied to clipboard

collision between object path and serialization

Open Heziode opened this issue 2 years ago • 8 comments

https://github.com/davestewart/vuex-pathify/blob/9a3537540914cc6114b894c1fb359b41263167ee/src/services/store.js#L23-L25

The above code create non Payload object for "root path", like the following example:

// global.js Vuex module
import { make } from "vuex-pathify";

export const state = () => ({
  something: true,
});

export const mutations = {
  ...make.mutations(state),
};

export const getters = {
  ...make.getters(state),
};

If I call $store.set("global/something", false), the Payload will be a simple object (like serialized object) and not a Payload instance.

Is there anything in particular to avoid using a Payload for the root path?


nb @davestewart : you did not push tags not the repo, and you did not release 1.5.1 on NPM. I know that you are very busy, so, stay in shape 💪

Heziode avatar Aug 09 '21 14:08 Heziode

Hey @Heziode,

Thanks for digging in and going over the code :)

The objPath property is essentially the @path.to.item part of the expression:

https://github.com/davestewart/vuex-pathify/blob/9a3537540914cc6114b894c1fb359b41263167ee/src/services/resolver.js#L107

The Payload class is used only so we can do target instanceof Payload and then use its update() method to deep-set the value:

https://github.com/davestewart/vuex-pathify/blob/master/src/classes/Payload.js#L25

So generally, unless your paths have a @ in them, data will remain as POJOs.

Does that answer your question?

Re. 1.5.1, yes, I need to publish 1.5.1! Thanks for spotting. Will do that now.

davestewart avatar Aug 10 '21 10:08 davestewart

The Payload class is used only so we can do target instanceof Payload and then use its update() method to deep-set the value:

So generally, unless your paths have a @ in them, data will remain as POJOs.

The problem with POJO is that it becomes impossible to distinguish between a call to set of pathify and a mutation done by a serialized payload, since they are both POJOs.

Heziode avatar Aug 10 '21 11:08 Heziode

OK. Not 100% sure what the problem is, as I haven't tested a serialisation.

If you want to create and post a sample repo, that would be useful.

davestewart avatar Aug 10 '21 11:08 davestewart

@davestewart take a look at the following repository: https://github.com/Heziode/vuex-pathify-serialization-multi-tab

The main reason in my case to be able to distinguish POJO of vuex-pathify and POJO of serialization is to be able to do some work if the orchestrator is Vuex-Pathify (sync the store with a backend)

Heziode avatar Aug 10 '21 12:08 Heziode

Thanks! I can't promise to look at it today, but let me try in the next couple of days.

Do you have any ideas to solve the problem (which I don't understand yet!) ?

davestewart avatar Aug 10 '21 12:08 davestewart

The solution would be to use the Payload class in any cases.

Like this it will be very trivial to detect if the change is orchestrated by vuex-pathify or something else (including serialization).

I do not analyzed the whole code of the lib, so I don’t know if it can be convenient to do this.

Heziode avatar Aug 10 '21 12:08 Heziode

So if we took that approach, you're basically saying:

  • if it's a POJO, it would have been deserialized
  • if it's a Payload, it would have been created internally by Pathify

?

I can't remember how everything is put together TBH but I'll dig in again in the next few days.

Do the changes you pushed for solve the serialisation aim, or is this additional work needed to completely solve it?

davestewart avatar Aug 10 '21 12:08 davestewart

So if we took that approach, you're basically saying:

  • if it's a POJO, it would have been deserialized
  • if it's a Payload, it would have been created internally by Pathify

?

Right, this is what I mean.

Do the changes you pushed for solve the serialisation aim, or is this additional work needed to completely solve it?

#125 cover a big part of the serialisation problem but not the not the problem raised here. So additional work is needed to cover this case.

Heziode avatar Aug 10 '21 13:08 Heziode