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 + redux-devtools-log-monitor displays states from incorrect actions #4

Closed
thomasboyt opened this issue Jan 14, 2016 · 4 comments

Comments

@thomasboyt
Copy link

I'm working on a game using redux. The game has its own run loop that dispatches a TICK action inside a requestAnimationFrame loop, so I was planning to use this component to filter that action.

I was surprised to see that when I triggered a non-tick action, causing an entry to display in the devtools monitor, the state that was reported seemed to be stale, as if it was computed from one of the first tick actions. Investigating the source of redux-devtools-log-monitor and redux-devtools-filter-actions, I quickly saw the problem:

  • In FilterMonitor.js, the stagedActionIds array is filtered to remove blacklisted/include whitelisted actions.
  • In LogMonitor.js, the stagedActionIds array is iterated over. The index of this iteration is used to look up the corresponding state from the computedStates passed from redux-devtools. However, computedStates contains all of the computed states, including blacklisted actions, meaning there's no way that this lookup will align with the filtered action IDs!

I'm not sure what the fix here is. I don't currently see any way to look up the correct computedState without having the index of the action. Maybe a map of action ID to state could be added to the props passed down from redux-devtools? A map of action ID to original index could also work.

@thomasboyt
Copy link
Author

for now, I've solved this with a fork of redux-devtools-log-monitor: thomasboyt/redux-devtools-log-monitor@35be431

@zalmoxisus
Copy link
Owner

Hey @thomasboyt,

Your solution is the same as it was here before extending it to a wrap monitor. The reason we switched from that is to support different monitors and to have new versions of log monitor supported without maintaining a fork.

I figure filtering computedStates along with stagedActionIds will solve it.

@zalmoxisus
Copy link
Owner

Fixed in v1.0.1. Thanks for reporting this! Feel free to reopen the issue if something goes wrong.

@thomasboyt
Copy link
Author

Awesome, thanks for such a fast response! Fix seems to work perfectly :)

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

2 participants