charkour / zundo

🍜 undo/redo middleware for zustand. <700 bytes
https://codesandbox.io/s/zundo-2dom9
MIT License
616 stars 20 forks source link

Current state not persisted / restored for arrays #121

Closed dahannes closed 1 year ago

dahannes commented 1 year ago

Great Library! I would like to use zundo with persist middleware as laid out in the readme. It's working fine for the provided bear counter example, but there seam to be issues with persisting arrays.

Can you confirm this is a bug?

Reproduce

  1. Open Sandbox https://codesandbox.io/s/zundo-forked-65pskw?file=/src/App.tsx
  2. Click "Add Bear!" three times
  3. Click "Reload"

Expected: It restores past and current states Actual: It restores past states, but sets current state to initial value

charkour commented 1 year ago

Thanks for the bug report and the reproduction steps! I hope to review this over the weekend, but if you have time, you could create a PR before hand.

dahannes commented 1 year ago

I just checked out the repo's persist example and realised it's wrapped with persist twice.

Using this structure, persisting arrays seams to work perfectly, but I doubt that api should be required for the use-case, right?


const useStore = create<Store>()(
  persist(                             // <---- persist 
    temporal(/*...*/),
    {
      wrapTemporal: (store) =>
        persist(store, {               // <---- persist 
          name: 'some-store-temporal',
        }),
      },
    ),
    {name: "some-store"},
  ),
);
charkour commented 1 year ago

haha wow, I'll need to update the example and fix that bug. You shouldn't need to wrap it twice, but I'm glad that's a workaround for now.

dahannes commented 1 year ago

ok great. I need to put this on hold for now, but I would look into it again next week if you don't get to fix it on the weekend already.

Generally though, how far from a stable release is the current beta?

charkour commented 1 year ago

It's not far, I just need to find time to write the migration docs. Maybe I'll release v2 before the migration docs are complete!

The core is well-tested and used in large projects (stable studio); the only edge cases appear to be related to using with other middlewares.

charkour commented 1 year ago

Hi @dahannes,

Thinking about this more, I believe it makes sense to have persist middleware in two places. The outer persist is persisting your userland store, and the inner persist, as part of the temporal options, will persist the temporal store that's created by the middleware.

Simply put: there are two zustand stores, so you must persist both.

Thanks for your question!