search

vuejs / pinia

🍍 Intuitive, type safe, light and flexible Store for Vue using the composition api with DevTools support
https://pinia.vuejs.org
MIT License
12.72k stars 996 forks source link

acceptHMRUpdate skips optional attributes without default values #2611

Open bodograumann opened 4 months ago

bodograumann commented 4 months ago

Reproduction

https://github.com/bodograumann/pinia-hmr-optional-attributes

Steps to reproduce the bug

  1. git clone https://github.com/bodograumann/pinia-hmr-optional-attributes
  2. cd pinia-hmr-optional-attributes
  3. npm install
  4. npm run dev
  5. xdg-open http://localhost:5173
  6. Press the "Increment" button. Sign says "positive".
  7. touch src/store/counter.ts

Expected behavior

All the state should be preserved.

Actual behavior

counter.nr is preserved as 1, but counter.sign is lost. The page shows -, while it should show positive.

Additional information

The relevant code is here: https://github.com/vuejs/pinia/blob/93b5546cf18bc54bb90de2397219dec7360fa697/packages/pinia/src/hmr.ts#L41-L43

posva commented 4 months ago

The problem is that the removal of the property sign could be intentional so I'm not sure if this is with changing as it would break other HMR cases.

In order to change this behavior, one could get each property with toRaw(newStore.$state) and check if the property is a ref in order to completely replace it rather than patchObject

bodograumann commented 4 months ago

Sounds to me like your proposed solution would keep the other use case of removing a property intact, right? I think it should also work for us. Normally I would be happy to contribute a PR, but given that my previous PR is still open and we are thus unable to use the latest pinia version, I am not sure how to proceed here :neutral_face:

posva commented 4 months ago

I do need to check that PR. Currently we won’t be able to release a new version of pinia soon because of other changes 🥲