diff --git a/.agents/skills/deco-migrate-script/SKILL.md b/.agents/skills/deco-migrate-script/SKILL.md index 042f70b2..ab72f1d6 100644 --- a/.agents/skills/deco-migrate-script/SKILL.md +++ b/.agents/skills/deco-migrate-script/SKILL.md @@ -79,6 +79,7 @@ packages/blocks-cli/scripts/migrate/ │ ├── imports.ts ← 70+ import rewriting rules │ ├── jsx.ts ← JSX attribute fixes │ ├── fresh-apis.ts ← Fresh framework API removal +│ ├── ctx-compat.ts ← Optional-chain section-loader ctx.* reads (#305) │ ├── deno-isms.ts ← Deno-specific cleanup │ ├── dead-code.ts ← Old cache/loader system removal │ └── tailwind.ts ← Tailwind v3→v4 + DaisyUI v4→v5 @@ -152,7 +153,7 @@ Generates 14+ configuration and infrastructure files: ### Phase 3: Transform -Applies 6 transforms in sequence to every source file: +Applies 7 transforms in sequence to every source file: #### 1. `imports.ts` — Import Rewriting (70+ rules) @@ -194,13 +195,27 @@ setTimeout → window.setTimeout (type safety) - `IS_BROWSER` → `typeof window !== "undefined"` - `Context.active()` → removed -#### 4. `dead-code.ts` — Old Deco Patterns +#### 4. `ctx-compat.ts` — Section-loader `ctx` compatibility (#305) + +Runs only on files that export a `loader`. deco.cx section loaders use a +3-arg `(props, req, ctx)` signature; the framework now supplies a real compat +`ctx` (device, `invoke`, per-app state, `response.headers`) as the 3rd arg. But +an app that isn't configured yields `undefined`, so a non-optional deep read +(`ctx.salesforce.cartExtension[0]`) still throws and `withSectionLoader`'s +try/catch silently drops the section's props (blank render). + +- Optional-chains every `ctx.*` read → `ctx?.salesforce?.cartExtension?.[0]`. +- Leaves already-optional chains and assignment targets alone. +- No-op on files without a `loader` export (so an unrelated `ctx`, e.g. a + canvas 2D context, is untouched). + +#### 5. `dead-code.ts` — Old Deco Patterns - Removes: `export const cache`, `export const cacheKey`, `export const loader` (old caching system) - Handles: `crypto.subtle.digestSync` (Deno-only → async) - Preserves: `invoke.*` calls (runtime.ts proxy) -#### 5. `deno-isms.ts` — Deno Cleanup +#### 6. `deno-isms.ts` — Deno Cleanup - `deno-lint-ignore` comments → removed - `npm:` prefix → removed @@ -208,7 +223,7 @@ setTimeout → window.setTimeout (type safety) - `Deno.*` API usage → flagged - `/// ` directives → removed -#### 6. `tailwind.ts` — Tailwind v3→v4 + DaisyUI v4→v5 +#### 7. `tailwind.ts` — Tailwind v3→v4 + DaisyUI v4→v5 **23 Tailwind class renames:** ``` @@ -477,7 +492,7 @@ This script handles **Phases 0-6** of the [migration playbook](../deco-to-tansta - Phase 0 (Scaffold) → `phase-scaffold.ts` - Phase 1 (Imports) → `transforms/imports.ts` - Phase 2 (Signals) → `transforms/imports.ts` (bulk only — manual `useSignal` → `useState` still needed) -- Phase 3 (Deco Framework) → `transforms/fresh-apis.ts` + `transforms/deno-isms.ts` +- Phase 3 (Deco Framework) → `transforms/fresh-apis.ts` + `transforms/ctx-compat.ts` + `transforms/deno-isms.ts` - Phase 4 (Commerce) → `transforms/imports.ts` - Phase 6 (Islands) → `phase-cleanup.ts` (deletes directory, repoints imports) diff --git a/.agents/skills/deco-to-tanstack-migration/references/hydration-fixes.md b/.agents/skills/deco-to-tanstack-migration/references/hydration-fixes.md index 2097327e..e9f40c97 100644 --- a/.agents/skills/deco-to-tanstack-migration/references/hydration-fixes.md +++ b/.agents/skills/deco-to-tanstack-migration/references/hydration-fixes.md @@ -517,16 +517,18 @@ function Header({ device, ...props }: Props) { The `device` prop comes from the section loader: ```typescript -export async function loader(props: Props, req: Request, ctx: AppContext) { +export async function loader(props: Props, req: Request, ctx?: AppContext) { return { ...props, - device: ctx.device as "mobile" | "desktop" | "tablet", + device: ctx?.device as "mobile" | "desktop" | "tablet", }; } ``` `ctx.device` is detected from the request `User-Agent` header server-side. TanStack Start serializes loaderData and sends it to the client, so both server and client always use the same value. No mismatch. +> **The 3rd-arg `ctx` is real (#305).** The framework passes a compat `ctx` (device, `invoke`, per-app state, `response.headers`) as the loader's 3rd argument — see vtex-commerce.md §32. `ctx.device` here is genuinely populated. Keep app-state reads optional-chained (`ctx?.vtex?.…`) since an unconfigured app is `undefined`. + Also fix the section's `LoadingFallback` to use `props.device` instead of `useDevice()`: ```tsx // Before diff --git a/.agents/skills/deco-to-tanstack-migration/references/vtex-commerce.md b/.agents/skills/deco-to-tanstack-migration/references/vtex-commerce.md index dcb4df88..3603e41b 100644 --- a/.agents/skills/deco-to-tanstack-migration/references/vtex-commerce.md +++ b/.agents/skills/deco-to-tanstack-migration/references/vtex-commerce.md @@ -56,6 +56,15 @@ During migration, section loaders (e.g., `sections/Header/Header.tsx`) may have **Fix**: Keep all section loader logic intact. The loader signature `(props, req, ctx) => {...}` and the `ctx.invoke` calls should be preserved as-is. +**The 3rd-arg `ctx` is real now (#305).** The framework builds a compat `ctx` and passes it as the loader's 3rd argument (`@decocms/blocks`'s `buildSectionLoaderContext`, wired through `withSectionLoader`/`withPageContext`). What it provides: + +- `ctx.device` — `"mobile" | "tablet" | "desktop"` from the request User-Agent (works in worker, dev, and SPA-nav paths — it's derived from `req`, not the ambient `RequestContext`). +- `ctx.invoke.*` — a nested invoke proxy bound to this request's origin + AbortSignal. `ctx.invoke.vtex.loaders.categories.tree()` works server-side via a self-fetch to `/deco/invoke`. +- `ctx.` (e.g. `ctx.vtex`, `ctx.salesforce`) — the app's request-scoped state via `RequestContext.getAppState(name)`, or `undefined` if the app isn't configured. +- `ctx.response.headers` — maps to `RequestContext.responseHeaders` (Set-Cookie forwarding) inside the worker request scope; writes are dropped on the dev/SPA serverFn path. + +**Still optional-chain app-state reads.** An unconfigured app yields `undefined`, so a non-optional deep read like `ctx.salesforce.cartExtension[0]` throws — and `withSectionLoader`'s try/catch swallows it, dropping the section's props (blank render). The migrator's `ctx-compat` transform auto-rewrites `ctx.*` reads to optional chains (`ctx?.salesforce?.cartExtension?.[0]`); write new/hand-fixed loaders the same way. + ## 34. Commerce Loaders Are Blind to the URL diff --git a/packages/blocks-cli/scripts/migrate/phase-transform.ts b/packages/blocks-cli/scripts/migrate/phase-transform.ts index cf59ac4b..1ce208da 100644 --- a/packages/blocks-cli/scripts/migrate/phase-transform.ts +++ b/packages/blocks-cli/scripts/migrate/phase-transform.ts @@ -6,6 +6,7 @@ import { log, logPhase } from "./types"; import { transformImports } from "./transforms/imports"; import { transformJsx } from "./transforms/jsx"; import { transformFreshApis } from "./transforms/fresh-apis"; +import { transformCtxCompat } from "./transforms/ctx-compat"; import { transformDenoIsms } from "./transforms/deno-isms"; import { transformTailwind } from "./transforms/tailwind"; import { transformDeadCode } from "./transforms/dead-code"; @@ -55,16 +56,19 @@ function applyTransforms(content: string, filePath: string, ctx?: MigrationConte return { content, changed: false, notes: [] }; } - // Pipeline: imports → jsx → htmx-on-events → fresh-apis → dead-code → deno-isms → tailwind + // Pipeline: imports → jsx → htmx-on-events → fresh-apis → ctx-compat → dead-code → deno-isms → tailwind // htmx-on-events runs after jsx (which renames class/onChange) and // before fresh-apis (which removes useScript imports the htmx // codemod's TODO might still reference). The codemod is a no-op on // files without hx-on, so it never adds latency to non-htmx sites. + // ctx-compat runs after fresh-apis so it sees the settled loader body; it's + // a no-op on files without a `loader` export (#305). const pipeline: Array<{ name: string; fn: (content: string) => TransformResult }> = [ { name: "imports", fn: (c) => transformImports(c, ctx?.islandWrapperTargets) }, { name: "jsx", fn: transformJsx }, { name: "htmx-on-events", fn: transformHtmxOnEvents }, { name: "fresh-apis", fn: transformFreshApis }, + { name: "ctx-compat", fn: transformCtxCompat }, { name: "dead-code", fn: (c) => transformDeadCode(c, ctx?.platform) }, { name: "deno-isms", fn: transformDenoIsms }, { name: "tailwind", fn: transformTailwind }, diff --git a/packages/blocks-cli/scripts/migrate/templates/section-loaders.ts b/packages/blocks-cli/scripts/migrate/templates/section-loaders.ts index 03751209..2233bd0b 100644 --- a/packages/blocks-cli/scripts/migrate/templates/section-loaders.ts +++ b/packages/blocks-cli/scripts/migrate/templates/section-loaders.ts @@ -314,9 +314,11 @@ export function generateSectionLoaders(ctx: MigrationContext): string { // ---------- SEO + analytics delegation ---------- if (hasSEOPDP) { entries.push(``); - entries.push(` "site/sections/SEOPDP.tsx": async (props: any, _req) => {`); + // The framework supplies the real compat ctx as the 3rd arg (#305) — + // forward it instead of faking `{ seo: {} }` (which violated policy D3). + entries.push(` "site/sections/SEOPDP.tsx": async (props: any, _req, ctx) => {`); entries.push(` const mod = await import("../sections/SEOPDP");`); - entries.push(` const result = mod.loader(props, _req, { seo: {} } as any);`); + entries.push(` const result = mod.loader(props, _req, ctx);`); entries.push(` return result ?? props;`); entries.push(` },`); } @@ -329,9 +331,9 @@ export function generateSectionLoaders(ctx: MigrationContext): string { } if (hasIsEvents) { - entries.push(` "site/sections/Analytics/IsEvents.tsx": async (props: any, req) => {`); + entries.push(` "site/sections/Analytics/IsEvents.tsx": async (props: any, req, ctx) => {`); entries.push(` const mod = await import("../sections/Analytics/IsEvents");`); - entries.push(` return mod.loader(props, req) as unknown as Record;`); + entries.push(` return mod.loader(props, req, ctx) as unknown as Record;`); entries.push(` },`); } diff --git a/packages/blocks-cli/scripts/migrate/transforms/ctx-compat.test.ts b/packages/blocks-cli/scripts/migrate/transforms/ctx-compat.test.ts new file mode 100644 index 00000000..349d89df --- /dev/null +++ b/packages/blocks-cli/scripts/migrate/transforms/ctx-compat.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it } from "vitest"; +import { transformCtxCompat } from "./ctx-compat"; + +const withLoader = (body: string) => + `export const loader = (props: Props, req: Request, ctx?: AppContext) => {\n${body}\n};`; + +describe("transformCtxCompat", () => { + it("optional-chains a deep app-state read", () => { + const src = withLoader(" const ext = ctx.salesforce.cartExtension[0];"); + const r = transformCtxCompat(src); + expect(r.changed).toBe(true); + expect(r.content).toContain("ctx?.salesforce?.cartExtension?.[0]"); + }); + + it("optional-chains ctx.device and ctx.invoke calls (harmless no-op safety)", () => { + const src = withLoader( + " const isMobile = ctx.device !== 'desktop';\n const page = await ctx.invoke.vtex.loaders.product.detailsPageGQL({ slug });", + ); + const r = transformCtxCompat(src); + expect(r.content).toContain("ctx?.device !== 'desktop'"); + expect(r.content).toContain("ctx?.invoke?.vtex?.loaders?.product?.detailsPageGQL({ slug })"); + }); + + it("leaves already-optional chains intact (no double ??.)", () => { + const src = withLoader(" const x = ctx?.salesforce?.cartExtension?.[0];"); + const r = transformCtxCompat(src); + // No change needed → not flagged as changed. + expect(r.content).toContain("ctx?.salesforce?.cartExtension?.[0]"); + expect(r.content).not.toContain("??."); + }); + + it("does not optional-chain an assignment target (would be a syntax error)", () => { + const src = withLoader(" ctx.state.count = 1;"); + const r = transformCtxCompat(src); + expect(r.content).toContain("ctx.state.count = 1;"); + expect(r.content).not.toContain("ctx?.state?.count ="); + }); + + it("still rewrites reads while leaving assignment targets alone", () => { + const src = withLoader(" ctx.state.count = ctx.state.count + 1;"); + const r = transformCtxCompat(src); + // LHS untouched, RHS optional-chained. + expect(r.content).toContain("ctx.state.count = ctx?.state?.count + 1;"); + }); + + it("is a no-op on files without a loader export", () => { + const src = "const ctx = canvas.getContext('2d');\nctx.fillRect(0, 0, 1, 1);"; + const r = transformCtxCompat(src); + expect(r.changed).toBe(false); + expect(r.content).toBe(src); + }); + + it("does not touch the ctx parameter declaration", () => { + const src = withLoader(" return props;"); + const r = transformCtxCompat(src); + expect(r.changed).toBe(false); + expect(r.content).toContain("ctx?: AppContext"); + }); + + it("does not match identifiers that merely contain ctx", () => { + const src = withLoader(" const c = canvasCtx.foo;\n const d = a.ctx.bar;"); + const r = transformCtxCompat(src); + expect(r.content).toContain("canvasCtx.foo"); + expect(r.content).toContain("a.ctx.bar"); + }); +}); diff --git a/packages/blocks-cli/scripts/migrate/transforms/ctx-compat.ts b/packages/blocks-cli/scripts/migrate/transforms/ctx-compat.ts new file mode 100644 index 00000000..31c32e7f --- /dev/null +++ b/packages/blocks-cli/scripts/migrate/transforms/ctx-compat.ts @@ -0,0 +1,151 @@ +import type { TransformResult } from "../types"; + +/** + * Make ported deco.cx section loaders defensive about the compat `ctx` + * (issue #305). + * + * The framework now hands section loaders a real 3rd-arg `ctx` + * (`@decocms/blocks`'s `buildSectionLoaderContext`) with `device`, `invoke`, + * `response` and per-app state (`ctx.vtex`, `ctx.salesforce`, …). But an app + * that isn't configured on the target site yields `undefined`, so a + * *non-optional* deep read like `ctx.salesforce.cartExtension[0]` still throws + * — and `withSectionLoader`'s try/catch would swallow it, dropping the + * section's props (blank render). The hand-fixed reference migration + * (`granadobr-tanstack`) solves this by optional-chaining every `ctx` read. + * + * This codemod reproduces that: every `ctx.` member-access chain becomes an + * optional chain (`ctx?.a?.b?.[0]`). Optional chaining short-circuits to + * `undefined` instead of throwing, which is exactly the defensive behavior the + * working migration relies on. `ctx.device`/`ctx.invoke` are always present so + * the extra `?.` is a harmless no-op there. + * + * Scope: only files that export a `loader` (section/loader files), so an + * unrelated `ctx` (e.g. a canvas 2D context) elsewhere isn't touched. + * Assignment targets (`ctx.x = …`, invalid as an optional chain) are skipped. + */ + +const IDENT = /[A-Za-z0-9_$]/; + +const LOADER_EXPORT_RE = + /export\s+(?:const|(?:async\s+)?function)\s+loader\b|export\s*\{\s*[^}]*\bloader\b/; + +function isIdentChar(c: string | undefined): boolean { + return c !== undefined && IDENT.test(c); +} + +/** + * Scan a single `ctx` member-access chain starting at `start` (which must + * point at the `c` of `ctx`). Returns the rewritten (optional) chain, the + * original text, the index just past the chain, and whether the chain is the + * target of an assignment (in which case it must NOT be optional-chained). + */ +function scanChain( + code: string, + start: number, +): { rewritten: string; original: string; end: number; isAssignTarget: boolean } { + let i = start + 3; // past "ctx" + const pieces: string[] = ["ctx"]; + + while (i < code.length) { + if (code.startsWith("?.", i)) { + // already optional — keep it, then consume the identifier OR, for an + // optional computed access (`?.[expr]`), the balanced bracket group. + i += 2; + if (code[i] === "[") { + let depth = 1; + let j = i + 1; + while (j < code.length && depth > 0) { + if (code[j] === "[") depth++; + else if (code[j] === "]") depth--; + j++; + } + pieces.push("?.[", code.slice(i + 1, j)); + i = j; + } else { + let j = i; + while (j < code.length && isIdentChar(code[j])) j++; + pieces.push("?.", code.slice(i, j)); + i = j; + } + } else if (code[i] === ".") { + i += 1; + let j = i; + while (j < code.length && isIdentChar(code[j])) j++; + pieces.push("?.", code.slice(i, j)); + i = j; + } else if (code[i] === "[") { + let depth = 1; + let j = i + 1; + while (j < code.length && depth > 0) { + if (code[j] === "[") depth++; + else if (code[j] === "]") depth--; + j++; + } + pieces.push("?.[", code.slice(i + 1, j)); // slice includes closing "]" + i = j; + } else { + break; + } + } + + // Peek past trailing whitespace to detect an assignment target. + let k = i; + while (k < code.length && (code[k] === " " || code[k] === "\t")) k++; + const isAssignTarget = code[k] === "=" && code[k + 1] !== "=" && code[k + 1] !== ">"; + + return { + rewritten: pieces.join(""), + original: code.slice(start, i), + end: i, + isAssignTarget, + }; +} + +export function transformCtxCompat(content: string): TransformResult { + // Only touch files that actually export a loader — avoids rewriting an + // unrelated `ctx` variable (canvas context, etc.) in components. + if (!LOADER_EXPORT_RE.test(content)) { + return { content, changed: false, notes: [] }; + } + + let out = ""; + let i = 0; + let count = 0; + + while (i < content.length) { + const isCtxToken = + content.startsWith("ctx", i) && + !isIdentChar(content[i - 1]) && + content[i - 1] !== "." && + (content[i + 3] === "." || content[i + 3] === "[" || content.startsWith("?.", i + 3)); + + if (isCtxToken) { + const { rewritten, original, end, isAssignTarget } = scanChain(content, i); + if (!isAssignTarget && rewritten !== original) { + out += rewritten; + count++; + } else { + out += original; + } + i = end; + continue; + } + + out += content[i]; + i++; + } + + if (count === 0) { + return { content, changed: false, notes: [] }; + } + + return { + content: out, + changed: true, + notes: [ + `Optional-chained ${count} ctx.* read(s) so unconfigured app state degrades to undefined instead of throwing (#305)`, + ], + }; +} + +export const _internals = { scanChain, LOADER_EXPORT_RE }; diff --git a/packages/blocks-cli/scripts/migrate/transforms/fresh-apis.ts b/packages/blocks-cli/scripts/migrate/transforms/fresh-apis.ts index cba9f1ac..aa0172a7 100644 --- a/packages/blocks-cli/scripts/migrate/transforms/fresh-apis.ts +++ b/packages/blocks-cli/scripts/migrate/transforms/fresh-apis.ts @@ -202,9 +202,11 @@ export function transformFreshApis(content: string): TransformResult { notes.push("Removed allowCorsFor (not needed in TanStack)"); } - // ctx.response.headers → not available, flag + // ctx.response.headers → supported by the compat ctx (#305): section loaders + // receive a 3rd-arg ctx whose `response.headers` maps to + // RequestContext.responseHeaders. Just an INFO note (no manual work). if (result.includes("ctx.response")) { - notes.push("MANUAL: ctx.response usage found — FnContext in @decocms/blocks does not have response object"); + notes.push("INFO: ctx.response.headers is provided by the compat ctx (RequestContext.responseHeaders) inside the worker request scope; writes are dropped on the dev/SPA serverFn path"); } // { crypto } from "@std/crypto" → use globalThis.crypto (Web Crypto API) diff --git a/packages/blocks/src/cms/index.ts b/packages/blocks/src/cms/index.ts index f62fe98b..f3025375 100644 --- a/packages/blocks/src/cms/index.ts +++ b/packages/blocks/src/cms/index.ts @@ -81,6 +81,8 @@ export { WELL_KNOWN_TYPES, } from "./resolve"; export type { SectionLoaderFn } from "./sectionLoaders"; +export type { SectionLoaderContext } from "./sectionLoaderContext"; +export { buildSectionLoaderContext } from "./sectionLoaderContext"; export { isLayoutSection, registerCacheableSections, diff --git a/packages/blocks/src/cms/sectionLoaderContext.test.ts b/packages/blocks/src/cms/sectionLoaderContext.test.ts new file mode 100644 index 00000000..47c5078e --- /dev/null +++ b/packages/blocks/src/cms/sectionLoaderContext.test.ts @@ -0,0 +1,111 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { RequestContext } from "../sdk/requestContext"; +import { buildSectionLoaderContext } from "./sectionLoaderContext"; + +const MOBILE_UA = + "Mozilla/5.0 (iPhone; CPU iPhone OS 17_0 like Mac OS X) AppleWebKit/605.1.15 Mobile/15E148"; +const DESKTOP_UA = "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36"; +const TABLET_UA = "Mozilla/5.0 (iPad; CPU OS 17_0 like Mac OS X) AppleWebKit/605.1.15"; + +function reqWithUA(ua: string, url = "https://store.example/path?q=shoes"): Request { + return new Request(url, { headers: { "user-agent": ua } }); +} + +describe("buildSectionLoaderContext", () => { + describe("device", () => { + it("detects mobile / desktop / tablet from the request User-Agent", () => { + expect(buildSectionLoaderContext(reqWithUA(MOBILE_UA)).device).toBe("mobile"); + expect(buildSectionLoaderContext(reqWithUA(DESKTOP_UA)).device).toBe("desktop"); + expect(buildSectionLoaderContext(reqWithUA(TABLET_UA)).device).toBe("tablet"); + }); + + it("defaults to desktop when there is no User-Agent", () => { + expect(buildSectionLoaderContext(new Request("https://store.example/")).device).toBe( + "desktop", + ); + }); + + it("never throws for a minimal/mock request without headers", () => { + const badReq = { url: "not a url" } as unknown as Request; + expect(() => buildSectionLoaderContext(badReq)).not.toThrow(); + expect(buildSectionLoaderContext(badReq).device).toBe("desktop"); + }); + }); + + describe("app state", () => { + it("resolves ctx. to the app's registered state inside a request scope", async () => { + const req = reqWithUA(DESKTOP_UA); + await RequestContext.run(req, async () => { + RequestContext.setBag("app:vtex:state", { config: { account: "acme" } }); + const ctx = buildSectionLoaderContext(req); + expect((ctx as any).vtex).toEqual({ config: { account: "acme" } }); + expect(ctx.getAppState("vtex")).toEqual({ config: { account: "acme" } }); + }); + }); + + it("returns undefined for an unconfigured app (no fake object)", async () => { + const req = reqWithUA(DESKTOP_UA); + await RequestContext.run(req, async () => { + const ctx = buildSectionLoaderContext(req); + expect((ctx as any).salesforce).toBeUndefined(); + }); + }); + + it("returns undefined for app state outside a request scope", () => { + const ctx = buildSectionLoaderContext(reqWithUA(DESKTOP_UA)); + expect((ctx as any).vtex).toBeUndefined(); + }); + }); + + describe("response.headers", () => { + it("writes through to RequestContext.responseHeaders inside a scope", async () => { + const req = reqWithUA(DESKTOP_UA); + await RequestContext.run(req, async () => { + const ctx = buildSectionLoaderContext(req); + ctx.response.headers.set("set-cookie", "a=1"); + expect(RequestContext.responseHeaders.get("set-cookie")).toBe("a=1"); + }); + }); + + it("degrades to an inert Headers outside a scope (does not throw)", () => { + const ctx = buildSectionLoaderContext(reqWithUA(DESKTOP_UA)); + expect(() => ctx.response.headers.set("x", "y")).not.toThrow(); + }); + }); + + describe("invoke (server-side self-fetch)", () => { + let fetchMock: ReturnType; + + beforeEach(() => { + fetchMock = vi + .fn() + .mockResolvedValue(new Response(JSON.stringify({ ok: true }), { status: 200 })); + vi.stubGlobal("fetch", fetchMock); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("POSTs to the request's absolute origin /deco/invoke/", async () => { + const ctx = buildSectionLoaderContext(reqWithUA(DESKTOP_UA, "https://store.example/pdp")); + const result = await ctx.invoke.vtex.loaders.product.detailsPageGQL({ slug: "x" }); + + expect(result).toEqual({ ok: true }); + const [url, init] = fetchMock.mock.calls[0]; + expect(url).toBe("https://store.example/deco/invoke/vtex/loaders/product/detailsPageGQL"); + expect(init.method).toBe("POST"); + expect(JSON.parse(init.body)).toEqual({ slug: "x" }); + }); + + it("injects the request AbortSignal when invoked inside a request scope", async () => { + const req = reqWithUA(DESKTOP_UA, "https://store.example/pdp"); + await RequestContext.run(req, async () => { + const ctx = buildSectionLoaderContext(req); + await ctx.invoke.vtex.loaders.x({}); + const [, init] = fetchMock.mock.calls[0]; + expect(init.signal).toBeInstanceOf(AbortSignal); + }); + }); + }); +}); diff --git a/packages/blocks/src/cms/sectionLoaderContext.ts b/packages/blocks/src/cms/sectionLoaderContext.ts new file mode 100644 index 00000000..f71a6847 --- /dev/null +++ b/packages/blocks/src/cms/sectionLoaderContext.ts @@ -0,0 +1,113 @@ +/** + * Section-loader compatibility context (`ctx`) — issue #305. + * + * deco.cx (Fresh) section loaders use a 3-arg signature + * `(props, req, ctx: AppContext)`, where `ctx` exposes device detection, + * `ctx.invoke.*`, per-app state (`ctx.vtex`, `ctx.salesforce`, …) and + * `ctx.response.headers`. `@decocms/blocks` invokes loaders with only + * `(props, req)`, so migrated loaders read `undefined` and throw. + * + * Rather than delete `ctx` (which would force rewriting every migrated + * loader by hand), we re-assemble a **real** compat `ctx` from the primitives + * the framework already has — no fabricated stubs (policy D3): + * + * - `device` → {@link detectDevice} on the request User-Agent. Derived from + * `req` (NOT `RequestContext`) on purpose: the `createServerFn` path used in + * dev / SPA navigation is not wrapped in `RequestContext.run`, so a + * RequestContext-only device would silently degrade to "desktop" there. + * - `invoke` → the same nested invoke proxy the client uses, but pointed at + * an absolute self-origin and routed through `RequestContext.fetch` so the + * request's AbortSignal propagates. `ctx.invoke.vtex.loaders.x(props)` works + * server-side via a self-fetch to `/deco/invoke`. + * - app state → `RequestContext.getAppState(name)`; any unknown property access + * (`ctx.vtex`, `ctx.salesforce`, …) resolves to the app's registered state + * or `undefined`. Real lookup, not a fake object. + * - `response.headers` → `RequestContext.responseHeaders` when in a request + * scope; an inert `Headers` otherwise (writes don't propagate in dev/SPA but + * never crash). + */ + +import { type Device, detectDevice } from "../sdk/detectDevice"; +import { createAppInvokeWith } from "../sdk/invoke"; +import { RequestContext } from "../sdk/requestContext"; + +/** + * The compat context handed to section loaders as the 3rd argument. + * + * It is indexable (`[appName: string]: unknown`) because Fresh loaders read + * per-app state off `ctx` directly (`ctx.vtex`, `ctx.salesforce`, …). Those + * reads resolve through {@link RequestContext.getAppState}. Migrated code + * should still optional-chain deep app-state reads (`ctx.vtex?.config`), since + * an app that isn't configured yields `undefined`. + */ +export interface SectionLoaderContext { + /** Device type detected from the request User-Agent. */ + device: Device; + /** + * Nested invoke proxy (`ctx.invoke.vtex.loaders.x(props)`), bound to this + * request's origin and AbortSignal. Works server-side via self-fetch. + */ + invoke: any; + /** Outgoing response headers (e.g. for Set-Cookie forwarding). */ + response: { headers: Headers }; + /** Typed access to an app's request-scoped state. */ + getAppState: (appName: string) => T | undefined; + /** Per-app state read directly off ctx (Fresh compatibility). */ + [appName: string]: unknown; +} + +/** + * Build the compat {@link SectionLoaderContext} for a single loader call. + * Cheap to construct (device detect + lazy proxies), so it's fine to build + * per section loader invocation. + */ +export function buildSectionLoaderContext(req: Request): SectionLoaderContext { + // Defensive: `req` may be a minimal/mock request without a real `headers` + // object. Building the ctx must never throw — that would take down the + // loader before it runs. + const device = detectDevice(req.headers?.get?.("user-agent") ?? ""); + + let origin = ""; + try { + origin = new URL(req.url).origin; + } catch { + // Relative/opaque request URL — fall back to a relative base path. + } + + const invoke = createAppInvokeWith({ + basePath: `${origin}/deco/invoke`, + fetcher: (input, init) => RequestContext.fetch(input, init), + }); + + const base = { + device, + invoke, + get response(): { headers: Headers } { + // `responseHeaders` throws outside a request scope (dev/SPA serverFn + // path). Degrade to an inert Headers rather than crash the loader. + let headers: Headers; + try { + headers = RequestContext.responseHeaders; + } catch { + headers = new Headers(); + } + return { headers }; + }, + getAppState(appName: string): T | undefined { + return RequestContext.getAppState(appName); + }, + }; + + const known = new Set(["device", "invoke", "response", "getAppState"]); + + // Wrap so unknown property access (`ctx.vtex`, `ctx.salesforce`, …) resolves + // to the app's registered state. Known fields fall through to `base`. + return new Proxy(base as SectionLoaderContext, { + get(target, prop, receiver) { + if (typeof prop === "string" && !known.has(prop) && !(prop in target)) { + return RequestContext.getAppState(prop); + } + return Reflect.get(target, prop, receiver); + }, + }); +} diff --git a/packages/blocks/src/cms/sectionLoaders.test.ts b/packages/blocks/src/cms/sectionLoaders.test.ts index 2d612747..b882c1b4 100644 --- a/packages/blocks/src/cms/sectionLoaders.test.ts +++ b/packages/blocks/src/cms/sectionLoaders.test.ts @@ -82,6 +82,59 @@ describe("runSingleSectionLoader — page context injection", () => { const result = await runSingleSectionLoader(section, new Request("https://store.com/")); expect(result).toBe(section); }); + + it("passes a 3rd-arg compat ctx (device from UA) — regular path (#305)", async () => { + const loader = vi.fn( + async (props: Record, _req: Request, ctx?: { device?: string }) => ({ + ...props, + seenDevice: ctx?.device, + }), + ); + registerSectionLoader("site/sections/Ctx.tsx", loader); + + const section = makeSection("site/sections/Ctx.tsx", {}); + const mobileReq = new Request("https://store.com/", { + headers: { "user-agent": "iPhone Mobile Safari" }, + }); + const result = await runSingleSectionLoader(section, mobileReq); + + expect((result.props as Record).seenDevice).toBe("mobile"); + }); + + it("passes the compat ctx through layout and cacheable paths (#305)", async () => { + const layoutLoader = vi.fn( + async (props: Record, _req: Request, ctx?: { device?: string }) => ({ + ...props, + seenDevice: ctx?.device, + }), + ); + const cacheableLoader = vi.fn( + async (props: Record, _req: Request, ctx?: { device?: string }) => ({ + ...props, + seenDevice: ctx?.device, + }), + ); + registerSectionLoader("site/sections/CtxHeader.tsx", layoutLoader); + registerLayoutSections(["site/sections/CtxHeader.tsx"]); + registerSectionLoader("site/sections/CtxShelf.tsx", cacheableLoader); + registerCacheableSections({ "site/sections/CtxShelf.tsx": { maxAge: 60_000 } }); + + const mobileReq = new Request("https://store.com/", { + headers: { "user-agent": "iPhone Mobile Safari" }, + }); + + const layoutResult = await runSingleSectionLoader( + makeSection("site/sections/CtxHeader.tsx"), + mobileReq, + ); + const cacheableResult = await runSingleSectionLoader( + makeSection("site/sections/CtxShelf.tsx"), + mobileReq, + ); + + expect((layoutResult.props as Record).seenDevice).toBe("mobile"); + expect((cacheableResult.props as Record).seenDevice).toBe("mobile"); + }); }); describe("runSingleSectionLoader — cache keying", () => { @@ -151,9 +204,7 @@ describe("unregisterLayoutSections", () => { }); it("is a no-op for keys that were never registered", () => { - expect(() => - unregisterLayoutSections(["site/sections/Never.tsx"]), - ).not.toThrow(); + expect(() => unregisterLayoutSections(["site/sections/Never.tsx"])).not.toThrow(); expect(isLayoutSection("site/sections/Never.tsx")).toBe(false); }); }); @@ -190,10 +241,7 @@ describe("registerSectionLoaders — request-dependent + layout warning (#206)", it("warns when a composed loader contains any request-dependent mixin", () => { registerLayoutSections(["site/sections/Footer.tsx"]); registerSectionLoaders({ - "site/sections/Footer.tsx": compose( - withSearchParam(), - async (props) => props, - ), + "site/sections/Footer.tsx": compose(withSearchParam(), async (props) => props), }); expect(warnSpy).toHaveBeenCalled(); }); diff --git a/packages/blocks/src/cms/sectionLoaders.ts b/packages/blocks/src/cms/sectionLoaders.ts index 2228254b..864c2927 100644 --- a/packages/blocks/src/cms/sectionLoaders.ts +++ b/packages/blocks/src/cms/sectionLoaders.ts @@ -14,10 +14,17 @@ import { djb2 } from "../sdk/djb2"; import { withInflightTimeout } from "../sdk/inflightTimeout"; import { withTracing } from "../sdk/observability"; import type { ResolvedSection } from "./resolve"; +import { buildSectionLoaderContext, type SectionLoaderContext } from "./sectionLoaderContext"; export type SectionLoaderFn = ( props: Record, req: Request, + /** + * Compat context (issue #305). Optional so 2-arg loaders/mixins remain + * valid — they simply ignore the extra argument. Framework-supplied at the + * call site via {@link buildSectionLoaderContext}. + */ + ctx?: SectionLoaderContext, ) => Promise> | Record; // globalThis-backed: server function split modules need access @@ -361,9 +368,14 @@ function injectPageContext( return enriched; } -/** Wrap a loader so it receives __pageUrl/__pagePath in its props. */ +/** + * Wrap a loader so it receives __pageUrl/__pagePath in its props AND the + * 3rd-arg compat `ctx` (issue #305). This is the single choke point that all + * four invocation paths (regular, layout, cacheable, SWR refresh) route + * through, so building `ctx` here covers every path. + */ function withPageContext(loader: SectionLoaderFn): SectionLoaderFn { - return (props, req) => loader(injectPageContext(props, req), req); + return (props, req) => loader(injectPageContext(props, req), req, buildSectionLoaderContext(req)); } /** diff --git a/packages/blocks/src/cms/sectionMixins.ts b/packages/blocks/src/cms/sectionMixins.ts index 391add58..dd2eb18c 100644 --- a/packages/blocks/src/cms/sectionMixins.ts +++ b/packages/blocks/src/cms/sectionMixins.ts @@ -83,10 +83,10 @@ export function withSearchParam(): SectionLoaderFn { * ``` */ export function compose(...mixins: SectionLoaderFn[]): SectionLoaderFn { - const composed: SectionLoaderFn = async (props, req) => { + const composed: SectionLoaderFn = async (props, req, ctx) => { let result = { ...props }; for (const mixin of mixins) { - const partial = await mixin(result, req); + const partial = await mixin(result, req, ctx); result = { ...result, ...partial }; } return result; @@ -138,12 +138,14 @@ export function compose(...mixins: SectionLoaderFn[]): SectionLoaderFn { export function withSectionLoader( modImport: () => Promise, ): SectionLoaderFn { - return async (props, req) => { + return async (props, req, ctx) => { const mod = (await modImport()) as { loader?: unknown } | undefined; const loader = mod?.loader; if (typeof loader !== "function") return props; try { - const result = await (loader as SectionLoaderFn)(props, req); + // Forward the compat `ctx` (issue #305) so migrated Fresh loaders that + // read `ctx.device`/`ctx.invoke`/`ctx.` get the real 3rd argument. + const result = await (loader as SectionLoaderFn)(props, req, ctx); return result ?? props; } catch (error) { console.error("[withSectionLoader] section loader threw:", error); diff --git a/packages/blocks/src/sdk/invoke.ts b/packages/blocks/src/sdk/invoke.ts index 33bab167..13df2fb7 100644 --- a/packages/blocks/src/sdk/invoke.ts +++ b/packages/blocks/src/sdk/invoke.ts @@ -199,12 +199,71 @@ export function createAppInvoke>( basePath?: string, ): NestedFromFlat; export function createAppInvoke(basePath = "/deco/invoke"): any { + return createAppInvokeWith({ basePath }); +} + +/** + * Default nested invoke proxy bound to `/deco/invoke`. + * + * Replaces site-level `~/runtime.ts` shims that wrap the same `createAppInvoke` + * call. Importing this singleton means no per-site boilerplate. + * + * @example + * ```ts + * import { invoke } from "@decocms/start/sdk/invoke"; + * + * await invoke.vtex.actions.checkout.addItemsToCart({ orderFormId, orderItems }); + * await invoke.site.loaders.Wishlist.getWishlist({}); + * ``` + * + * For a custom `basePath` or typed handlers, call `createAppInvoke()` directly: + * ```ts + * const invoke = createAppInvoke("/my/invoke"); + * ``` + */ +export const invoke = createAppInvoke(); + +/** + * A `fetch`-compatible function. Lets callers route the invoke proxy's HTTP + * calls through a custom transport — e.g. server-side, an absolute-origin + * self-fetch that injects the request's AbortSignal (`RequestContext.fetch`). + */ +export type InvokeFetcher = (input: RequestInfo | URL, init?: RequestInit) => Promise; + +/** + * Like {@link createAppInvoke}, but with an injectable `fetcher`. + * + * The default `createAppInvoke` targets a relative path (`/deco/invoke`) via + * the global `fetch`, which only works in the browser. Server-side callers + * (e.g. a section loader's `ctx.invoke`) need an **absolute** base URL plus a + * fetcher that forwards the request's AbortSignal. This factory keeps the exact + * same nested-proxy semantics and 404→`.ts` retry, only swapping the transport. + * + * @example + * ```ts + * const origin = new URL(req.url).origin; + * const invoke = createAppInvokeWith({ + * basePath: `${origin}/deco/invoke`, + * fetcher: (input, init) => RequestContext.fetch(input, init), + * }); + * await invoke.vtex.loaders.product.detailsPageGQL({ slug }); + * ``` + */ +export function createAppInvokeWith(options?: { basePath?: string; fetcher?: InvokeFetcher }): any; +export function createAppInvokeWith>(options?: { + basePath?: string; + fetcher?: InvokeFetcher; +}): NestedFromFlat; +export function createAppInvokeWith(options?: { basePath?: string; fetcher?: InvokeFetcher }): any { + const basePath = options?.basePath ?? "/deco/invoke"; + const fetcher: InvokeFetcher = options?.fetcher ?? ((input, init) => fetch(input, init)); + function buildProxy(path: string[]): any { return new Proxy( Object.assign(async (props: any) => { const key = path.join("/"); for (const k of [key, `${key}.ts`]) { - const response = await fetch(`${basePath}/${k}`, { + const response = await fetcher(`${basePath}/${k}`, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify(props ?? {}), @@ -237,24 +296,3 @@ export function createAppInvoke(basePath = "/deco/invoke"): any { return buildProxy([]); } - -/** - * Default nested invoke proxy bound to `/deco/invoke`. - * - * Replaces site-level `~/runtime.ts` shims that wrap the same `createAppInvoke` - * call. Importing this singleton means no per-site boilerplate. - * - * @example - * ```ts - * import { invoke } from "@decocms/start/sdk/invoke"; - * - * await invoke.vtex.actions.checkout.addItemsToCart({ orderFormId, orderItems }); - * await invoke.site.loaders.Wishlist.getWishlist({}); - * ``` - * - * For a custom `basePath` or typed handlers, call `createAppInvoke()` directly: - * ```ts - * const invoke = createAppInvoke("/my/invoke"); - * ``` - */ -export const invoke = createAppInvoke(); diff --git a/packages/blocks/src/types/index.ts b/packages/blocks/src/types/index.ts index ec6dae40..5daf9b8c 100644 --- a/packages/blocks/src/types/index.ts +++ b/packages/blocks/src/types/index.ts @@ -4,8 +4,22 @@ * without depending on the Deno-specific @deco/deco package. */ +/** + * Compat context handed to ported deco.cx (Fresh) section loaders as the 3rd + * argument (issue #305). `state` is the app state; `device`/`invoke`/`response` + * mirror what Fresh's `ctx` exposed, and the index signature lets migrated + * loaders read per-app state directly off `ctx` (`ctx.vtex`, `ctx.salesforce`). + * Deep reads should still be optional-chained — an unconfigured app is + * `undefined`. See `@decocms/blocks/cms`'s `SectionLoaderContext` for the + * runtime shape. + */ export interface FnContext { state: TState; + device?: "mobile" | "tablet" | "desktop"; + invoke?: any; + response?: { headers: Headers }; + getAppState?: (appName: string) => T | undefined; + [key: string]: any; } export type App = {