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

caching only while fetching new records #39

Merged
merged 1 commit into from
Sep 22, 2024
Merged

caching only while fetching new records #39

merged 1 commit into from
Sep 22, 2024

Conversation

gokulk16
Copy link
Owner

@gokulk16 gokulk16 commented Sep 22, 2024

Summary by Sourcery

Refactor the service worker to cache network responses immediately after fetching, removing the pre-caching of specific URLs during the install event. Introduce an activate event to clean up old caches.

Enhancements:

  • Implement caching of network responses immediately after fetching to improve performance and reduce unnecessary network requests.

Copy link

sourcery-ai bot commented Sep 22, 2024

Reviewer's Guide by Sourcery

This pull request modifies the service worker (sw.js) to implement a more efficient caching strategy. The changes focus on caching responses only when fetching new records, rather than pre-caching a list of URLs during installation. The new implementation also includes cache cleanup on activation.

File-Level Changes

Change Details Files
Removed pre-caching of specific URLs during service worker installation
  • Removed the urlsToCache array containing specific URLs to cache
  • Removed the 'install' event listener that pre-cached the URLs
sw.js
Implemented a new caching strategy in the 'fetch' event listener
  • Added logic to check for cached responses before fetching from the network
  • Implemented caching of valid network responses after fetching
  • Added checks for response validity before caching (status 200 and type 'basic')
  • Implemented cloning of network responses to allow caching and returning the original response
sw.js
Added cache cleanup functionality in the 'activate' event listener
  • Implemented a cache whitelist to determine which caches to keep
  • Added logic to delete old caches that are not in the whitelist
sw.js

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

Copy link

netlify bot commented Sep 22, 2024

Deploy Preview for typetocalculate ready!

Name Link
🔨 Latest commit 77d2951
🔍 Latest deploy log https://app.netlify.com/sites/typetocalculate/deploys/66efed6a90ea380008f118f4
😎 Deploy Preview https://deploy-preview-39--typetocalculate.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 60.29%
⬇️ -1.05%
700 / 1161
🔵 Statements 60.29%
⬇️ -1.05%
700 / 1161
🔵 Functions 62.68%
🟰 ±0%
42 / 67
🔵 Branches 73.68%
🟰 ±0%
98 / 133
File Coverage
File Stmts % Branch % Funcs % Lines Uncovered Lines
Changed Files
sw.js 0% 0% 0% 0% 1-50
Generated in workflow #59

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @gokulk16 - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Consider a hybrid approach: pre-cache critical assets in the install event, while using runtime caching for other resources. This maintains offline-first benefits while optimizing cache storage.
  • Improve error handling in the fetch event. Currently, error responses are cached. Consider adding more robust checks and fallback strategies for network failures.
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

@gokulk16 gokulk16 merged commit c670cbf into main Sep 22, 2024
8 checks passed
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.

1 participant