Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support store structure changing #30

Closed
andrey-hohlov opened this issue Feb 5, 2021 · 8 comments
Closed

Support store structure changing #30

andrey-hohlov opened this issue Feb 5, 2021 · 8 comments
Assignees
Labels
enhancement New feature or request

Comments

@andrey-hohlov
Copy link

andrey-hohlov commented Feb 5, 2021

For example, we have a user vuex-module with this state:

{
  token: '',
  name: '',
}

The state is saved in local storage.

createMultiTabState({ statesPaths: ['user']  })
{ "token": "tokenstring", "name": "Jonn Smith" }

But some reasons we change state object to:

{
  name: '',
  auth: {
    token: ''
  },
}

And now want to keep in storage only user.auth path.

createMultiTabState({ statesPaths: ['use.auth']  })

Reload page - and it removes the state auth key! The app is broken.

client.js?06a0:97 TypeError: Cannot read property 'token' of undefined

This error happens in the getter:

{
  isAuthenticated(state) {
    return !!state.auth.token;
  },
}

And yes, I can fix it:

{
  isAuthenticated(state) {
    return !!state.auth && !!state.auth.token;
  },
}

But maybe It will be better if vuex-multi-tab-state will respect the default state and do not override it if the path does not exist in the local storage. And do not create top-level state properties if they were removed from the default state. What do you think?

@gabrielmbmb gabrielmbmb self-assigned this Feb 6, 2021
@gabrielmbmb gabrielmbmb added the enhancement New feature or request label Feb 6, 2021
@gabrielmbmb
Copy link
Owner

Hi @andrey-hohlov, this is something I experienced myself. The only fix right now is to go Application > Local Storage and remove the key of vuex-multi-tab-state in order to force the resync of the state to the local storage. I'm trying to find a solution for this issue.

@joshuabyler
Copy link

@andrey-hohlov What I did to get around this at least for the people using the application was on login

window.localStorage.removeItem('vuex-multi-tab')

For myself while developing on localhost I just had to manually clear it

@andrey-hohlov
Copy link
Author

andrey-hohlov commented May 28, 2021

@gabrielmbmb I thought a lot about this problem.
There are 2 points:

  1. Properties can disappear (see my example above).

We can modify tab.fetchState or put a simple plugin before vuex-multi-tab-state.
Get local storage data and merge the default vuex state with this data.

Works for my example, but unpredictable consequences are possible.
It can be used in a project, but too dangerous for open source lib.

But, we can add a new option, hook onBeforeFetchState where any logic can be implemented.

  1. Unnecessary properties can be added.

The same.

So, my conclusion is:

There is no way to manage it in a universal way.
In each project developers should be careful and manage such cases by themselves.
onBeforeFetchState hook option, where we can modify data that comes from local storage, can be helpful for such cases.

What do you think?

@entioentio
Copy link

Random: I can imagine that for some cases it'd be actually better to store the whole store behind a version. Then by initialization you'd be able to migrate existing data.

abernh added a commit to abernh/vuex-multi-tab-state that referenced this issue Oct 18, 2021
both hooks get the current state passed and if they return a falsy value
the operation is canceled
this allows to pre-filter/-validate a state once more before it is
restored and/or saved

see also gabrielmbmb#30
@abernh
Copy link
Contributor

abernh commented Oct 19, 2021

I currently solve that via another store.subscribe which does an "undo" if the new state is not valid

// ~/plugins/multiTabState.client.js
import createMultiTabState from 'vuex-multi-tab-state';
import { simpleCloneState } from '~/common/utils'

export default ({ store }) => {

  // prevent multiTabState to overwrite store with "old" version
  let validState = simpleCloneState(store.state);
  store.subscribe((mutation, state) => {
    if(!state.storeVersion || state.storeVersion < validState.storeVersion){
      store.replaceState(validState)
    }else{
      validState = simpleCloneState(state);
    }
  })

  createMultiTabState()(store);
};

But I'd rather have that invalid state never land in my store. I hope for the hook-parameters #65 to be worthy of a merge.

gabrielmbmb added a commit that referenced this issue Oct 23, 2021
…-and-onBeforeSave

Feat #30 add params onBeforeReplace and onBeforeSave
@gabrielmbmb
Copy link
Owner

gabrielmbmb commented Oct 23, 2021

I will release the v1.0.17 which already has the before onBeforeReplace and onBeforeSave hooks. Thank you so much @abernh for the PRs!

@wparad
Copy link

wparad commented Oct 23, 2021

Just a question, wouldn't this also be solved once there is a solution for #63, since this plugin will instead start by taking the new current store and never using what's in the localstorage until after the store has been loaded?

@abernh
Copy link
Contributor

abernh commented Oct 24, 2021

I am actually not sure if #63 is resolvable. But let's discuss that over there.

The onBeforeReplace and onBeforeSave hooks allow much more than just prevent old-structured-state to be saved into store.

State timeout

It could allow you to have a simple "timeout" functionality, where the cached state is treated as "old" after some idle time

State validation based on token

You could use a token value in the current store (due to an read-in of a user-session before-multi-tab-state-plugin-init e.g. from a cookie) to compare with the cached-state to make sure an out-dated user session is not accidentally restored with a half-valid UI state based on a closed user session, causing the UI trying to request content from the backend that it has no longer permission to to request, leaving the UI in a broken state.

Custom filtering

Or you could implement your own filtering functionality to filter for the "to be saved/restored" parts of the state yourself e.g. if you have a complex state where you'd rather use a "blacklist" than the currently available "whitelist" of paths.

.
The possibilities are endless. The hooks could now even be used to find a solution for #63 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants