feat(universal-cache): add universal cache middleware package#1764
feat(universal-cache): add universal cache middleware package#1764lord007tn wants to merge 1 commit intohonojs:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 89ea559 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1764 +/- ##
==========================================
+ Coverage 91.71% 91.90% +0.19%
==========================================
Files 113 115 +2
Lines 3778 4164 +386
Branches 954 1067 +113
==========================================
+ Hits 3465 3827 +362
- Misses 281 302 +21
- Partials 32 35 +3
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:
|
|
Addressed the Codecov coverage gaps with additional targeted tests. What I added
Validation run
All passed locally. The new commit is: |
| throw new Error('Base64 encoding is not available in this runtime') | ||
| } | ||
|
|
||
| const base64ToUint8Array = (base64: string) => { |
There was a problem hiding this comment.
How about using decodeBase64 in hono/utils/encode?
https://github.com/honojs/hono/blob/b6f4bcdc2e991652cda78cb52626cb5c39406a82/src/utils/encode.ts#L25
Btw, you can use atob always. It's standard: https://min-common-api.proposal.wintertc.org/#api-interfaces
| return null | ||
| } | ||
|
|
||
| const arrayBufferToBase64 = (buffer: ArrayBuffer) => { |
There was a problem hiding this comment.
How about using encodeBase64 in hono/utils/encode?
https://github.com/honojs/hono/blob/b6f4bcdc2e991652cda78cb52626cb5c39406a82/src/utils/encode.ts#L15
| /** Default storage group for cached functions. */ | ||
| export const DEFAULT_FUNCTION_GROUP = 'hono/functions' | ||
| /** Default cache max age in seconds. */ | ||
| export const DEFAULT_MAX_AGE = 1 |
There was a problem hiding this comment.
1 is really proper? Isn't it too short for real users?
There was a problem hiding this comment.
By default, for our needs, we kind of bypass it by default. Can I change it to 60, maybe?
There was a problem hiding this comment.
Yes. 60 and writing about it in the docs seem good.
| return next() | ||
| } | ||
|
|
||
| const isRevalidateRequest = ctx.req.header(revalidateHeader) === '1' |
There was a problem hiding this comment.
Is it good design that any user agent can revalidate if it sends the request with the revalidate header like x-cache-revalidate: 1?
There was a problem hiding this comment.
currently we use a custom header ourselves, for that case
Maybe I need to mention that in the docs, that its recommanded to change the header name
or make the shouldRevalidate required so they can implement whatever gates they want here
There was a problem hiding this comment.
Adding shouldRevalidate and allowing users to define the logic is good for DX. And, I think "revalidate" should be optional. It makes safety-side.
There was a problem hiding this comment.
I don't think SWR works on Workers. The behaviors of the following code on Bun and Workers are different:
import { Hono } from 'hono'
import { cacheMiddleware } from '@hono/universal-cache'
const app = new Hono()
let handlerCallCount = 0
app.get(
'/api/data',
cacheMiddleware({
maxAge: 2,
swr: true,
staleMaxAge: 10,
}),
(c) => {
handlerCallCount++
console.log(`[handler] called #${handlerCallCount} at ${new Date().toISOString()}`)
return c.json({
count: handlerCallCount,
time: Date.now(),
})
}
)
app.get('/api/stats', (c) => {
return c.json({ handlerCallCount })
})
export default appd79c864 to
89ea559
Compare
Summary
This PR introduces
@hono/universal-cacheas a new third-party middleware package for Hono.It follows the direction discussed in
honojs/hono#3857:honojs/hono#3857
This middleware has been running internally in our production systems for a while. After validating it under real traffic and edge cases, we felt it was in a good place to open-source and get broader feedback on it.
Dependency Model
@hono/universal-cachedepends onunstorageas the storage abstraction.That gives us:
Major Features
cacheMiddleware()cacheFunction()cacheDefaults()getKey) and integrity (integrity/hash)serialize/deserializehooks for response and function entriesshouldBypassCache,shouldInvalidateCache,shouldRevalidate,validatevariessupport for cache key variationrevalidateHeaderkeepPreviousOn5xxbehavior for safer refresh failuresRuntime Notes
maxAgeis60workerd, stale middleware entries refresh synchronously instead of relying on background self-fetchTest Coverage
Added coverage for:
shouldRevalidategatingworkerdValidation
corepack yarn eslint packages/universal-cache/src packages/universal-cache/vitest.config.ts packages/universal-cache/vitest.workerd.config.ts --cache --cache-location .cache/.eslintcachecorepack yarn workspace @hono/universal-cache typecheckcorepack yarn workspace @hono/universal-cache test --runcorepack yarn workspace @hono/universal-cache test:workerd --runcd packages/universal-cache && bunx vitest run src/index.test.ts src/utils.test.tsRelated
Merge Note
Please merge this together with the docs PR above.
The author should do the following, if applicable
yarn changesetat the top of this repo and push the changeset