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

Refactor payments-server API calls to use axios-hooks rather than Redux? #3526

Open
lmorchard opened this issue Nov 28, 2019 · 4 comments
Open

Refactor payments-server API calls to use axios-hooks rather than Redux? #3526

lmorchard opened this issue Nov 28, 2019 · 4 comments

Comments

@lmorchard
Copy link
Member

@lmorchard lmorchard commented Nov 28, 2019

In payments-server, we use Redux with promise & thunk middleware to handle HTTP API calls & call sequences. Redux actions & state track them through pending, success, and failure states. This results in a lot of paperwork for each API call - actions to trigger & track calls, state properties to manage call status, etc.

I started reading about axios-hooks recently - and it looks like this library could handle just about everything we're doing for API calls with Redux but with significantly less paperwork. It also mostly fits the call patterns we're already using in components.

We could probably reduce the whole API-handling system down to one module exporting prefab per-call hooks. Maybe use AppContext to share around globally-used data like plans, subscriptions, etc. We might even be able to excise Redux from the project altogether, since we haven't ended up using it for anything but this API-tracking function. (Well, that and also triggering some metrics pings.)

@lmorchard

This comment has been minimized.

@LZoog

This comment has been minimized.

Copy link
Contributor

@LZoog LZoog commented Dec 3, 2019

With how we're using Redux in payments, I do think this could really simplify things. I've used axios (not axios-hooks) and found it easy to use, and I once interviewed someone that raved about replacing their store completely with Context so this is a good thing to keep on our radar. I suppose we just need to consider if/when we could need Redux in the future.

@lmorchard

This comment has been minimized.

Copy link
Member Author

@lmorchard lmorchard commented Dec 3, 2019

Yeah, we could also replace the API handling with axios and keep Redux around for what it's good for. The current system of API calls through Redux seemed (to me) like a good idea at the time, but now it honestly looks a bit burdensome since seeing something like axios-hooks.

Since hooks arrived in general, we could replace a lot of Redux functionality with useReducer + useContext. Though, we'd lose things like easy middleware and Redux dev tools.

@lmorchard

This comment has been minimized.

Copy link
Member Author

@lmorchard lmorchard commented Dec 4, 2019

Poking at this a bit today and I realized one thing that axios-hooks is missing: There's no way to reset the fetch state. So, if we POST to an API and get an error result, there's no way (as far as I can tell) to reset the request state on error dialog dismiss to try again.

But, this might still be a good idea, even if we have to write our own local version of a fetch hook.

lmorchard added a commit to lmorchard/fxa that referenced this issue Dec 5, 2019
lmorchard added a commit to lmorchard/fxa that referenced this issue Dec 6, 2019
lmorchard added a commit to lmorchard/fxa that referenced this issue Dec 6, 2019
lmorchard added a commit to lmorchard/fxa that referenced this issue Dec 6, 2019
lmorchard added a commit to lmorchard/fxa that referenced this issue Dec 6, 2019
lmorchard added a commit to lmorchard/fxa that referenced this issue Dec 7, 2019
lmorchard added a commit to lmorchard/fxa that referenced this issue Dec 10, 2019
lmorchard added a commit to lmorchard/fxa that referenced this issue Dec 10, 2019
lmorchard added a commit to lmorchard/fxa that referenced this issue Dec 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.