-
-
Notifications
You must be signed in to change notification settings - Fork 584
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
feat(cache): add duration
option
#3367
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3367 +/- ##
==========================================
- Coverage 95.77% 95.55% -0.22%
==========================================
Files 155 155
Lines 9310 9345 +35
Branches 2725 2742 +17
==========================================
+ Hits 8917 8930 +13
- Misses 393 415 +22 ☔ View full report in Codecov by Sentry. |
I agree with adding an "expiration" feature to the Cache Middleware. This implementation seems good. If the PR is ready, please ping me! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the great pull request! I've commented on two of them, please check them out.
The part using Date.now()
, I think we can test using vi.useFakeTimers()
. If we want to be careful about testing, we may add runtime_tests, but in that case we are not sure if we can mock Date.now()
cleanly or not.
Co-Authored-By: Taku Amano <taku@taaas.jp>
Co-Authored-By: Taku Amano <taku@taaas.jp>
This PR is almost complete apart from conflicts, but I have other ideas so it's on hold. The app.use(cache({
cacheName: 'my-app',
cacheControl: 'max-age=60',
respect: true, // Control cache by respecting headers like Cloudflare Workers
wait: true,
})) @usualoma |
closes #3340
WIP.
The author should do the following, if applicable
bun run format:fix && bun run lint:fix
to format the code