openapi-client
openapi-client copied to clipboard
mikestead
Feature request: dispatch an _ERROR action if there was an API error
You're going to love me @mikestead .
When I've used Redux before, one of the starters that I used had a really nice model, that was, when making AJAX requests, to dispatch _REQUEST, _SUCCESS and _FAILURE actions depending on what happened in the api service. Originally I thought that this was daft, but actually, you want to be able to handle errors in different kinds of requests differently.
It looks like it should be a pretty easy change as you'll just have to add a .catch() on the dispatch around here – and dispatch a new action, such MY_ACTION_NAME_ERROR. If I'm not mistaken, makeFetchRequest already returns a fetch promise, which should already reject if there's an error, which means that there shouldn't be too much working through the entire codebase.
Are my assumptions above correct? If so, would you accept a PR for that?
(Disclaimer – I don't know TypeScript, but this looks like a pretty vanilla JS change anyways)
Oh and obvs I could do this in my reducer, but I think it's tidier to do all the main switching on actions.
@shankiesan I can see the angle you're coming from but I'm not totally sure about this one.
As we always dispatch a FSA you can add middleware to inspect all actions for errors. From there you could siphon off more specific actions as required.
An aim here is to give some basic wiring but to also keep things light and not get too opinionated around how Redux has been setup. Not super easy, but was one of the reasons I went with FSA as it was well established.
Fair enough @mikestead . As you say, adding middleware to inspect all actions for errors is a good idea. I'll run with that. Cheers
The middleware/api actions in the official real world Redux example uses the _REQUEST, _SUCCESS and _FAILURE pattern. For my part I think this is more idiomatic Redux / best practice than the FSA format, which I haven't really seen get wide spread adoption in the wild.
https://github.com/reactjs/redux/blob/master/examples/real-world/src/actions/index.js
Just my 2 cents worth :)
Hey @mikestead I only just noticed @jedrichards comment. This is basically my feeling as well. I appreciate the FSA approach, but I think the idiomatic Redux approach would be more suitable.
How about a CLI flag for the action type? Obviously this is a major change, but something like --action-type=FSA or --action-type=redux would be pretty sweet.
@shankie-san @jedrichards Thanks for the feedback. Yeah a flag to opt into it sounds good. I'll reopen the ticket.
I'd probably want to keep the current default to be backwards compatible. Don't have time to look at this anytime soon I'm afraid.
Yep that makes sense to leave the current as the default @mikestead
I'd be happy to help add this when we've got a new project that we're going to base on OpenAPI. Maybe @jedrichards and I could take a look at it? Anyways yes let's leave it open in the hopes that one of us has a project that makes it worthwhile.