vuejs / vuex

🗃️ Centralized State Management for Vue.js.
https://vuex.vuejs.org
MIT License
28.42k stars 9.58k forks source link

Dynamic module state not created with hot loading (preserveState set to true) #1675

Open ashahbazFV opened 4 years ago

ashahbazFV commented 4 years ago

Version

3.1.2

Reproduction link

https://codesandbox.io/s/compassionate-hermann-msyze

Steps to reproduce

  1. Create a module with vuex-module-decorators
  2. Set the dynamic and preserveState flags to true
    @Module({ dynamic: true, name: 'SomeModule', store, preserveState: true })
  3. Attempt to access a property of the module, retrieved with getModule function to initialise the module from vuex-module-decorators
    const someModule = getModule(SomeModule);
    someModule.someProperty;
  4. Note someProperty will be undefined

What is expected?

When installing a dynamic module and there is no state present, a default state should be created

What is actually happening?

When installing a dynamic module and there is no state present, the module is not registered causing reads to the state for that module to fail.


NOTE: Was unable to reproduce the issue cleanly in CodeSandbox so the reproduction is not working, but the key aspects are still there.

Upon investigation, it was discovered the vuex-module-decorators lib passes their preserveState flag down to the vuex layer. When the dynamic module is installed with the installModule function and the hot option is enabled, the current implementation skips creating any state for the new module. This introduces side effects, where the module is registered in the vuex store but the state can't be retrieved if there was no state previously present.

The vuex-persist lib is in use, but it is used to hydrate any past state down to vuex. The store, while using the vuex-persist lib, has the data initialised in the following order.

  1. The store is created by vuex and any known modules are initialised with their default values.
  2. If there are data in the local storage within the browser, it is loaded into the store state replacing the default state data.
  3. Dynamic modules are added as they are needed.

Upon modifying the installModule function, to check if the state of the new dynamic module exists, the problem is resolved. Essentially, if the state of the module doesn't currently exist in the parentState within the store, then the default module state is created and registered.

One set of changes that fix this case are as follows.

In src/store.js the following code seems to be the offending code.

// set state
if (!isRoot && !hot) {
  const parentState = getNestedState(rootState, path.slice(0, -1))
  const moduleName = path[path.length - 1]
  store._withCommit(() => {
    if (process.env.NODE_ENV !== 'production') {
      if (moduleName in parentState) {
        console.warn(
          `[vuex] state field "${moduleName}" was overridden by a module with the same name at "${path.join('.')}"`
        )
      }
    }
    Vue.set(parentState, moduleName, module.state)
  })
}

When modified, the default state is created when the moduleName isn't found in the parentState.

// set state
var parentState = getNestedState(rootState, path.slice(0, -1));
var moduleName = path[path.length - 1];
var moduleStateExists = moduleName in parentState
if (!isRoot && (!hot || !moduleStateExists )) {
 store._withCommit(function () {
   {
     if (moduleStateExists) {
       console.warn(
         ("[vuex] state field \""   moduleName   "\" was overridden by a module with the same name at \""   (path.join('.'))   "\"")
       );
     }
   }
   Vue.set(parentState, moduleName, module.state);
 });
}

There may be other ways to fix this issue, this is one potential solution that was discovered after some investigation.

filips123 commented 3 years ago

I also had the problem because of this. And note that is also happens with other plugins similar to vuex-persist, such as vuex-persistedstate (robinvdvleuten/vuex-persistedstate#311).

I agree with your proposed solution, but I think there should also be a way to merge default state with stored state, so all keys from stored state will stay the same while adding all other keys/fields from the default store. This will be useful in cases when new keys are added to the store, but they don't exist yet in localStorage. If merging is implemented, the new keys would just be merged with existing ones. If not, complete localStorage module would overwrite new keys, causing them to be undefined.