Skip to content

Conversation

@artonge
Copy link
Collaborator

@artonge artonge commented Sep 6, 2022

This is so new requests can be made by the component

Signed-off-by: Louis Chemineau [email protected]

@artonge artonge added bug Something isn't working 3. to review Waiting for reviews labels Sep 6, 2022
@artonge artonge added this to the Nextcloud 25 milestone Sep 6, 2022
@artonge artonge self-assigned this Sep 6, 2022
@artonge
Copy link
Collaborator Author

artonge commented Sep 6, 2022

/compile amend /

This is so new requests can be made by the component

Signed-off-by: Louis Chemineau <[email protected]>
Signed-off-by: nextcloud-command <[email protected]>
@nextcloud-command nextcloud-command force-pushed the fix/abort_controller_route_leave branch from 86e35dc to 1ec13e4 Compare September 6, 2022 10:12
@marcelklehr
Copy link
Member

What if new requests are made while the route is changing, won't these new requests use the newly created abort controller then? Instead of being auto-cancelled? Would it make sense to add the recreation code to a routeEnter hook?

@artonge
Copy link
Collaborator Author

artonge commented Sep 6, 2022

What if new requests are made while the route is changing, won't these new requests use the newly created abort controller then? Instead of being auto-cancelled? Would it make sense to add the recreation code to a routeEnter hook?

Unsure how this could happen unless there is some code inside a beforeRouteLeave hook ?
The beforeRouteEnter hook is probably called just after beforeRouteLeave, so the issue would be the same for other cases.
Also, beforeRouteEnter is probable called on initial load, and it does not make sens to recreate the abort controller at that point.

And I think that keeping the recreation logic just after we call abort() make more sense. We recreate one just after we burned the current one.

@artonge artonge merged commit c4089ce into master Sep 6, 2022
@artonge artonge deleted the fix/abort_controller_route_leave branch September 6, 2022 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants