captbaritone / raven-for-redux

A Raven middleware for Redux
295 stars 25 forks source link

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

Closed ndbroadbent closed 6 years ago

ndbroadbent commented 6 years ago

See: #95, and https://github.com/captbaritone/raven-for-redux/pull/95#issuecomment-429611599

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 commented 6 years ago

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%> (ø) :arrow_up:

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 commented 6 years ago

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 commented 6 years ago

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