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

Unified request function #675

Open
matthew-white opened this issue Dec 5, 2022 · 0 comments
Open

Unified request function #675

matthew-white opened this issue Dec 5, 2022 · 0 comments
Labels
refactor Improves code without altering behavior requestData Changes to requestData resources

Comments

@matthew-white
Copy link
Member

At the moment, we send requests in one of three different ways:

  1. Using the request() method of a requestData resource
  2. Using a method from the request mixin
  3. Using http in the container directly (http is set to axios)

(1) and (2) are both common. (3) is uncommon and maybe only happens for the logout request.

(1) and (2) do a lot of the same things, for example, around error handling, so they have some near-duplicate code. That's mostly a historical artifact, reflecting that things like showing an alert worked differently within the Vuex store vs. within components. We no longer have a Vuex store, and everything can now use the container to access app-wide state, so I think we're now well positioned to move that shared logic into a function that both requestData and the mixin can call. Once we have a function like that, cases like (3) could also call it.

I still think it makes sense to have multiple ways of sending requests. requestData resources have one way of storing request state, but components need a separate mechanism. And requests sent by resources vs. requests sent by components have slightly different lifecycles: they can be canceled in different ways. So the goal of this issue isn't to remove all distinctions between these different ways of sending requests, but rather to reduce duplicate code and to remove distinctions that aren't necessary. For example, we used to document that the request module of the Vuex store was strictly for GET requests, while the request mixin was for non-GET requests. However, requestData resources are now able to send non-GET requests, and I think the request mixin should be allowed to send GET requests as well. (Not that we should necessarily encourage that pattern given that components can now create local resources, which will usually be the most convenient approach.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Improves code without altering behavior requestData Changes to requestData resources
Projects
None yet
Development

No branches or pull requests

1 participant