fix(loader): don't let a cached loader's inner invokes veto page caching

Draft
PE
18h ago
+125-2

Problem

A loader with cache = "stale-while-revalidate" and a non-null cacheKey runs handler(props, req, ctx) with the request's shared context on a cache miss (callHandlerAndCache). Loaders it calls through ctx.invoke build their FnContext from the same state.global, so they share the page's vary. If one of them is no-store, or returns a null key, it sets vary.shouldCache = false and the whole page goes no-store.

This only happens on the outer loader's miss. On a hit the inner loaders never run, so page cacheability flips with loader-cache state.

Real case (a VTEX storefront): a site loader relatedProductList (SWR, key = product slug) invokes vtex/loaders/legacy/productDetailsPage, intelligentSearch/productList (key is null when ctx.isInvoke) and legacy/productList (no cache export, so it defaults to no-store). Bots render deferred sections inline, so every bot PDP whose related-products entry is cold comes back no-store: about 19k requests/day at ~1.3s each of origin time on that one site. For human visitors, the same veto hits that section's /deco/render.

Change

  • RequestContext gets an optional vary.
  • callHandlerAndCache runs the handler inside RequestContext.bind({ ...active, vary: scoped }), and also passes { ...ctx, vary: scoped } to the handler itself.
  • fnContextFromHttpContext uses RequestContext.active()?.vary when present. Every loader resolved inside that async scope, invokes included, pushes keys and vetoes into the scoped vary. Debug entries still forward to the page's vary.debug, so ?__d still shows them.
  • Siblings running concurrently on the same request are outside the scope and keep the request's vary. This is not a save/restore of a shared boolean: a sibling vetoing while the cached loader is mid-flight still vetoes. There's a test for exactly that.
  • no-store, no-cache, null-key and cache-disabled paths are untouched.

Is it correct to cache the outer result when an inner loader said no-store?

I think so. The outer loader's output is written to the loader cache and served to every request on a hit, and no veto runs on a hit. The inner veto therefore never protected the data; it only made the single miss request no-store. The outer cacheKey already owns correctness (it's what other requests are served by). This change makes a miss behave like a hit and adds no new exposure: anything an outer loader leaks is already leaked on hits today.

I checked deco-cx/apps for cacheable loaders that invoke others (vtex/loaders/options/productIdByTerm, wake/loaders/productDetailsPage, wake/loaders/productListingPage). The wake PLP returns a null key when a partner cookie is present, so it stays on the uncached path, and its partnerAssociate call is an action, which never sets vary. Nothing in apps writes ctx.vary directly.

Inner vary keys are also scoped now. They were already only present on misses, so dropping them makes the page vary stable across hit and miss; the outer key is pushed as before.

Tests

blocks/loader.test.ts, which goes through loaderBlock.adapt with fake HttpContexts that share request state, the way the resolver does for invokes:

  • A cacheable loader invoking a no-store loader leaves page shouldCache true. This fails on main.
  • A sibling no-store loader that vetoes while the cacheable loader's handler is in flight still vetoes the page.
  • A no-store or no-cache outer loader still propagates the inner veto.

Full suite: 87 passed, 0 failed. deno check live.ts is clean, and deno fmt --check/deno lint are clean on the touched files. The 12 unformatted files in the repo were already unformatted on main.

Draft until it has been checked end to end on a site preview; results to follow in a comment.

Summary by cubic

Fixes page caching being vetoed by loaders invoked inside a cacheable loader's handler, so a miss now behaves like a hit.

Previously, a cacheable loader (stale-while-revalidate with a cacheKey) that invoked inner loaders via ctx.invoke ran them with the request's shared vary on a cache miss. If any inner loader was no-store or returned a null key, it set the page's vary to no-store—but only on that miss, because hits skip inner loaders. This made page cacheability depend on loader-cache state.

This change scopes each cacheable loader's handler run to its own Vary via RequestContext, so inner loaders push keys and vetoes into that scoped vary instead of the page's. Siblings running concurrently on the same request are outside the scope and still veto the page. Non-cacheable loaders (no-store/no-cache) propagate vetoes as before, and debug entries still surface in the page's vary. Adds tests covering the scoped behavior, the sibling mid-flight veto, and the unchanged non-cacheable paths.