Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep action that created state change #233

Open
vincerubinetti opened this issue Jul 20, 2019 · 4 comments
Open

Keep action that created state change #233

vincerubinetti opened this issue Jul 20, 2019 · 4 comments

Comments

@vincerubinetti
Copy link

vincerubinetti commented Jul 20, 2019

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 vincerubinetti changed the title Store action that created state change Keep action that created state change Jul 20, 2019
@omnidan
Copy link
Owner

omnidan commented Jul 21, 2019

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

@vincerubinetti
Copy link
Author

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.

@omnidan
Copy link
Owner

omnidan commented Jul 22, 2019

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

@vincerubinetti
Copy link
Author

vincerubinetti commented Jul 22, 2019

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.

@nmay231 nmay231 mentioned this issue Feb 20, 2020
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants