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

Fix repeated fetching in the API #176

Merged
merged 11 commits into from
Dec 13, 2023
Merged

Fix repeated fetching in the API #176

merged 11 commits into from
Dec 13, 2023

Conversation

tgolen
Copy link
Contributor

@tgolen tgolen commented Dec 8, 2023

I was finding a few problems recently that I've been fixing.

  1. I noticed that the network tab kept having (what seemed) infinite numbers of requests over and over
  2. The dashboard would flash an rerender a lot (especially when cmd+click on items)

This PR should fix both of those problems.

Tests

  1. Open the dashboard and watch the network tab for a few minutes. There should only be a dozen or so requests every minute when data gets re-fetched
  2. Hold down command and click on several items at the top of the dashboard
  3. Verify the page doesn't flash

@tgolen tgolen requested a review from a team December 8, 2023 16:45
@tgolen tgolen self-assigned this Dec 8, 2023
@melvin-bot melvin-bot bot requested review from grgia and removed request for a team December 8, 2023 16:46
@tgolen
Copy link
Contributor Author

tgolen commented Dec 12, 2023

@grgia bump for review please.

grgia
grgia previously approved these changes Dec 12, 2023
Copy link
Contributor

@grgia grgia left a comment

Choose a reason for hiding this comment

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

Code looks good and I tested locally and only get ~25 network requests each minute. Also, there is no flashing while command clicking.

The only suggestion I'd make is documenting a guideline about when to use this pattern- i.e. should it wrap every API call?

@tgolen
Copy link
Contributor Author

tgolen commented Dec 12, 2023

OK, added!

@grgia grgia merged commit 8f1f7ab into main Dec 13, 2023
3 checks passed
@grgia grgia deleted the tgolen-fix-repeated-fetching branch December 13, 2023 17:30
@tgolen
Copy link
Contributor Author

tgolen commented Dec 13, 2023 via email

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.

2 participants