Skip to content

Automatically remove 'past' and 'future' history from redux-undo #96

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

Closed

Conversation

ndbroadbent
Copy link

See: #95, and #95 (comment)

The past and present arrays include full copies of the state whenever a user makes a change. It's not a problem in the browser because they share the same memory references, but it's a huge problem when you need to serialize that data. This library is already specific to redux, and redux-undo is the most common way of adding undo/redo functionality to a React/Redux application. So maybe this makes sense as a good default. Otherwise most redux-undo users will end up with 413 responses, or no state at all (when using the code in this PR (#95).)

@codecov-io
Copy link

codecov-io commented Oct 14, 2018

Codecov Report

Merging #96 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #96   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines          26     45   +19     
  Branches       10     21   +11     
=====================================
+ Hits           26     45   +19
Impacted Files Coverage Δ
index.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ab4e52...4c7c841. Read the comment docs.

@captbaritone
Copy link
Owner

Cool! I don’t think this deserves to be in the core library, but I appreciate that you took the time to post it so others could find it.

Maybe we could make the example directory into an examples directory with a few sub directories. Then you could include this as an example project. What do you think?

@captbaritone
Copy link
Owner

I'll close this for now. Feel free to reply if you think it should be reopened.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants