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

Abort large number of fetch requests #435

Merged
merged 1 commit into from
Dec 28, 2021

Conversation

csc-felipe
Copy link
Contributor

Description

Loading tags is unnecessary when navigating to another page, and it
keeps browser resources busy. This change cancels those requests when
browsing away.

Related issues

Type of change

  • New feature (non-breaking change which adds functionality

Changes Made

Container and object view can cancel loading of tags when moving away of the page using an AbortController.

Testing

  • Tests do not apply

Mentions

@csc-felipe csc-felipe changed the base branch from master to devel December 22, 2021 16:10
@csc-felipe csc-felipe changed the title Feature/abort unnecessary fetch Abort large number of fetch requests Dec 22, 2021
@csc-felipe csc-felipe self-assigned this Dec 22, 2021
@csc-felipe csc-felipe force-pushed the feature/abort-unnecessary-fetch branch from 33d1686 to 3fc73e2 Compare December 22, 2021 16:36
@csc-felipe csc-felipe marked this pull request as draft December 23, 2021 08:14
@csc-felipe csc-felipe force-pushed the feature/abort-unnecessary-fetch branch from 3fc73e2 to cd837a1 Compare December 27, 2021 10:50
@csc-felipe csc-felipe marked this pull request as ready for review December 27, 2021 11:23
Loading tags is unnecessary when navigating to another page, and it
keeps browser resources busy. This change cancels those requests when
browsing away.
@csc-felipe csc-felipe force-pushed the feature/abort-unnecessary-fetch branch from cd837a1 to 18ef1f0 Compare December 28, 2021 10:38
Copy link
Contributor

@blankdots blankdots left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this sparks the idea of displaying a message, e.g. after 30 sec or so, that the loading time will take longer for this request and data is being processed and I assume this can be handled in a different PR.

Should we add this abortcontroller to errorCaptured https://vuejs.org/v2/api/#errorCaptured as well ?

otherwise seems ok.

@csc-felipe
Copy link
Contributor Author

There is an issue for showing long background ops #425.

I'm not sure if I understood what do you mean with the errorCaptured.

Copy link
Member

@sampsapenna sampsapenna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solves the issue with existing fetch calls preventing updating the new route view when etc. opening a container, approved. The experience was considerably faster now, with projects that have hundreds of buckets.

@blankdots
Copy link
Contributor

I'm not sure if I understood what do you mean with the errorCaptured.

Don't we need to abort the calls when an error is captured, or we will never have that case ?

@csc-felipe
Copy link
Contributor Author

Don't we need to abort the calls when an error is captured, or we will never have that case ?

That would be good if we know what kind of errors might happen. I think first we should be able to observe what errors people are having, so we don't have to guess.

@csc-felipe csc-felipe merged commit 6c67bdf into devel Dec 28, 2021
@csc-felipe csc-felipe deleted the feature/abort-unnecessary-fetch branch December 28, 2021 15:00
@sampsapenna sampsapenna mentioned this pull request Jan 12, 2022
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants