prazdevs / pinia-plugin-persistedstate

💾 Configurable persistence and rehydration of Pinia stores.
https://prazdevs.github.io/pinia-plugin-persistedstate/
MIT License
2.05k stars 117 forks source link

[core] Object data is empty after HMR (Hot Module Replacement) reload #79

Closed Zehir closed 1 year ago

Zehir commented 2 years ago

Hi, it's a quite specific case but there is my issue;

More info on HMR here.

Context

I started with the vitesse template from github.

You can chekout this commit if you wan't to try on your side.

I have a store defined like this :

import { acceptHMRUpdate, defineStore } from 'pinia'
export const useGatewaysStore = defineStore('gateways', () => {
  const data_array = ref<any[]>([])
  // data_array.value[0] = 3

  const data_record = reactive({})
  // data_record.foo = 35

  const test = computed(() => 1)

  return { data_array, data_record, test }
}, {
  // https://github.com/prazdevs/pinia-plugin-persistedstate
  persist: {
    paths: [
      'data_array',
      'data_record',
    ],
  },
})

// https://pinia.vuejs.org/cookbook/hot-module-replacement.html
if (import.meta.hot)
  import.meta.hot.accept(acceptHMRUpdate(useGatewaysStore, import.meta.hot))

With this data in the store :

{
  "data_array": [1],
  "data_record": {"foo":{"foo":38}}
}

Issue

When I load the page I get the correct value from the store. When I edit the store vite is sending the update to the browser to reload the store. For example if I edit the value of the computed property, it's updated in the browser. The data_array still contain the data but data_record became empty.

With manual defined values (the comments) without persist configuration, I keep the previous values and the computed value is updated. That why I think it's an issue with the plugin.

prazdevs commented 2 years ago

Sorry this issue completely went under the radar, i'll look into it as soon as V2 is live 🙏

Zehir commented 2 years ago

Sorry this issue completely went under the radar, i'll look into it as soon as V2 is live 🙏

No problem, I replaced the hot reload for à full reload for the store for now

prazdevs commented 1 year ago

Is this issue still a thing with the latest version of pinia and the plugin ?

Zehir commented 1 year ago

can't test for now, I will test again in a few month

Zehir commented 1 year ago

Sorry @prazdevs for late response but it's still happening in 3.0.2

{
    "@vueuse/core": "^9.11.0",
    "@vueuse/head": "^1.0.22",
    "axios": "^1.2.2",
    "binary-parser": "^2.2.1",
    "color": "^4.2.3",
    "nprogress": "^0.2.0",
    "pinia": "^2.0.29",
    "pinia-plugin-persistedstate": "^3.0.2",
    "postcss": "^8.4.21",
    "prism-theme-vars": "^0.2.4",
    "vite-plugin-vuetify": "^1.0.1",
    "vue": "^3.2.45",
    "vue-demi": "^0.13.11",
    "vue-i18n": "^9.2.2",
    "vue-json-viewer": "^3.0.4",
    "vue-router": "^4.1.6",
    "vue3-perfect-scrollbar": "^1.6.1",
    "vuetify": "^3.1.1"
}

Before I save : image

After I save : image No relevant log in the console.

And after I refresh the page I get the data back.

prazdevs commented 1 year ago

Is there a commit/repo/stackblitz I can use to reproduce it with the latest version ?

Zehir commented 1 year ago

Sure, I made a light test : https://github.com/Zehir/deconz-ruler/commit/3fbc39fa2c3c859b55f3c9b789cfc2de7e8ebbc0

Run it by using pnpm dev (after a pnpm install). Open localhost:4000/demo on a browser. Click on the button generate data.

How to reproduce :

When you remove the persist configuration I don't lose the data. When you update the vue in src/pages/demo.vue I don't lose the data.

prazdevs commented 1 year ago

So i looked a bit into it and the issue only occurs when using reactive and setup-syntax. Maybe i should look deeper into pinia HMR and try to understand why it won't work with this particular case. If you can use refs instead of reactive, it should work

prazdevs commented 1 year ago

I'd think the reason it refreshes is because it'd just hot-reload initial state, and not trigger the hydration from the plugin 🤔 have to find a way to trigger it on hmr i guess

prazdevs commented 1 year ago

Ok i can get it working by changing this line from pinia itself into useStore(pinia, existingStore)?.$hydrate() But it's kinda inside pinia, gotta find a way to call $hydrate in this situation smh

edit: this also works in ur reproduction

if (import.meta.hot) {
  import.meta.hot.accept(acceptHMRUpdate(useDemoStore, import.meta.hot))
  import.meta.hot.accept((newModule) => {
    const pinia = import.meta.hot?.data.pinia
    for (const exportName in newModule) {
      const useStore = newModule[exportName]
      const existingStore = pinia._s.get(useStore.$id)
      useStore(pinia, existingStore)?.$hydrate()
    }
  })
}

but it's just a modified/simplified copy paste of pinia's AcceptHMRUpdate code to call $hydrate

Zehir commented 1 year ago

what about creating your own acceptHMRUpdate function ?

prazdevs commented 1 year ago

that's literally what i'm doing rn 😬 i'll probably export it in some special package to avoid polluting the main one as it's more experimental than anything

prazdevs commented 1 year ago

Ok i have made and published @pinia-plugin-persistedstate/hmr that exports acceptHMRUpdateWithHydration. You should just be able to replace pinia's with this one and it should work. 🤔

Zehir commented 1 year ago

Thanks I will try this when I have some time available

MZ-Dlovely commented 1 year ago

have u try your code? image i think u need to read the additional context of my pr.

MZ-Dlovely commented 1 year ago

wait for me to write its workflow

prazdevs commented 1 year ago

🤔 i think i get it, the 2 would complement each other ?

MZ-Dlovely commented 1 year ago

the hot store will be create and merge with old store. what we need to do is $hydrate after store merged

MZ-Dlovely commented 1 year ago

if used original code will be this image print by afterRestore hook

MZ-Dlovely commented 1 year ago

when i mark debug for it image it will $patch image but not found at pinia.state image

MZ-Dlovely commented 1 year ago

🤔 i get the best solution, maybe

prazdevs commented 1 year ago

@MZ-Dlovely feel free to reopen a PR if that can fix this issue, would this also need the custom AcceptHMRUpdate method ?

MZ-Dlovely commented 1 year ago

i think we need. i'm doing for it

MZ-Dlovely commented 1 year ago

i will check it a little time

prazdevs commented 1 year ago

Im very busy these days and wanna solve this problem properly. Need to see how HMR should be handled:

hi-reeve commented 1 year ago

any update on this?

waney commented 1 year ago

Thank you. any update on this?

prazdevs commented 1 year ago

Hi, been very busy with work and life these days. I wanna try to fix this for good, but I want to avoid adding much stuff to the main plugin, or risky side effects.

There's also the concept of "what is stale data" when using HMR: the stored one? or the initial state one?

Once these questions are solved, and we can have a solution that should be good :)

MZ-Dlovely commented 1 year ago
moment description store hot store storage
before initial first using store \ \ up to date
doing initial store created but not hydrate empty \ up to date
after initial store hydrate by us up to date \ up to date
before hmr save as new store file stale up to date stale
doing hmr pinia try to merge up to date up to date stale
after hmr hot store destoried after merge up to date \ stale
prazdevs commented 1 year ago

@MZ-Dlovely could you test HMR with the current repo if it works for you as intended ? using the default AcceptHMRUpdate function provided by pinia? If it works, i'll make a release :)

MZ-Dlovely commented 1 year ago

@MZ-Dlovely could you test HMR with the current repo if it works for you as intended ? using the default AcceptHMRUpdate function provided by pinia? If it works, i'll make a release :)

i'm sorry that i just saw your msg. have you tested for it? what should i do for it more? XD

prazdevs commented 1 year ago

@MZ-Dlovely could you test HMR with the current repo if it works for you as intended ? using the default AcceptHMRUpdate function provided by pinia? If it works, i'll make a release :)

i'm sorry that i just saw your msg. have you tested for it? what should i do for it more? XD

Just take the latest commit of the repo, and try HMR the way Pinia recommends it (using their AcceptHMRUpdate function), and tell me if it behaves as you would expect it to :)

MZ-Dlovely commented 1 year ago

Just take the latest commit of the repo, and try HMR the way Pinia recommends it (using their AcceptHMRUpdate function), and tell me if it behaves as you would expect it to :)

i did find no bug by my tests in development mode, using pinia's AcceptHMRUpdate. if there is some bug arisen, tell me pls. 👍

prazdevs commented 1 year ago

Ok then, i'll issue a release then and hopefully close this issue

prazdevs commented 1 year ago

🎉 3.2.0 is released. This should now be fixed (by #203) and we can finally open that long running issue ! Thanks @MZ-Dlovely and everyone involved in it !