-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
feat(StoreDevtools): implement actionsBlacklist/Whitelist & predicate #970
Conversation
You could add a new property |
Thanks for the PR @Wykks! As an alternative, I'm looking into sending all actions/states through the extension since it already has support for blacklist/whitelist/predicate and I don't think we need to duplicate all that on our end. |
👍 something like this will be nice ! |
@Wykks Any word/work on this? Happy to help get this through. |
Hi! Sorry for this very long silence... I totally got demoralized doing this PR. Just like Brandon said, this should be handled by the extension itself. But it doesn't work when sending the full state (with But, I really don't get why it's implemented like that in the extension. This should just be a visual thing, with some skip when moving through states. There's no actual need to filter the state like that... |
@Wykks apologies for getting behind on this one. Will you rebase on master? Handling it here may be the option for now. We may need to open an issue on the extension to at least get some clarification on filtering when full state is provided. |
@Wykks will you rebase this one? |
ops didn't saw the last message. I can do the rebase this weekend yep! |
👍 |
Rebased and updated! |
@Wykks will you rebase again? Thanks |
Done, sorry for the delay.. |
🎉 |
Really nice addition. In what version will this be included in the NPM package? |
It will become available in the next version (7). |
Ref: #938
Note: the unit tests doesn't test jump / slider (I'm not sure how to test that).
There an issue when using the slider (and the action filter feature).
The app doesn't get the right state (whereas the extension show the correct state).
To reproduce in the example app, add
actionsBlacklist
:Do two different search, then, jump or slide back to the previous
Search Complete
.Sorry for leaving this issue here, I spend more than half a day figuring out how this works... This is as far as I'm able to do, for now... I'll try to investigate more If needed, but you probably already know what's going on 😅
Edit: Ok I see what's going on. Since I filter the
liftedState
only for thedevtoolsExtension.send
, thecurrentStateIndex
send by the extension is desync with the internalliftedState
.I could actually filter-out the state outside
extension.notify
but according to: zalmoxisus/redux-devtools-extension#316 (comment) , the internalliftedState
should not be filtered.Edit 2 : Welp, I guess I should filter the state outside
extension.notify
after all. It works fine. But I'm a bit puzzled about how to "properly" code this. I'll update the PR next time (maybe tomorow).