Call next only on when not emitting
Closed this issue · 4 comments
Hej,
loving the simplicity of your package, it was really easy for me to spot the proposed code change 👍
The issue is, that when your middleware is matching an action type, the next handler still gets called.
Is this intended behavior?
I'd like to have at least an configuration option, whether it should act like an interceptor or passthrough, so that the client-side "replay" that redux offers still works.
Currently it would emit all server actions again, what in some use-cases might be wanted, but in others may not.
I currently face such situation and thus would offer a PR in case that's something you're up to.
(it's a bit redux-thunk style, where you can whether swallow an action or emit something)
Yeah, this was a big debate for me... should it swallow the actions or continue to pass them down the middleware chain. For my use cases, I like that it continues through and I can write a reducer that handles it. It's a good way to implement an optimistic update, where you send the action to socket.io and to the reducer both, so that the data is updated immediately and then might be updated again when the server sends a socket.io message back. It's also nice because you can see the action in the redux devtools.
I do understand that there are cases where it might be nicer to swallow the action and not continue with next, so perhaps making it configurable is the right idea. I'd be open to a PR that does this, so long as the default remains that it sends the action through the reducer chain.
@itaylor The Optimistic update issue is probably the biggest question for all, even major promise handlers handle them separately. your package saved me 2 hours of work on the drawing board (:
@darkyen Did you get a chance to check out the Readme.md on the separateExecuteFunction branch?
It has my proposed new syntax with an example that shows how to use it to do both optimistic and pessimistic updates.
https://github.com/itaylor/redux-socket.io/tree/separateExecuteFunction
I'm thinking I'm going to merge that to master pretty soon, and I'd like to know if it meets your use cases (as I think others will share the same use cases you have)
I (finally) pushed these changes to master with version 1.3.0.