feat(website): cacheable prop on Page to cache CMS-only pages - #1652
feat(website): cacheable prop on Page to cache CMS-only pages#1652igoramf wants to merge 2 commits into
cacheable prop on Page to cache CMS-only pages#1652Conversation
… cache CMS-only pages (a home built purely from website sections, no commerce loader) never get PAGE_CACHE_ALLOWED_KEY set, so the runtime never emits a public Cache-Control and they stay uncached. Commerce apps (VTEX) set that key from their middleware, but an app middleware is a resolver middleware and only runs when a block from that app is resolved. Add an opt-in `cacheable` boolean to the Page block. When enabled, the Page loader sets PAGE_CACHE_ALLOWED_KEY (same API/mechanism as the VTEX middleware), routing the page through the existing, guarded caching pipeline. Safe by construction: the runtime still forces no-store on a foreign Set-Cookie, vary.shouldCache=false, or a non-cacheable matcher/flag, and never overrides a Cache-Control already set by another block. Default (unset/false) is fully backwards compatible — no page changes behavior unless explicitly toggled. Verified: Page.cacheable.test.tsx (loader sets the key iff cacheable=true) plus deco's runtime/middleware.test.ts (key ⇒ public, guards ⇒ no-store). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Tagging OptionsShould a new tag be published when this PR is merged?
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe page loader accepts an optional ChangesPage cache opt-in
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@website/pages/Page.cacheable.test.tsx`:
- Line 14: Remove the unnecessary async modifier from the get mock function in
the cacheable page test, keeping its existing synchronous object return
unchanged so it satisfies Deno’s require-await lint rule.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f2768afd-499f-434f-95fe-6598d2559550
📒 Files selected for processing (2)
website/pages/Page.cacheable.test.tsxwebsite/pages/Page.tsx
…-await) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Problem
CMS-only pages (e.g. a home built purely from
websitesections, no commerce loader) are never CDN-cached.The runtime (
deco/runtime/middleware.ts→applyPageCacheDecision) is opt-in: it only emits a publicCache-Controlwhen a middleware setPAGE_CACHE_ALLOWED_KEYon the request bag. The only setter isapps/vtex/middleware.ts— and an appmiddlewareis composed as a resolver middleware (blocks/appsUtil.ts), so it only runs when a block from that app resolves. A page with no VTEX loader never triggers it →isPageCacheAllowed=false→ no publicCache-Control→ never cached.Fix
Add an opt-in
cacheable?: booleanto thePageblock. When enabled, the existing PageloadersetsPAGE_CACHE_ALLOWED_KEY(same API the VTEX middleware uses), routing the page through the existing caching pipeline.Enable Allow CDN caching on a public, non-personalized page (e.g. the home) to make it cacheable.
Safe by construction
The opt-in only lets the page reach the decision; the runtime guards still apply and override to
no-store:Set-Cookie→no-storevary.shouldCache === false(a loader withcache:"no-store"or null cache key) →no-storecacheable !== true(e.g. a non-cacheable A/B test) →no-storeCache-Controlalready set by another block (e.g. VTEXno-storefor a logged-in/segmented user) wins — the runtime only sets the public directiveif (!headers.has("Cache-Control")).So it never forces caching of a personalized response. Default (unset/
false) is fully backwards compatible — no existing page changes behavior unless explicitly toggled.Tests
website/pages/Page.cacheable.test.tsx(new): the real PageloadersetsPAGE_CACHE_ALLOWED_KEYiffcacheable=true; off/omitted leaves the bag untouched.deco/runtime/middleware.test.ts(existing, 22 passing): confirms key + clean ⇒public,isPageCacheAllowed=false⇒ headers untouched, and every guard ⇒no-store.Verified compatible with the versions FARM pins today (
@deco/deco@1.202.0exports the key;apps@0.158.9already uses it in the VTEX middleware).🤖 Generated with Claude Code
Summary by cubic
Adds a
cacheableprop to the websitePageto let CMS-only pages opt into CDN caching via the existing runtime pipeline. Public, non-personalized pages can now be cached; default stays off.New Features
cacheable?: booleanonPage; when true, the loader setsPAGE_CACHE_ALLOWED_KEY.Cache-Controlforceno-store.Refactors
Page.cacheabletest.Written for commit e67634d. Summary will update on new commits.
Summary by CodeRabbit
New Features
cacheablesetting.Tests