feat(render): section-controlled JSON output via renderJson export#1209
Open
marcoferreiradev wants to merge 4 commits into
Open
feat(render): section-controlled JSON output via renderJson export#1209marcoferreiradev wants to merge 4 commits into
marcoferreiradev wants to merge 4 commits into
Conversation
New helper serializeResolvedSection turns the resolved page tree into a
JSON-safe shape: strips Component functions, unwraps eager-resolved Lazy
wrappers, and emits {component, lazyUrl} placeholders for deferred Lazies.
Recursive walk covers Sections nested inside arbitrary props.
The /deco/render handler branches on ?format=json before the HTML render
path, returning Response.json(serialized) with the same cache-control
treatment. Single-flight is intentionally not added on the JSON path
(serialization is cheap).
Companion to ADR-0001 in oficina-reserva (mobile-app appJson lazy support).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…n export serializeResolvedSection now honors a section's named export: drops the section from the JSON output (null return — arrays stay dense, object values become null), a function projects the resolved props before serialization. Module lookup runs over the merged manifest via sectionModuleLookup() (Context.active().runtime) and is injected through SerializeContext.getSectionModule — the serializer stays pure and behaves identically when no lookup is provided. /deco/render?format=json passes the lookup, so projections also apply on lazy fetches. Also promotes deepOmit (dot-paths, wildcard over records/arrays, immutable) from oficina-reserva's sdk into @deco/deco/utils.
Extracts the /deco/render __cb computation out of the useSection hook closure into an exported function in the same module. useSection now calls it, and JSON serialization consumers (e.g. the website app's ?renderJson handler) can import the exact same recipe instead of copying it — web and JSON cache invalidation can no longer diverge. Recipe preserved byte-for-byte (raw join semantics, shared sync-only hasher). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3 tasks
|
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)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a JSON serialization path for the ChangesSection JSON Serialization
deepOmit Utility
Sequence DiagramsequenceDiagram
participant Client
participant render.tsx
participant serializeResolvedSection
participant sectionModuleLookup
participant buildLazyUrl
Client->>render.tsx: GET /deco/render?format=json&href=...
render.tsx->>sectionModuleLookup: build component resolver from manifest
sectionModuleLookup-->>render.tsx: getSectionModule fn
render.tsx->>serializeResolvedSection: resolvedPage as ResolvedSection, SerializeContext
loop each nested section
serializeResolvedSection->>sectionModuleLookup: getSectionModule(component)
sectionModuleLookup-->>serializeResolvedSection: SectionJsonModule or undefined
alt lazy placeholder
serializeResolvedSection->>buildLazyUrl: resolveChain, LazyUrlContext
buildLazyUrl-->>serializeResolvedSection: /deco/render?format=json&...
end
end
serializeResolvedSection-->>render.tsx: SerializedSection or null
render.tsx-->>Client: Response.json(serialized) + cache-control
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Contributor
Tagging OptionsShould a new tag be published when this PR is merged?
|
3 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
runtime/routes/serialize-section.ts (2)
156-168: 💤 Low valueNon-null assertions assume valid metadata.
Line 157 uses
current.metadata!.component!without a guard. WhileResolvedSection.metadatais optional, if a malformed node reaches this path, it will throw. Consider adding an early check or returningnullif metadata is missing, for defensive robustness.🛡️ Defensive guard suggestion
+ const component = current.metadata?.component; + if (!component) return null; - const component = current.metadata!.component!; const renderJson = renderJsonOf(ctx, component);🤖 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 156 - 168, The code uses non-null assertions on `current.metadata!.component!` without verifying these properties exist, which can throw at runtime if metadata is missing or malformed. Add an early guard check at the beginning of the block that returns null if `current.metadata` is falsy or if `current.metadata.component` is falsy, ensuring the function safely handles cases where these properties are not present before attempting to access them.
143-155: Clarify whetherbuildLazyUrlshould use the inner section'sresolveChainwhen present.Lines 149–150 extract and check
renderJsonon the inner section's component when it exists, but line 153 always builds the lazy URL using the outer wrapper'sresolveChain. If the inner section was resolved in a different context and has a different resolve path, this mismatch could cause the lazy fetch to reconstruct a different section than the one whoserenderJsonexport was consulted. Either useinner?.metadata?.resolveChain ?? current.metadata!.resolveChainfor consistency, or add a comment explaining why the wrapper's chain is the correct choice regardless of which component is serialized.🤖 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 143 - 155, In the lazy component serialization block, there is an inconsistency between which component's metadata is used for the renderJson check versus which resolveChain is used for the lazy URL. The component to serialize is determined by preferring the inner section's component when available (line 150), but the buildLazyUrl call on line 153 always uses the outer wrapper's resolveChain from current.metadata. To fix this, update the buildLazyUrl call to use inner?.metadata?.resolveChain ?? current.metadata!.resolveChain to ensure the resolve chain matches the component that was selected for serialization, maintaining consistency between the component check and the lazy URL construction.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@utils/object.ts`:
- Around line 181-184: The omitAtPath function currently creates undefined keys
when the next path segment does not exist in the object. Add a conditional check
before assigning the recursive result to the key: only call omitAtPath
recursively and assign the result if current[key] actually exists (is not
undefined). If the key does not exist in the current object, simply return the
current object without adding that key to avoid creating undefined branches in
the result.
---
Nitpick comments:
In `@runtime/routes/serialize-section.ts`:
- Around line 156-168: The code uses non-null assertions on
`current.metadata!.component!` without verifying these properties exist, which
can throw at runtime if metadata is missing or malformed. Add an early guard
check at the beginning of the block that returns null if `current.metadata` is
falsy or if `current.metadata.component` is falsy, ensuring the function safely
handles cases where these properties are not present before attempting to access
them.
- Around line 143-155: In the lazy component serialization block, there is an
inconsistency between which component's metadata is used for the renderJson
check versus which resolveChain is used for the lazy URL. The component to
serialize is determined by preferring the inner section's component when
available (line 150), but the buildLazyUrl call on line 153 always uses the
outer wrapper's resolveChain from current.metadata. To fix this, update the
buildLazyUrl call to use inner?.metadata?.resolveChain ??
current.metadata!.resolveChain to ensure the resolve chain matches the component
that was selected for serialization, maintaining consistency between the
component check and the lazy URL construction.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 272a199e-f884-415b-ac26-d6ac22f56c94
📒 Files selected for processing (7)
hooks/mod.tshooks/useSection.tsmod.tsruntime/routes/render.tsxruntime/routes/serialize-section.tsutils/mod.tsutils/object.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
3 issues found across 7 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- utils/object.ts: omitAtPath is now a no-op for absent paths instead of creating `key: undefined` branches (deepOmit must never add keys). - serialize-section.ts: tighten RenderJson projection return to Record<string, unknown> so a non-object projection fails to type-check — the serializer's cast is now sound by construction. - serialize-section.ts: document why the lazy slot uses the wrapper's resolveChain (it reconstructs the inner on fetch; the inner is not independently resolvable). - mod.ts: drop the Murmurhash3 re-export — computeRenderCb already encapsulates the cache-bust hash; no consumer needs the raw primitive. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="mod.ts">
<violation number="1" location="mod.ts:63">
P1: Potential unintended breaking change: `Murmurhash3` re-export removed from the public barrel (`mod.ts`) without deprecation path or PR description mention.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| SerializeContext, | ||
| SerializedSection, | ||
| } from "./runtime/routes/serialize-section.ts"; | ||
| export * from "./types.ts"; |
Contributor
There was a problem hiding this comment.
P1: Potential unintended breaking change: Murmurhash3 re-export removed from the public barrel (mod.ts) without deprecation path or PR description mention.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mod.ts, line 63:
<comment>Potential unintended breaking change: `Murmurhash3` re-export removed from the public barrel (`mod.ts`) without deprecation path or PR description mention.</comment>
<file context>
@@ -60,7 +60,6 @@ export type {
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";
</file context>
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
1 participant
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.
Summary
Adds a
renderJsonsection export that lets each section control its own JSON serialization, surfaced through a?format=jsonrender branch:renderJsonexport on a section module:false→ the section is dropped from the JSON payload (its prop-loaders short-circuit, seeengine/core/resolver.ts).SectionProps<typeof loader>— no new type to declare).?format=jsonrender branch +serializeResolvedSection(newruntime/routes/serialize-section.ts) that walks the resolved section tree and applies each section'srenderJson.computeRenderCblifted out ofhooks/useSectionso the lazy-section URL builder and the JSON serializer share one cache-bust recipe instead of drifting.deepOmithelper inutils/object.tsfor ergonomic projections.Why
Headless / app consumers need a stable JSON projection of a page's sections without scraping rendered HTML. The alternatives are over-fetching the full resolved tree or maintaining a parallel serializer that drifts from the render path. Co-locating a
renderJsoncontract with each section keeps the JSON shape next to the section and reuses the real render + cache machinery — one source of truth for the cache-bust recipe.How it works
computeRenderCbis exported fromuseSectionso the lazy-section URL builder and the serializer compute the same render callback.Test plan
deno fmt --checkclean on touched filesdeno check mod.tspasses againstmain?asJsonbyte-identical to baseline;?format=jsonmatches the resolved render tree; lazy fetch applies the projectionCompanion PR
Native
?renderJsonsupport in the Fresh handler lands indeco-cx/appsas a separate PR that depends on this one being published.🤖 Generated with Claude Code
Summary by cubic
Adds a JSON render path for pages via
?format=json, letting each section control its JSON with an optionalrenderJsonexport. Tightens typing and utils for predictable output and exposes a sharedcomputeRenderCbfor consistent cache-busting.New Features
?format=jsonon/deco/renderreturns a JSON-safe section tree with proper cache-control.renderJson:falseto omit; function to project resolved props (must return an object); defaults to full props.{ component, lazyUrl }; omitted sections are removed from arrays and becomenullin object fields.serializeResolvedSection,sectionModuleLookup,buildLazyUrl,computeRenderCb, anddeepOmit.Bug Fixes
deepOmitno longer createsundefinedbranches when a path is missing.renderJsonreturn type (object) prevents unsafe projections.Written for commit b409595. Summary will update on new commits.
Summary by CodeRabbit
format=jsonsupport for section render requests, including cache headers tuned to variant behavior.*wildcards.