redux-undo icon indicating copy to clipboard operation
redux-undo copied to clipboard

Keep action that created state change

Open vincerubinetti opened this issue 6 years ago • 4 comments
trafficstars

Is there a built-in way in this library to get what action created a particular state? I apologize if this is possible, but I can't find it anywhere in the documentation. I also can't find the action information stored anywhere in the state.

I want to implement an undo/redo button set that also has a description, eg "Undo toggle grid on". I feel like the expected way would be to have this information stored in the history, maybe like this:

{
  past: [
    {
      state: { /* state stuff */ },
      action: { /* redux action that created this state */ }
    },
    ...
  ],
  current: { ... },
  future: [ ... ]
}

I feel like this is an oversight if it's not built-in, as almost every software I've ever used that has undo/redo functionality also has descriptions along with it.

vincerubinetti avatar Jul 20 '19 22:07 vincerubinetti

that is currently not implemented. I am not sure if it should be done in this library. You could, for example, handle the undo action in your reducers and then additionally store some descriptions for the changes that happened.

There is also a library that stores only the actions, but then you have to define functions that "revert" each action to be able to undo/redo. Each solution obviously has its own trade-offs: https://github.com/intactile/redux-undo-redo

omnidan avatar Jul 21 '19 08:07 omnidan

Thank you for the swift reply!

I did look into that other library, but found that it didn't match my needs.

My workaround is to attach a field to every action's payload called something like description, and then have a reducer that sets a state variable called something like actionDescription that gets updated on every state change with the value of payload.description or a blank string if it's not defined. That way, the necessary information is just stored right in the state.

I believe that is what you were suggesting as well. It will luckily work fine in my case, with a bit of extra work. I'd keep this feature in mind if you ever do a big rehaul, as it seems pretty useful? I'm surprised no one has brought it up yet, given this library is so popular.

vincerubinetti avatar Jul 21 '19 16:07 vincerubinetti

@vincerubinettiv you are right, it would be interesting for certain use-cases, but we will have to think how to integrate it without breaking stuff for other users.

Most probably do not need this, it really depends on your use-case. I would definitely make it optional, ideally we find a way to be able to toggle it without having to change the way the state is accessed.

omnidan avatar Jul 22 '19 12:07 omnidan

Agreed. Maybe it could be something like this:

{
  past: [ ... ],
  current: { ... },
  future: [ ... ],
  pastActions: [ ..., { type: 'toggle_something', payload: { description: 'toggle something on/off' } }, ... ],
  futureActions: [ ... ]
}

Keep arrays of pastActions and futureActions in-sync/in-order with the regular past and future arrays. Not the prettiest thing in the world, but at least it wouldn't break anything.

FWIW I just ended up implementing my own middlware that just has all of the current state in the root level of the state object and then has the past and future arrays as sort of "reserved" variable names that can't be used. Then I just make sure to filter them out when pushing a state entry to past or future. Makes it so I don't have to add .current every time i want to access the state, too.

vincerubinetti avatar Jul 22 '19 13:07 vincerubinetti