search

championswimmer / vuex-persist

A Vuex plugin to persist the store. (Fully Typescript enabled)
http://championswimmer.in/vuex-persist
MIT License
1.67k stars 117 forks source link

Proposal: Consider always using RESTORE_MUTATION instead of replaceState() #121

Open morphatic opened 5 years ago

morphatic commented 5 years ago

Per the Vuex docs:

Plugins are not allowed to directly mutate state - similar to your components, they can only trigger changes by committing mutations.

Current Behavior

Currently, when strictMode === false (the default, and the correct setting for production), this plugin uses store.replaceState() to update the state. When strictMode === true, the plugin commits a RESTORE_MUTATION that accomplishes the same thing.

Proposed Behavior

Always use RESTORE_MUTATION regardless of the strictMode setting.

Rationale

There are several rationale for this proposal:

  1. It will bring the plugin into alignment with the spec, referenced above.
  2. It will simplify the tests. Currently there are more tests than are probably necessary to account for testing the plugin in both strict and non-strict modes. Using RESTORE_MUTATION would allow you to get rid of all of the non-strict mode tests.
  3. It will address issues #15 and #43 and make PR #118 unnecessary. Users of the plugin will always be able to subscribe to RESTORE_MUTATION to find out when the store has been refreshed and is ready to use.
  4. It will prevent a potential developer pitfall when using Vue.js devtools. Currently, if you're serving a Vue app locally in development mode, but you've set strictMode === false in both vuex and vuex-persist even though the state gets restored by the plugin, it does not update the state if you inspect it with devtools. This is really confusing until you figure out what's going on. Always using RESTORE_MUTATION prevents this.

Of course, developers can already configure their apps to use the mutation by just setting strictMode === true regardless of whether vuex or the app itself is in strict mode or being served in production. I guess there's the possibility that by making the mutation the default behavior, it might cause issues for anyone who has subscribed to RESTORE_MUTATION strictly to provide debugging information.

Even if it is decided not to abandon using replaceState(), it would be a good idea to put a note in the README letting people know that in production this plugin will mutate state outside of a mutation.

Thanks for a great plugin!!!

championswimmer commented 5 years ago

This sounds like a good proposal! Makes sense. If you want to make a PR for the same, would be happy to accept!