-
Notifications
You must be signed in to change notification settings - Fork 264
RFC: Response cache #985
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
base: main
Are you sure you want to change the base?
RFC: Response cache #985
Conversation
🦋 Changeset detectedLatest commit: cc46026 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
c42ca1c to
b8b1987
Compare
|
@yusukebe Hy, could you check this out please? :) Still very basic, no tests even yet, just to get feeling/idea if there would be interest to continue with this |
|
At first glance, I don't think this is needed. We already have the built-in Cache Middleware. If you want to cache just "Response", it's enough to use it. |
|
As per my understanding, that middleware only works on cloudflare an deno, and uses specific cache mechanism. This allows little bit more freedom, for example, by using Also, correct me if I'm wrong, but that Cache API helps cache'ing it on Cloudflare/DenoDeploy level, basically "edge/cdn" rather than on server, right? |
|
Forgot to mention - this allows to customize cache’ing keys. So for example, if you’re running A/B test, and store A/B tests list in cookie - you can use that cookie to allow to cache page rendered under specific A/B conditions. Quite niche example, but solution is quite lightweight with provided flexibility. |
|
Thank you for the explanation! Makes sense.
Yes. The Cache API is standardized, but you can only use it in particular runtimes like Cloudflare Workers and Deno. This middleware looks good. Let's go ahead. |
|
redis - or for example Cloudflare KV.. @muningis are you planning on finishing this PR? |
|
Had completely forgoten about this. I will get back to this next week. |
b8b1987 to
f02320b
Compare
…der filtering - Store full response (body, status, headers) instead of just body text - Filter sensitive headers (Set-Cookie, WWW-Authenticate, etc.) - Remove `respond` as it's inferred from `Content-Type` header - Add vitest tests
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #985 +/- ##
==========================================
+ Coverage 84.35% 84.46% +0.11%
==========================================
Files 119 120 +1
Lines 3879 3907 +28
Branches 1047 1054 +7
==========================================
+ Hits 3272 3300 +28
Misses 516 516
Partials 91 91
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
If this is ready to review, please ping me. |
|
Hello @yusukebe, yes this is ready now. |
The author should do the following, if applicable
yarn changesetat the top of this repo and push the changeset