Truly batch actions with batch middleware #34
Conversation
|
Hi @nbgraham , I've not used Redux in years and consequently have totally neglected this PR. There was also some BS about a pandemic, etc... Anyway, I'd like to give you commit access to this repo so that the code can stay alive. Let me know if interested. |
|
@nbgraham so, i just forked this to make it work with redux 5/rtk 2, and i thought i'd try out this change with it as well. I merged your change into my fork, and batchActions that are dispatched never hit. Am I misunderstanding something here, or is this just broken? Inspector shows it handling the BATCHING_REDUCER.BATCH action, but it doesn't show the actual batch being dispatched.
|
|
ok, so... i dug into what this is doing and... no, you're not allowed to do that, and it doesn't work. If you don't call next() for each relevant thing, then none of the reducers or middleware fire, so none of the data store gets updated, because the updates happen in the reducers. The whole idea is incorrect. What you can do is not call next() for the item we are actually processing, ie, the batched request. Then that does not show up in the inspector, and reduces (har har) the number of reducer calls here by 1. You cannot reduce the number of calls TO 1. only BY 1. And that's if you want to make the assumption that you'll never need any other middleware or reducer to actually pay attention to the batch actions. That seems like a safe enough assumption, though. But if someone installs a middleware before the batch handler, then that can be done anyway. |
Fixes #23
Also, check out the discussion around this on the original batch middleware PR: #18
Problem:
Calling
store.dispatchsends the action through all the middlewares again and updates the store.So because the batch middleware dispatches each bundled action, the reducer receives each one and the state updates each time.
When using batch middleware, the store updates n times for a batch action with n actions, instead of 1 time.
Solution:
Whenever the batch middleware dispatches one of the bundled actions in a batch action, it marks it as
unwrappedFromBatch. When this action comes back through the middlewares, the batch middleware looks for theunwrappedFromBatchflag and, if present, does nothing (does not callnext, so it does not reach later middlewares nor the reducer/store to update).I have used this approach with redux-saga successfully.
Tests Update
I added "integration tests" to test the behavior of the middleware in a real redux store.
At first glance, I thought the existing code would not send each bundled action to the reducer, because of this test:
However, this test is for the middleware in isolation, not in a redux store. So I added tests for the middleware behavior in a real redux store.