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

Filtered actions are inconsistent when followed by unrelated action dispatches #215

Closed
mrcoles opened this issue Oct 5, 2018 · 3 comments

Comments

@mrcoles
Copy link

mrcoles commented Oct 5, 2018

See this gist.

I noticed that filtered actions in redux-undo behave inconsistently when followed by actions that do not affect the state (tested in the 1.0.0 beta 9.9.7).

Run the example code in this gist via:

yarn install
yarn start

and then open the server it starts and view the developer JS console.

The action TOGGLE_FOO is supposed to get filtered. However, by dispatching the __DOES_NOT_CHANGE_STATE__ action, the state does actually change—it seems that _latestUnfiltered updates. Then after dispatching TOGGLE_BAR followed by an undo, it only undos the TOGGLE_BAR and we are left with the TOGGLE_FOO as if it were not filtered.

As a contrast, if you comment out the does not change state action dispatch, the single undo undoes both the TOGGLE_BAR and TOGGLE_FOO.

@HarelM
Copy link

HarelM commented Dec 3, 2018

I have a similar issue that can be seen here:
https://stackblitz.com/edit/angular-redux-undo-decorator-avn28p?file=src%2Fapp%2Fapp.component.ts
steps to reproduce:

  1. click on advance/regular - gets filtered and undo button does not get enabled - as it should.
  2. click on other which changes another part of the state and undo button becomes enabled.

full steps to reproduce:

  1. use a filtered action
  2. change part of the state that is not undoable
  3. the past of the undoable part of the state changes.

@mrcoles did you find a work around for this?

@HarelM
Copy link

HarelM commented Dec 3, 2018

If anyone is reading this, seems like using includeAction(...) instead of excludeAction(...) (with the proper actions of course) allows to work around this issue.

@nmay231
Copy link
Collaborator

nmay231 commented Dec 13, 2019

This is the expected behavior. See #173.

As an example, imagine the unfiltered __DOES_NOT_CHANGE_STATE__ was instead a SET_BAR_TO_FALSE. This would leave present unchanged ({foo: true, bar: false} => {foo: true, bar: false}), but you would want to record this to history since the action was unfiltered.

As @HarelM mentioned, you should use a whitelist filter like includeAction(...) to get your desired result.

@nmay231 nmay231 closed this as completed Dec 13, 2019
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

No branches or pull requests

3 participants