Add feature to view if it can undo or redo

[vixalien]

I was thinking if it would be possible to add apis to check if it can undo or redo. I was thinking maybe: canUndo and canRedo which would return true or false. it would be nice to also store history change itself like: history

store.history()
// returned object:
{
  undo: {
    depth: 3,
    changes: [
      "bears",
      "bears.females",
      "allPopulation"
    ],
    // same for redo
}

Of course this is a proposal and any comment is welcome.

Or we could even attach the apis to undo and redo directly like so: undo.history or undo.depth or undo.canUndo.

[charkour]

This is an interesting proposal. Thanks for sharing. I'll think about it over the next week and respond with my thoughts so we can discuss the best updates for the API.

Thanks for suggesting this!!

[vixalien]

you can decide if this is needed and decide which way it will need to be implemented. After deciding, I can create a PR myself.

[charkour]

Sounds good.

[charkour]

Hey @vixalien, sorry for the delay. I like your suggestion but I don't completely understand everything you are suggesting.

What information will story.history() return? And how would you use this inside of your web app? Thanks.

[vixalien]

I was thinking that maybe history returns past state (that can be undo-ed into).

Here is an example from Microsoft Word:

The history shows the past state (like state changed because: "You typed" or "You boldened some text"). I don't think it would be necessary to store why state changed (i.e which properties changed) but i think it would be nice to show past history nevertheless.

Please forgive my very bad wording; my mind ain't right rn and I can't seem to fix it

[charkour]

Okay, yeah I think that makes sense. The history function would return all of the past states. That can be done!

Do you want to make a PR for that?

We can make a new discussion to talk about the canUndo and canRedo.

[cyango]

Hi, this is a interesting feature and makes total sense. Is it implemented already?

[charkour]

Hey @dotesfera,

It's not currently implemented. Would you like to make a PR? I'm not sure how soon I can get to this.

[vixalien]

Hello. I was trying to implement a PR but I think this feature is not necessary because of the following different statements:

1. First, canUndo and canRedo can be easily implemented by checking the length of state.prevStates and state.futureStates respectively. That is, canUndo can be implemented by !!state.prevStates.length (The !! basically converts the value into a boolean, or you can do state.prevStates.length>0.
2. Second, history is the merger of prevStates and futureStates.

I'm kind of disappointed that the library stores the states as snapshots instead of diffs (but Git does this too so yeah). I think I saw somewhere that you wanted to implement diffs instead of snapshots so as a starting point you can use microdiff, although you are on your own while implementing a patching algorithm.

I can also recommend that you use lodash.pick and lodash.omit in src/utils>filterState instead of rolling your own methods.

[charkour]

Hey @vixalien,

Thanks for the feedback! In v2 there will be support for using diffs or snapshots. I agree that snapshots can be a limitation.

Also, as for filtering, I'd like to not use third party dependencies, but you as the library consumer will be able to pass your own equality function which can be lodash.pick or one you hand rolled yourself. Kinda like how React.memo has a default equality function but you can also pass your own. Zustand does this as well.

[vixalien]

Nice. Looking forward to v2. Giving the user their own choices is a good idea.

Nice work!!!

[charkour]

Thanks. I appreciate the support. Wishing I had more time to devote to this project.

[feuersteiner]

@vixalien any status on the PR you wanted to make ? even though it can be "computed", a library should makes it's user's life easier (aka provide the properties directly). can we work together to make this PR ? cc @charkour

concerning the diffing, if more people start using zustand, more people will start using zundo, and thus more contributors will come. For now it's us, so let's make this library better together 😉

[vixalien]

wow it's been almost a year since i opened this issue. i don't use zundo much now so i can't think clearly on the best implementation. do you got ideas??

[charkour]

We can add a computed property in v2!

Hopefully in the next beta release or shortly after. You can try out v2 right now with npm i zundo@beta. It's 900 Bytes instead of the ~10 kB it was before.

[charkour]

Hey @vixalien and @feuersteiner,

As far as the canUndo, canRedo, undoDepth, redoDepth functionality, since those are computed values, I don't think it is necessary to add the to main temporal store in the zundo middleware. However, I think a meta-data helper function would be useful to add so I'll work on that.

For the time being, here is a code sandbox canUndo, canRedo, undoDepth, redoDepth that implements canUndo, canRedo, undoDepth, and redoDepth with zundo v2.

Thanks for the feedback!

[skrat]

If I only use the .temporal store in my component, it doesn't get re-rendered when I call undo() or redo() because pastStates and futureStates are not changing identity (not 100% about this), they're simply being changed in place with push and splice.

It works in your demos, because you have everything in one component, but if there's a component that is using just the temporal state, it breaks.

[cyango]

Same here

[charkour]

Ah, that's good to know. I'll look into fixing this issue. Thanks!

[skrat]

@charkour demo here https://codesandbox.io/s/zundo-forked-xf0t5p?file=/src/App.tsx

[charkour]

@skrat, thank you! This is helpful.

I completely forgot about this edge case, and you're right that the pastStates and futureStates are not re-rendering the comments because they are not changing identity.

[charkour]

I think the code needs to be updated to call the set function in https://github.com/charkour/zundo/blob/main/packages/zundo/src/temporal.ts inside the functions undo, redo, clear, and __internal.handleUserSet.

It might be a couple of days to confirm this theory. You're welcome to try it out if you're interested!

[skrat]

@charkour maybe, but IMO it won't suffice, you simply need to change pastStates and futureStates identities, so it's either some form of [...pastStates, nextState] or wrapping them in an object that changes identity, eg. { states: pastStates } or some such. My 2 cents.

[charkour]

Ah yes, I think you're right. So inside of those functions, set needs to be called while also returning a new object as you said.

That would likely work, and in the future, we can find a way to force a re-render because we don't want to constantly be generating new objects. It will slow down the JS with garbage collection.

[charkour]

I have a fix for this and can publish it later tonight!

[charkour]

@skrat, a fix to your bug was released in v2.0.0-beta.6

I'll leave this PR open for now since it is slightly different than the one you reported.

[skrat]

@charkour yay, fixed the issue, thank you

[charkour]

Closing this because canUndo and canRedo is something that should be added from userland.

Here is an example.