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

Add cache layer to axios #1031

Merged
merged 6 commits into from
Sep 27, 2024
Merged

Conversation

yaodingyd
Copy link
Collaborator

Should be merged after #1028 to validate

When investigating #1026 I found out there is no existing cache layer on axios so there's a lot of duplicate calls:

For example when in site page, there are four identical calls to latest_data
Screenshot 2024-09-21 at 10 39 53 PM

with axios-cache-interceptor in place, we only have unique calls
Screenshot 2024-09-21 at 10 40 30 PM

the default cache TTL is 5min which looks good. I don't think the backend data will change in a 5min window.

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

✅ This pull request was sent to the PullRequest network for review. Expert reviewers are now being matched to your request based on the code's requirements. Stay tuned!

What to expect from this code review:
  • Comments posted to any areas of potential concern or improvement.
  • Detailed feedback or actions needed to resolve issues that are found.
  • Turnaround times vary, but we aim to be swift.

@yaodingyd you can click here to see the review status or cancel the code review job.

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

PullRequest Breakdown

Reviewable lines of change

+ 18
- 5

44% TypeScript
30% TypeScript (tests)
22% TSX
4% JSON

Generated lines of change

+ 24
- 0

Type of change

Minor Update - These changes appear to be a minor update to existing functionality and features.

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

I have no concerns with this pull request.

Image of Jacob Jacob


Reviewed with ❤️ by PullRequest

@ericboucher
Copy link
Member

Nice addition @yaodingyd. You might need to mock caching in the tests to turn the CI green.

@yaodingyd
Copy link
Collaborator Author

tests are fixed

@ericboucher ericboucher merged commit b9fa835 into aqualinkorg:master Sep 27, 2024
3 checks passed
@yaodingyd yaodingyd deleted the add-cache branch October 1, 2024 14:24
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