fix(cms): defer below-fold sections on SPA navigation too (#277)

packages/blocks/src/cms/resolve.test.ts

@@ -23,14 +23,16 @@ vi.mock("./registry", () => ({
 import { normalizeUrlsInObject } from "../sdk/normalizeUrls";
 import { findPageByPath } from "./loader";
 import { getSection } from "./registry";
-import type { AsyncRenderingConfig, DeferredSection } from "./resolve";
+import type { AsyncRenderingConfig, DeferredSection, MatcherContext } from "./resolve";
 import {
   clearCommerceLoaders,
   DEFAULT_FOLD_THRESHOLD,
   extractSeoFromProps,
   getAsyncRenderingConfig,
   isEagerRequest,
+  reExtractRawProps,
   registerCommerceLoader,
+  registerMatcher,
   registerEagerSections,
   registerAlwaysDeferSections,
   registerNeverDeferSections,
@@ -736,16 +738,6 @@ describe("resolvePageSeoBlock — per-section ignoreStructuredData drives the fe
   });
 });
 
-// ---------------------------------------------------------------------------
-// resolveDecoPage — #277 client-side navigation disables deferral
-// ---------------------------------------------------------------------------
-//
-// When isClientNavigation is true, resolveDecoPageImpl sets useAsync = false so
-// shouldDeferSection is never called. All sections — including CMS ⚡-wrapped
-// ones — are resolved eagerly. This prevents client-nav from returning a
-// deferredSections array that loadDeferredSection would then try to resolve
-// without the per-request commerce app context.
-
 describe("extractSeoFromProps — commerce jsonLD structured data", () => {
   const plp = (overrides: Record<string, unknown> = {}) => ({
     "@type": "ProductListingPage",
@@ -888,22 +880,42 @@ describe("extractSeoFromProps — commerce jsonLD structured data", () => {
   });
 });
 
-describe("resolveDecoPage — #277 client-side navigation disables deferral", () => {
+// ---------------------------------------------------------------------------
+// resolveDecoPage — client nav gets the SAME eager/deferred split as SSR
+// ---------------------------------------------------------------------------
+//
+// A TanStack route loader is BLOCKING: the router will not commit the
+// transition until the loader promise settles. Eager-resolving every ⚡
+// below-fold section on client nav therefore freezes the previous page for as
+// long as the slowest upstream takes (measured on a real PDP: 20 awaited
+// sections, 2717ms, 3.41MB — worse than a full reload). So deferral must apply
+// to client nav exactly as it does to SSR.
+//
+// The historical `!isClientNavigation` gate (decocms/blocks#277) was a
+// workaround for deferred loaders that appeared to lose per-request app
+// context. It traded a page-wide latency regression for that symptom. The
+// second hop (`loadDeferredSection`) is the SAME server fn the SSR path has
+// always used and rebuilds MatcherContext from the real request — see the
+// "#277 — per-request context survives the deferred second hop" cases below.
+// ---------------------------------------------------------------------------
+
+describe("resolveDecoPage — deferral parity between SSR and client nav", () => {
   const lazySec = {
     __resolveType: WELL_KNOWN_TYPES.LAZY,
     section: { __resolveType: "site/sections/Hero.tsx" },
   };
+  const eagerSec = { __resolveType: "site/sections/Banner.tsx" };
 
   beforeEach(() => {
-    // Enable async rendering so useAsync can be true for SSR requests.
+    // Enable async rendering so useAsync can be true.
     setAsyncRenderingConfig({ foldThreshold: Infinity, respectCmsLazy: true });
     // resolveSectionShallow unwraps the ⚡ and looks up the inner key via
     // getSection — return truthy so it produces a DeferredSection rather than
     // falling back to eager resolution.
     (getSection as ReturnType<typeof vi.fn>).mockReturnValue({ default: () => null });
-    // Return a page with one CMS ⚡-wrapped section.
+    // A page with one plain section followed by one CMS ⚡-wrapped section.
     (findPageByPath as ReturnType<typeof vi.fn>).mockReturnValue({
-      page: { name: "test", sections: [lazySec] },
+      page: { name: "test", sections: [eagerSec, lazySec] },
       params: {},
       blockKey: "test-page",
     });
@@ -914,15 +926,178 @@ describe("resolveDecoPage — #277 client-side navigation disables deferral", ()
     (findPageByPath as ReturnType<typeof vi.fn>).mockReset();
   });
 
-  it("SSR request defers a CMS ⚡ section", async () => {
+  it("SSR request defers the CMS ⚡ section and keeps the plain one eager", async () => {
     const result = await resolveDecoPage("/product/foo", {});
     expect(result?.deferredSections).toHaveLength(1);
     expect(result?.deferredSections[0].component).toBe("site/sections/Hero.tsx");
+    expect(result?.resolvedSections.map((s) => s.component)).toEqual(["site/sections/Banner.tsx"]);
+  });
+
+  it("client nav produces the IDENTICAL split — deferral is not disabled", async () => {
+    const ssr = await resolveDecoPage("/product/foo", {});
+    const nav = await resolveDecoPage("/product/foo", { isClientNavigation: true });
+
+    // The acceptance criterion: client nav returns deferredSections and awaits
+    // only the eager set, exactly like SSR.
+    expect(nav?.deferredSections).toHaveLength(1);
+    expect(nav?.deferredSections.map((d) => d.component)).toEqual(
+      ssr?.deferredSections.map((d) => d.component),
+    );
+    expect(nav?.resolvedSections.map((s) => s.component)).toEqual(
+      ssr?.resolvedSections.map((s) => s.component),
+    );
   });
 
-  it("client-nav (isClientNavigation: true) resolves the ⚡ section eagerly — empty deferredSections", async () => {
-    const result = await resolveDecoPage("/product/foo", { isClientNavigation: true });
+  it("the ⚡ section's index is preserved on client nav (ordering on the merge)", async () => {
+    const nav = await resolveDecoPage("/product/foo", { isClientNavigation: true });
+    expect(nav?.deferredSections[0].index).toBe(1);
+  });
+
+  it("bots stay fully eager on a client-nav-flagged request (SEO guarantee)", async () => {
+    const result = await resolveDecoPage("/product/foo", {
+      isClientNavigation: true,
+      userAgent: "Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)",
+    });
+    expect(result?.deferredSections).toHaveLength(0);
+    expect(result?.resolvedSections).toHaveLength(2);
+  });
+
+  it("?__deco_ssr=1 stays fully eager on a client-nav-flagged request", async () => {
+    const result = await resolveDecoPage("/product/foo", {
+      isClientNavigation: true,
+      url: "https://store.com/product/foo?__deco_ssr=1",
+    });
     expect(result?.deferredSections).toHaveLength(0);
+    expect(result?.resolvedSections).toHaveLength(2);
+  });
+
+  it("a genuine programmatic fetch (Sec-Fetch-Dest: empty, no client-nav flag) stays eager", async () => {
+    const result = await resolveDecoPage("/product/foo", {
+      request: new Request("https://store.com/product/foo", {
+        headers: { "sec-fetch-dest": "empty" },
+      }),
+    });
+    expect(result?.deferredSections).toHaveLength(0);
+  });
+});
+
+// ---------------------------------------------------------------------------
+// #277 — per-request context survives the deferred second hop
+// ---------------------------------------------------------------------------
+//
+// This is the invariant that made it safe to re-enable deferral on client nav.
+// #277 reported deferred sections rendering blank because their loaders "lost"
+// per-request app context. The guarantee is that `resolveDeferredSectionFull`
+// (and `loadDeferredSection`, which wraps it in @decocms/tanstack) threads the
+// caller's MatcherContext — cookies, url, path, userAgent, request — into BOTH
+// the cache-hit and the cache-miss (`reExtractRawProps`) branch. A different
+// isolate misses the in-process rawProps Map, so the miss path is the one that
+// actually runs in production on Cloudflare Workers; if it ever stops receiving
+// matcherCtx, cookie-dependent loaders silently resolve against an anonymous
+// request. Do not relax these assertions.
+
+describe("#277 — deferred second hop keeps per-request context", () => {
+  const PROBE = "test/matchers/probe.ts";
+  const CTX: MatcherContext & { request: Request } = {
+    userAgent: "Mozilla/5.0 (iPhone; CPU iPhone OS 17_0) Mobile Safari",
+    url: "https://store.com/product/foo?utm=x",
+    path: "/product/foo",
+    cookies: { deco_segment: "abc", VtexIdclientAutCookie: "tok" },
+    request: new Request("https://store.com/product/foo?utm=x"),
+  };
+
+  /** Every MatcherContext the probe matcher was evaluated with. */
+  let seen: MatcherContext[] = [];
+
+  // The ⚡ section sits behind a multivariate flag whose rule is the probe
+  // matcher. Reaching the inner Shelf at all therefore PROVES the resolver
+  // evaluated the rule, and the probe records exactly which context it saw —
+  // an end-to-end assertion rather than a mock of the call site.
+  const gatedLazy = {
+    __resolveType: WELL_KNOWN_TYPES.MULTIVARIATE,
+    variants: [
+      {
+        rule: { __resolveType: PROBE },
+        value: {
+          __resolveType: WELL_KNOWN_TYPES.LAZY,
+          section: { __resolveType: "site/sections/Shelf.tsx", title: "Mais vendidos" },
+        },
+      },
+    ],
+  };
+
+  beforeEach(() => {
+    seen = [];
+    registerMatcher(PROBE, (_rule, ctx) => {
+      seen.push(ctx);
+      return true;
+    });
+    setAsyncRenderingConfig({ foldThreshold: Infinity, respectCmsLazy: true });
+    (getSection as ReturnType<typeof vi.fn>).mockReturnValue({ default: () => null });
+    (findPageByPath as ReturnType<typeof vi.fn>).mockReturnValue({
+      page: { name: "test", sections: [gatedLazy] },
+      params: {},
+      blockKey: "test-page",
+    });
+    (runSingleSectionLoader as ReturnType<typeof vi.fn>).mockImplementation(
+      async (s: unknown) => s,
+    );
+  });
+
+  afterEach(() => {
+    (getSection as ReturnType<typeof vi.fn>).mockReset();
+    (findPageByPath as ReturnType<typeof vi.fn>).mockReset();
+    (runSingleSectionLoader as ReturnType<typeof vi.fn>).mockReset();
+  });
+
+  it("reExtractRawProps (cross-isolate cache miss) resolves with the caller's cookies/UA/url", async () => {
+    // A cold isolate never populated the in-process rawProps Map, so this is
+    // the branch that actually runs in production on Cloudflare Workers.
+    const rawProps = await reExtractRawProps("/product/foo", "site/sections/Shelf.tsx", 0, CTX);
+
+    expect(rawProps).toMatchObject({ title: "Mais vendidos" });
+    expect(seen).not.toHaveLength(0);
+    for (const ctx of seen) {
+      expect(ctx.cookies).toEqual(CTX.cookies);
+      expect(ctx.userAgent).toBe(CTX.userAgent);
+      expect(ctx.url).toBe(CTX.url);
+      expect(ctx.request).toBe(CTX.request);
+    }
+  });
+
+  it("resolveDeferredSectionFull on a cold cache still resolves + enriches the section", async () => {
+    const ds = {
+      component: "site/sections/Shelf.tsx",
+      index: 0,
+      props: {},
+    } as unknown as DeferredSection;
+
+    const section = await resolveDeferredSectionFull(ds, "/product/foo", CTX.request, CTX);
+
+    expect(section).not.toBeNull();
+    expect(section?.component).toBe("site/sections/Shelf.tsx");
+    expect(section?.index).toBe(0);
+    // The deferred hop must run the section's own loader — this is what #277
+    // reported as missing, and it is what makes the second hop equivalent to
+    // eager resolution. Scope note: this asserts the request THIS function was
+    // handed reaches the loader. `loadDeferredSection` in @decocms/tanstack
+    // constructs its own `new Request(pageUrl || serverUrl, { headers })` before
+    // calling in, so the fidelity of that reconstruction is a separate concern
+    // and is not covered here.
+    expect(runSingleSectionLoader).toHaveBeenCalled();
+    const [, passedRequest] = (runSingleSectionLoader as ReturnType<typeof vi.fn>).mock
+      .calls[0] as [unknown, Request];
+    expect(passedRequest).toBe(CTX.request);
+  });
+
+  it("a context-free second hop is observably different — guards against dropping matcherCtx", async () => {
+    // If reExtractRawProps ever stops threading matcherCtx, cookie/UA-gated
+    // variants silently resolve against an anonymous request. Assert the probe
+    // can actually tell the two apart, so the test above is not vacuous.
+    await reExtractRawProps("/product/foo", "site/sections/Shelf.tsx", 0, undefined);
+    expect(seen).not.toHaveLength(0);
+    expect(seen[0].cookies).toBeUndefined();
+    expect(seen[0].userAgent).toBeUndefined();
   });
 });
 

packages/blocks/src/cms/resolve.ts

@@ -388,9 +388,10 @@ function hasForceEagerParam(ctx?: MatcherContext): boolean {
  * (`image`/`script`/`style`/`font`) do not — navigations stay deferred because
  * they CAN hydrate and resolve deferred sections. SPA navigations (TanStack
  * `<Link>` → `/_serverFn`) also send `empty` but set `isClientNavigation`, and
- * are excluded here so page-SEO commerce loaders stay off for humans
- * (decocms/blocks#286); their sections already render eagerly via the
- * `!isClientNav` branch of the `useAsync` gate.
+ * are excluded here on both counts: page-SEO commerce loaders stay off for
+ * humans (decocms/blocks#286), and their sections stay deferred, because a
+ * client nav renders through `DecoPageRenderer` and therefore CAN resolve a
+ * deferred section on scroll — unlike a real AJAX consumer.
  *
  * Like {@link hasForceEagerParam}, this must stay in lock-step with the edge
  * cache key: `workerEntry` keys `Sec-Fetch-Dest: empty` page requests into a
@@ -447,11 +448,15 @@ export interface MatcherContext {
    */
   flags?: StoredFlag[];
   /**
-   * Client-side (SPA) navigation via TanStack `<Link>`. Disables section
-   * deferral: deferral is a streaming-SSR optimization, but a client nav
-   * receives the server-fn JSON in one shot, so deferral adds a round-trip +
-   * skeleton with no benefit (and breaks loaders that need per-request app
-   * context — see decocms/blocks#277). Set by the route loaders.
+   * Client-side (SPA) navigation via TanStack `<Link>`. Set by the route
+   * loaders, where the incoming request URL is the `/_serverFn/...` endpoint
+   * rather than the page being navigated to.
+   *
+   * Consumed by `derivePageUrl` (rebuilding the real page URL, #280) and by
+   * {@link isProgrammaticFetch} (a SPA nav sends `Sec-Fetch-Dest: empty` but is
+   * NOT an AJAX consumer). It deliberately does NOT affect section deferral —
+   * a TanStack route loader is blocking, so eager-resolving below-fold sections
+   * on client nav stalls the whole transition. See `resolveDecoPageImpl`.
    */
   isClientNavigation?: boolean;
 }
@@ -1523,6 +1528,15 @@ function isCmsDeferralWrapped(section: unknown, matcherCtx?: MatcherContext): bo
  * then they can only force a NON-⚡ section eager — they never override the
  * editor's ⚡ choice.
  *
+ * Because deferral now applies to client (SPA) navigations too, a ⚡ section is
+ * resolved on a SECOND server hop (`loadDeferredSection`) in both cases. That
+ * hop rebuilds MatcherContext from the real request, but it cannot reconstruct
+ * "which branch of the page renders at all" — so a *gate* section (one whose
+ * loader picks between `children`/`fallback`, e.g. a combined PDP/PLP route)
+ * must be left un-⚡ in the admin. There is deliberately no code-level override
+ * for this: `neverDefer`/`alwaysEager` sit below the admin check and cannot win
+ * against an explicit editorial ⚡ (see decocms/blocks#277).
+ *
  * Exported for unit testing.
  */
 export function shouldDeferSection(
@@ -2008,13 +2022,32 @@ async function resolveDecoPageImpl(
   }
 
   const isBotReq = isEagerRequest(matcherCtx);
-  // SPA navigation (TanStack <Link>) receives the server-fn JSON in one shot —
-  // there is no HTTP streaming, so deferral adds a round-trip + skeleton with
-  // no benefit (and breaks loaders that need per-request app context, #277).
-  // Resolve everything eagerly on client nav; SSR/bots keep deferral.
-  const isClientNav = matcherCtx?.isClientNavigation ?? false;
+  // Deferral applies identically to SSR documents and SPA navigations. A
+  // TanStack route loader is BLOCKING: the router does not commit the
+  // transition until the loader promise settles, so resolving every
+  // below-the-fold section eagerly on client nav freezes the previous page for
+  // as long as the slowest shelf/upstream takes (measured: 2.7s and a 3.4MB
+  // payload on a real PDP — worse than a full reload). Deferral's job is
+  // decoupling first paint from below-fold data, which matters MORE here, not
+  // less.
+  //
+  // #277 (deferred loaders missing per-request app context) is not a reason to
+  // disable deferral on client nav: the deferred second hop is the same
+  // `loadDeferredSection` server fn the SSR path has always used, and it runs
+  // server-side with matcherCtx rebuilt from the real request
+  // (url/path/cookies/request). It may well land in a DIFFERENT isolate — that
+  // is exactly why `reExtractRawProps` exists as the rawProps cache-miss path —
+  // but per-request state is reconstructed from the request either way, so a
+  // client nav is no more exposed than an SSR document already was. A section
+  // whose loader genuinely cannot be resolved on a second hop (e.g. a PDP/PLP
+  // gate that decides what renders at all) must not be marked ⚡ in the admin;
+  // see `shouldDeferSection`.
+  //
+  // `isClientNavigation` is still load-bearing for `derivePageUrl` (duplicate
+  // query params) and for `isProgrammaticFetch` (SPA `Sec-Fetch-Dest: empty`
+  // must not be mistaken for an AJAX call) — it just no longer gates deferral.
   const currentAsyncConfig = getAsyncConfig();
-  const useAsync = currentAsyncConfig !== null && !isBotReq && !isClientNav;
+  const useAsync = currentAsyncConfig !== null && !isBotReq;
 
   const eagerResults: (ResolvedSection[] | Promise<ResolvedSection[]>)[] = [];
   const deferredSections: DeferredSection[] = [];