fix(website): isolate Lazy fallback-discovery from page cache state - #1646
Open
igoramf wants to merge 1 commit into
Open
fix(website): isolate Lazy fallback-discovery from page cache state#1646igoramf wants to merge 1 commit into
igoramf wants to merge 1 commit into
Conversation
…che state
Lazy's loader resolves the selected section once during the loader phase
solely to discover its LoadingFallback and metadata. That resolution ran
against the parent page's request state (`resolveSection({}, ...)` spreads
the ambient context), so a deferred section's loaders/matchers could set
`vary.shouldCache = false`, push flags, or emit Set-Cookie on the page —
disqualifying the whole page from the shared CDN cache even though the
section is deferred.
Pages with many Lazy sections (e.g. a storefront home with ~70 deferred
sections, one of which reads a no-store loader such as cart/weather) were
served `Cache-Control: no-store` while PLP/PDP cached normally.
Resolve the fallback against an isolated request state (`bag`, `flags`,
`response`, `vary`) so speculative mutations are discarded. The eager
partial still re-resolves the section with the real request state, so
personalized sections stay personalized.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Tagging OptionsShould a new tag be published when this PR is merged?
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughChangesLazy resolution isolation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Storefront home pages (e.g.
www.farmrio.com.br/) were served withCache-Control: no-storewhile PLP/PDP cached normally(
public,max-age=180,stale-while-revalidate=3600). CDN full-page cachewas fully disabled on the highest-traffic page.
Root cause
website/sections/Rendering/Lazy.tsxdefers rendering of its childsection, but its loader resolves that child once up front just to
read its
LoadingFallbackandmetadata:The engine resolves a deferred value with
{ ...context, ...partialCtx }(
engine/core/mod.ts). Passing{}means the child's whole resolvesubtree runs against the parent page's request state — its
vary,flags,responseandbag. So any deferred section whose loader isno-store(or returns a null cache key) setsctx.vary.shouldCache = falseon the page (blocks/loader.ts), and the middleware then writesno-storefor the entire page (runtime/middleware.ts,applyPageCacheDecision).A home with ~70
Lazysections only needs one of them to touch aper-user loader (cart, weather, session, …) for the whole page to become
uncacheable — even though that section is deferred and its real render
happens later in a separate partial request.
Fix
Resolve the fallback against an isolated request state
(
newIsolatedRequestState()), covering exactly the four keys the enginethreads as request state (
bag,flags,response,vary— seeruntime/utils.ts). Speculative mutations during fallback discovery arediscarded. The eager partial still re-resolves the section with the real
request state, so personalized/deferred sections stay personalized.
Tests
website/utils/isolatedRequestState.test.tsasserts two isolated statesdon't share
vary/flags/response/bagand that side effects on onedon't leak to the other.
deno testpasses;deno checkclean on bothchanged files.
🤖 Generated with Claude Code
Summary by cubic
Fixes a bug where deferred sections in
Lazycould disable full-page caching by leaking per-user loader mutations into the page state. Fallback discovery now runs in an isolated request state, restoring CDN caching on homepages while keeping personalization intact.Lazyfallback withnewIsolatedRequestState()sobag,flags,response, andvarymutations don’t affect the page.Written for commit 0bb69c2. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests