Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions hooks/mod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export {
addAllowedQS,
addBlockedQS,
addBlockedQS as unstable_blockUseSectionHrefQueryStrings,
computeRenderCb,
useSection,
} from "./useSection.ts";
export { useSetEarlyHints } from "./useSetEarlyHints.ts";
43 changes: 35 additions & 8 deletions hooks/useSection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,36 @@ const createStableHref = (href: string): string => {
return hrefUrl.href;
};

export interface RenderCbInput {
revision: unknown;
vary: unknown;
href: string;
deploymentId?: string;
}

/**
* Cache-bust value for `/deco/render` URLs. Single source of truth shared by
* `useSection` (web partials) and JSON serialization consumers (e.g. the
* website app's ?renderJson handler) — both sides MUST produce identical
* values or web/JSON cache invalidation diverges.
*
* `revision`/`vary` join as-is (undefined → "undefined") to preserve the
* historical recipe byte-for-byte. Shared `hasher` is safe ONLY because this
* function is fully synchronous — no await between hash() and reset().
*/
export const computeRenderCb = (input: RenderCbInput): string => {
const cbString = [
input.revision,
input.vary,
createStableHref(input.href),
input.deploymentId,
].join("|");
hasher.hash(cbString);
const cb = `${hasher.result()}`;
hasher.reset();
return cb;
};

export type Options<P> = {
/** Section props partially applied */
props?: Partial<P extends ComponentType<infer K> ? K : P>;
Expand All @@ -119,15 +149,12 @@ export const useSection = <P>(

const hrefParam = href ?? request.url;
const stableHref = createStableHref(hrefParam);
const cbString = [
revisionId,
const cb = computeRenderCb({
revision: revisionId,
vary,
stableHref,
ctx?.deploymentId,
].join("|");
hasher.hash(cbString);
const cb = hasher.result();
hasher.reset();
href: hrefParam,
deploymentId: ctx?.deploymentId,
});

const params = new URLSearchParams([
["props", JSON.stringify(props)],
Expand Down
16 changes: 16 additions & 0 deletions mod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,22 @@ export { type DecoRouteState } from "./runtime/middleware.ts";
export * from "./runtime/mod.ts";
export { Deco } from "./runtime/mod.ts";
export type { PageData } from "./runtime/mod.ts";
export {
buildLazyUrl,
sectionModuleLookup,
serializeResolvedSection,
} from "./runtime/routes/serialize-section.ts";
export { computeRenderCb } from "./hooks/useSection.ts";
export type { RenderCbInput } from "./hooks/useSection.ts";
export type {
LazyUrlContext,
RenderJson,
ResolvedSection,
SectionJsonModule,
SerializeContext,
SerializedSection,
} from "./runtime/routes/serialize-section.ts";
export { Murmurhash3 } from "./deps.ts";
export * from "./types.ts";
export { allowCorsFor } from "./utils/http.ts";
export type { StreamProps } from "./utils/invoke.ts";
Expand Down
30 changes: 30 additions & 0 deletions runtime/routes/render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ import { singleFlight } from "../../utils/mod.ts";
import { createHandler, DEBUG_QS } from "../middleware.ts";
import type { PageParams } from "../mod.ts";
import Render, { type PageData } from "./entrypoint.tsx";
import {
type ResolvedSection,
sectionModuleLookup,
type SerializeContext,
serializeResolvedSection,
} from "./serialize-section.ts";

interface Options {
resolveChain?: FieldResolver[];
Expand Down Expand Up @@ -91,6 +97,30 @@ export const handler = createHandler(async (
if (isDebugRequest) {
return Response.json({ debugData: state.vary.debug.build() });
}

if (opts.searchParams.get("format") === "json") {
const serializeCtx: SerializeContext = {
href: opts.href,
pathTemplate: opts.pathTemplate,
renderSalt: opts.renderSalt,
cb: opts.searchParams.get("__cb") ?? undefined,
getSectionModule: await sectionModuleLookup(),
};
const serialized = serializeResolvedSection(
page as ResolvedSection,
serializeCtx,
);
Comment on lines +109 to +112

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Validate root shape before casting page to ResolvedSection.

Line 110 force-casts page, but the serializer uses non-null assertions on metadata.component. If render output is not section-shaped, this can throw and return 500 instead of a controlled client error.

Suggested fix
   if (opts.searchParams.get("format") === "json") {
+    if (
+      !page || typeof page !== "object" ||
+      typeof (page as ResolvedSection).metadata?.component !== "string"
+    ) {
+      throw badRequest({
+        code: "400",
+        message: "JSON format requires a resolved section root",
+      });
+    }
+
     const serializeCtx: SerializeContext = {
       href: opts.href,
       pathTemplate: opts.pathTemplate,
       renderSalt: opts.renderSalt,
       cb: opts.searchParams.get("__cb") ?? undefined,
       getSectionModule: await sectionModuleLookup(),
     };
🤖 Prompt for 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.

In `@runtime/routes/render.tsx` around lines 109 - 112, The
serializeResolvedSection function call casts page to ResolvedSection without
validating the page object's shape first. If the render output is missing
required properties like metadata.component, the serializer's non-null
assertions will throw, causing an unhandled 500 error. Before calling
serializeResolvedSection with the cast, add validation to check that page has
the expected ResolvedSection shape (particularly that page.metadata and
page.metadata.component exist). If the validation fails, return a controlled
error response rather than attempting the cast and serialization.

const jsonResponse = Response.json(serialized);
const shouldCacheFromVary = ctx?.var?.vary?.shouldCache === true;
jsonResponse.headers.set(
"cache-control",
shouldCache && shouldCacheFromVary

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Cache-control logic is duplicated between the new JSON response path and the existing HTML response path. Extract into a shared helper to prevent drift.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At runtime/routes/render.tsx, line 117:

<comment>Cache-control logic is duplicated between the new JSON response path and the existing HTML response path. Extract into a shared helper to prevent drift.</comment>

<file context>
@@ -91,6 +97,30 @@ export const handler = createHandler(async (
+    const shouldCacheFromVary = ctx?.var?.vary?.shouldCache === true;
+    jsonResponse.headers.set(
+      "cache-control",
+      shouldCache && shouldCacheFromVary
+        ? DECO_RENDER_CACHE_CONTROL
+        : "no-store, no-cache, must-revalidate",
</file context>

? DECO_RENDER_CACHE_CONTROL
: "no-store, no-cache, must-revalidate",
);
return jsonResponse;
}

const props = {
params: ctx.req.param(),
url: ctx.var.url,
Expand Down
198 changes: 198 additions & 0 deletions runtime/routes/serialize-section.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
import { Context } from "../../deco.ts";
import { FieldResolver } from "../../engine/core/resolver.ts";

const LAZY_SECTION_PATH = "/Rendering/Lazy.tsx";

export interface ResolvedSection {
Component?: unknown;
props?: Record<string, unknown>;
metadata?: {
component?: string;
resolveChain?: FieldResolver[];
};
}

export type SerializedSection =
| { component: string; props: Record<string, unknown> }
| { component: string; lazyUrl: string };

/**
* Module-level control of how a section renders to JSON.
*
* - `false` — the section has no JSON rendering; it is dropped from the
* serialized output entirely.
* - function — a projection applied to the resolved props before
* serialization. Annotate the parameter with `SectionProps<typeof loader>`
* (or the section's own Props) to keep it compile-checked.
*
* ```ts
* export const renderJson = false;
*
* export const renderJson = (
* { internalOnly, ...rest }: SectionProps<typeof loader>,
* ) => rest;
* ```
*/
export type RenderJson =
// deno-lint-ignore no-explicit-any
| ((props: any) => unknown)
| false;

export interface SectionJsonModule {
renderJson?: RenderJson;
}

export interface LazyUrlContext {
href: string;
pathTemplate: string;
renderSalt?: string;
cb?: string;
}

export interface SerializeContext extends LazyUrlContext {
/**
* Resolves a section module by its component name (resolveType) so the
* serializer can honor its `renderJson` export. When omitted, every
* section serializes with its full resolved props.
*/
getSectionModule?: (component: string) => SectionJsonModule | undefined;
}

/**
* Builds a `getSectionModule` lookup over the active context's merged
* manifest (site + apps), keyed by resolveType.
*/
export const sectionModuleLookup = async (): Promise<
(component: string) => SectionJsonModule | undefined
> => {
const runtime = await Context.active().runtime;
const sections = (runtime?.manifest as unknown as {
sections?: Record<string, SectionJsonModule>;
})?.sections ?? {};
return (component) => sections[component];
};

export function buildLazyUrl(
resolveChain: FieldResolver[],
ctx: LazyUrlContext,
): string {
const params = new URLSearchParams([
["format", "json"],
["props", JSON.stringify({ loading: "eager" })],
["href", ctx.href],
["pathTemplate", ctx.pathTemplate],
[
"resolveChain",
JSON.stringify(FieldResolver.minify(resolveChain.slice(0, -1))),
],
]);
if (ctx.renderSalt) params.set("renderSalt", ctx.renderSalt);
if (ctx.cb) params.set("__cb", ctx.cb);
return `/deco/render?${params}`;
}

function isSectionShape(value: unknown): value is ResolvedSection {
if (!value || typeof value !== "object") return false;
const meta = (value as ResolvedSection).metadata;
return typeof meta?.component === "string";
}

function isLazyComponent(component: string | undefined): boolean {
return !!component?.endsWith(LAZY_SECTION_PATH);
}

function getInnerSection(node: ResolvedSection): ResolvedSection | undefined {
const inner = (node.props as { section?: unknown } | undefined)?.section;
return isSectionShape(inner) ? inner : undefined;
}

function getLoading(node: ResolvedSection): string | undefined {
return (node.props as { loading?: string } | undefined)?.loading;
}

function renderJsonOf(
ctx: SerializeContext,
component: string,
): RenderJson | undefined {
return ctx.getSectionModule?.(component)?.renderJson;
}

/**
* Serializes a resolved section honoring its `renderJson` export.
* Returns `null` when the section opted out (`renderJson === false`) —
* callers must drop it from the output.
*
* A `renderJson` projection cannot run for lazy placeholders (their props
* are not resolved yet); it applies on the lazy fetch itself, when
* `/deco/render?format=json` serializes the resolved section.
*/
export function serializeResolvedSection(
node: ResolvedSection,
ctx: SerializeContext,
): SerializedSection | null {
let current = node;
while (
isLazyComponent(current.metadata?.component) &&
getLoading(current) === "eager"
) {
const inner = getInnerSection(current);
if (!inner) break;
current = inner;
}

if (
isLazyComponent(current.metadata?.component) &&
getLoading(current) === "lazy"
) {
const inner = getInnerSection(current);
const component = inner?.metadata?.component ??
current.metadata!.component!;
if (renderJsonOf(ctx, component) === false) return null;
return {
component,
lazyUrl: buildLazyUrl(current.metadata!.resolveChain ?? [], ctx),
};
}

const component = current.metadata!.component!;
const renderJson = renderJsonOf(ctx, component);
if (renderJson === false) return null;

const props = typeof renderJson === "function"
? renderJson(current.props ?? {})
: current.props ?? {};

return {
component,
props: walkValue(props, ctx) as Record<string, unknown>,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: renderJson projection result is not validated as an object before being emitted as props, causing potential schema/type contract breakage at runtime.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At runtime/routes/serialize-section.ts, line 167:

<comment>`renderJson` projection result is not validated as an object before being emitted as `props`, causing potential schema/type contract breakage at runtime.</comment>

<file context>
@@ -0,0 +1,198 @@
+
+  return {
+    component,
+    props: walkValue(props, ctx) as Record<string, unknown>,
+  };
+}
</file context>

};
}

function walkValue(value: unknown, ctx: SerializeContext): unknown {
if (Array.isArray(value)) {
// Dropped sections (renderJson === false) are removed from arrays so
// consumers iterate a dense list; non-section nulls pass through.
const out: unknown[] = [];
for (const v of value) {
if (isSectionShape(v)) {
const serialized = serializeResolvedSection(v, ctx);
if (serialized !== null) out.push(serialized);
} else {
out.push(walkValue(v, ctx));
}
}
return out;
}
if (value && typeof value === "object") {
if (isSectionShape(value)) {
return serializeResolvedSection(value, ctx);
}
const out: Record<string, unknown> = {};
for (const [k, v] of Object.entries(value)) {
if (k === "Component" && typeof v === "function") continue;
out[k] = walkValue(v, ctx);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Object traversal keeps opted-out sections as null instead of omitting keys, producing inconsistent "drop" semantics versus arrays.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At runtime/routes/serialize-section.ts, line 193:

<comment>Object traversal keeps opted-out sections as `null` instead of omitting keys, producing inconsistent "drop" semantics versus arrays.</comment>

<file context>
@@ -0,0 +1,198 @@
+    const out: Record<string, unknown> = {};
+    for (const [k, v] of Object.entries(value)) {
+      if (k === "Component" && typeof v === "function") continue;
+      out[k] = walkValue(v, ctx);
+    }
+    return out;
</file context>

}
Comment on lines +191 to +194

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Drop object properties for sections that opt out of JSON (renderJson === false).

Line 193 always writes the walked value, so opted-out nested sections become null on object fields instead of being removed. That contradicts the documented “dropped entirely” behavior and creates a contract mismatch between arrays and objects.

Suggested fix
   const out: Record<string, unknown> = {};
   for (const [k, v] of Object.entries(value)) {
     if (k === "Component" && typeof v === "function") continue;
-    out[k] = walkValue(v, ctx);
+    if (isSectionShape(v)) {
+      const serialized = serializeResolvedSection(v, ctx);
+      if (serialized !== null) out[k] = serialized;
+      continue;
+    }
+    out[k] = walkValue(v, ctx);
   }
   return out;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for (const [k, v] of Object.entries(value)) {
if (k === "Component" && typeof v === "function") continue;
out[k] = walkValue(v, ctx);
}
for (const [k, v] of Object.entries(value)) {
if (k === "Component" && typeof v === "function") continue;
if (isSectionShape(v)) {
const serialized = serializeResolvedSection(v, ctx);
if (serialized !== null) out[k] = serialized;
continue;
}
out[k] = walkValue(v, ctx);
}
🤖 Prompt for 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.

In `@runtime/routes/serialize-section.ts` around lines 191 - 194, The loop
iterating through object entries in the serialization block unconditionally
assigns walked values to the output object, causing opted-out nested sections
with renderJson === false to become null properties instead of being removed
entirely. Fix this by checking the result of walkValue and only assigning the
key-value pair to out when the walked value is not null, using continue to skip
adding dropped properties so that objects behave consistently with arrays by
removing opted-out sections entirely rather than keeping them as null.

return out;
}
return value;
}
2 changes: 1 addition & 1 deletion utils/mod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export { adminUrlFor, isAdmin, resolvable } from "./admin.ts";
*/
export { readFromStream } from "./http.ts";
export { metabasePreview } from "./metabase.tsx";
export { tryOrDefault } from "./object.ts";
export { deepOmit, tryOrDefault } from "./object.ts";
export type { DotNestedKeys } from "./object.ts";
export { createServerTimings } from "./timings.ts";
export type { Device } from "./userAgent.ts";
53 changes: 53 additions & 0 deletions utils/object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,3 +146,56 @@ export const tryOrDefault = <R>(fn: () => R, defaultValue: R): R => {
return defaultValue;
}
};

const omitAtPath = (obj: unknown, parts: string[]): unknown => {
if (!obj || typeof obj !== "object" || parts.length === 0) return obj;

const [key, ...rest] = parts;

// `*` fans the remaining path out over every array element / record value.
if (key === "*") {
if (Array.isArray(obj)) {
return obj.map((v) => omitAtPath(v, rest));
}
return Object.fromEntries(
Object.entries(obj as Record<string, unknown>).map(([k, v]) => [
k,
omitAtPath(v, rest),
]),
);
}

// Preserve array shape: apply the same path to each element.
if (Array.isArray(obj)) {
return obj.map((v) => omitAtPath(v, parts));
}

const current = obj as Record<string, unknown>;

if (rest.length === 0) {
const copy = { ...current };
delete copy[key];
return copy;
}

return {
...current,
[key]: omitAtPath(current[key], rest),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When key does not exist in current, this still writes [key]: omitAtPath(undefined, rest) into the result, materializing a path that was never present (e.g., deepOmit({}, "a.b") produces { a: undefined }). Add a guard to return current unchanged when the key is missing.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At utils/object.ts, line 183:

<comment>When `key` does not exist in `current`, this still writes `[key]: omitAtPath(undefined, rest)` into the result, materializing a path that was never present (e.g., `deepOmit({}, "a.b")` produces `{ a: undefined }`). Add a guard to return `current` unchanged when the key is missing.</comment>

<file context>
@@ -146,3 +146,56 @@ export const tryOrDefault = <R>(fn: () => R, defaultValue: R): R => {
+
+  return {
+    ...current,
+    [key]: omitAtPath(current[key], rest),
+  };
+};
</file context>

};
Comment on lines +181 to +184

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not materialize missing branches during deep omit.

At Line 181, nested omission always rebuilds [key], so a missing path like "a.b" on {} produces { a: undefined }. deepOmit should not add new keys while removing others.

Proposed fix
   if (rest.length === 0) {
     const copy = { ...current };
     delete copy[key];
     return copy;
   }
+
+  if (!(key in current)) {
+    return current;
+  }
 
   return {
     ...current,
     [key]: omitAtPath(current[key], rest),
   };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return {
...current,
[key]: omitAtPath(current[key], rest),
};
if (rest.length === 0) {
const copy = { ...current };
delete copy[key];
return copy;
}
if (!(key in current)) {
return current;
}
return {
...current,
[key]: omitAtPath(current[key], rest),
};
🤖 Prompt for 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.

In `@utils/object.ts` around lines 181 - 184, The omitAtPath function at lines
181-184 is unconditionally rebuilding the key property even when it doesn't
exist in the current object, causing missing nested paths to materialize as
undefined properties. Modify the return statement to conditionally include the
key only if it exists in the current object. Check whether the key is present in
the current object before adding it to the spread result, so that paths that
don't exist are not created as new properties with undefined values during the
deep omit operation.

};

/**
* Removes properties from `obj` by path, immutably.
*
* Supports top-level keys (`"seoProps"`), nested dot-notation paths
* (`"page.seo"`), and the `*` wildcard over record/array values
* (`"page.productsMap.*.internalFlag"`). Prefer typed rest-destructuring
* for top-level keys; reach for deepOmit on deep paths and dynamic keys.
*/
export const deepOmit = <T extends object>(obj: T, ...paths: string[]): T => {
let result: unknown = obj;
for (const path of paths) {
result = omitAtPath(result, path.split("."));
}
return result as T;
};
Loading