-
-
Notifications
You must be signed in to change notification settings - Fork 62
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: Use a global AbortController #1162
Conversation
40d06b5
to
9649696
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I think only requests that fetch photos or albums should be canceled. They are read only requests and the only ones that can take a long time to load. The other ones are here to alter the DB, so they should not be canceled.
I would prefer for the views to keep the responsibility to cancel the requests, and not a central service. Using a mixin here might make more sense.
Mh, but then wouldn't we need a mixin and a cancelRequest property for every request we do? Because every request gets its own cancelRequest property to cancel on view destruction. It's a lot of manual book keeping. Update: Or do you mean putting only the cancelAll() call into a mixin, so views can decide whether they want to use it? |
141386e
to
ca0aef4
Compare
/compile amend / |
b20219f
to
d5e5909
Compare
…anges Signed-off-by: Marcel Klehr <mklehr@gmx.net>
d5e5909
to
02e9750
Compare
/compile amend / |
Signed-off-by: Marcel Klehr <mklehr@gmx.net> Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
02e9750
to
a5557cc
Compare
The rationale for this is that request code currently lives in components without any way to keep the code DRY.
Ideally, request code would live in vuex actions IMO, but then we lose the ability to cancel requests from views.
This PR removes logic from views that handles request cancellation and moves it to the router, which cancels requests upon route changes.
I've also introduced a global priority queue for requests to replace the semaphore business, which also would have to be added to every request otherwise.