pinia-plugin-persistedstate icon indicating copy to clipboard operation
pinia-plugin-persistedstate copied to clipboard

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

Open Zehir opened this issue 2 years ago • 2 comments

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.

Zehir avatar Jun 21 '22 20:06 Zehir

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

prazdevs avatar Jul 27 '22 22:07 prazdevs

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

Zehir avatar Jul 27 '22 22:07 Zehir

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

prazdevs avatar Nov 26 '22 13:11 prazdevs

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

Zehir avatar Nov 26 '22 13:11 Zehir

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.

Zehir avatar Jan 17 '23 16:01 Zehir

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

prazdevs avatar Jan 17 '23 17:01 prazdevs

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 :

  • After a page refresh the data is still present.
  • In the file src/stores/demo.ts comment the line const testMethod = computed(() => 1) and then save.
  • The page reload but the state is lost.

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.

Zehir avatar Jan 17 '23 18:01 Zehir

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 avatar Feb 10 '23 21:02 prazdevs

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 avatar May 09 '23 19:05 prazdevs

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

prazdevs avatar May 09 '23 19:05 prazdevs

what about creating your own acceptHMRUpdate function ?

Zehir avatar May 09 '23 20:05 Zehir

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 avatar May 09 '23 21:05 prazdevs

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. 🤔

prazdevs avatar May 09 '23 21:05 prazdevs

Thanks I will try this when I have some time available

Zehir avatar May 09 '23 22:05 Zehir

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

MZ-Dlovely avatar May 10 '23 11:05 MZ-Dlovely

wait for me to write its workflow

MZ-Dlovely avatar May 10 '23 11:05 MZ-Dlovely

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

prazdevs avatar May 10 '23 11:05 prazdevs

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

MZ-Dlovely avatar May 10 '23 11:05 MZ-Dlovely

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

MZ-Dlovely avatar May 10 '23 11:05 MZ-Dlovely

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

MZ-Dlovely avatar May 10 '23 11:05 MZ-Dlovely

🤔 i get the best solution, maybe

MZ-Dlovely avatar May 10 '23 12:05 MZ-Dlovely

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

prazdevs avatar May 10 '23 12:05 prazdevs

i think we need. i'm doing for it

MZ-Dlovely avatar May 10 '23 12:05 MZ-Dlovely

i will check it a little time

MZ-Dlovely avatar May 10 '23 13:05 MZ-Dlovely

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

  • do we want to hydrate on hot reload with stored data
  • persist new store (and initial data), and consider stored data stale
  • we could even make that configurable maybe

prazdevs avatar May 16 '23 18:05 prazdevs

any update on this?

hi-reeve avatar May 26 '23 05:05 hi-reeve

Thank you. any update on this?

waney avatar May 29 '23 08:05 waney

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 :)

prazdevs avatar May 29 '23 19:05 prazdevs

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

MZ-Dlovely avatar May 30 '23 15:05 MZ-Dlovely

@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 :)

prazdevs avatar Jul 16 '23 15:07 prazdevs