feat(render): add observability headers for lazy sections and pages#1029
feat(render): add observability headers for lazy sections and pages#1029
Conversation
- Add x-deco-section header with component type and title - Add x-deco-route header with matched route template - Add x-deco-page header with matched page block name
📝 WalkthroughWalkthroughThe pull request enhances observability in the rendering system by computing a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Tagging OptionsShould a new tag be published when this PR is merged?
|
![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
🤖 Fix all issues with AI agents
In `@runtime/middleware.ts`:
- Around line 364-366: The code references ctx.var.matchedPageBlock but that
property isn't declared on the State types; add matchedPageBlock?: string to the
appropriate state interface (e.g., DecoState in types.ts, alongside
pathTemplate) so ctx.var.matchedPageBlock is typed; update any related
RequestState/DecoSiteState aliases if necessary to propagate the optional string
property through the state union.
🧹 Nitpick comments (2)
runtime/features/render.tsx (2)
150-193: Consider extracting the resolveChain index lookup to avoid duplication.The
resolveChain.findLastIndex((x) => x.type === "resolvable")logic is duplicated from lines 67-68. Consider extracting it to a variable earlier in the function and reusing it here.Also, verify the prefix being stripped in the fallback (
pages-on line 187). The AI summary mentionspage-prefix, so confirm which is correct for your block IDs.♻️ Optional refactor to reduce duplication
let section; + let resolvableIndex = -1; if (resolveChain) { - const index = resolveChain.findLastIndex((x) => x.type === "resolvable"); - section = resolvables[resolveChain[index].value]; + resolvableIndex = resolveChain.findLastIndex((x) => x.type === "resolvable"); + section = resolvables[resolveChain[resolvableIndex].value]; // ... rest of loop } // Later in the fallback: - if (!sectionName && resolveChain) { - const index = resolveChain.findLastIndex((x) => x.type === "resolvable"); - if (index >= 0) { + if (!sectionName && resolveChain && resolvableIndex >= 0) { - const blockId = resolveChain[index].value?.toString(); + const blockId = resolveChain[resolvableIndex].value?.toString();
173-177: Title truncation produces 20 characters, not fewer.The condition checks
title.length > 20but the substring produces 17 chars + 3 for ellipsis = 20 chars total. This is fine, just noting the math for clarity. The header value will include the component type plus this truncated title, so total length could be significant.Consider whether a shorter limit might be more appropriate for HTTP headers, though this is unlikely to cause issues in practice.
| if (ctx?.var?.matchedPageBlock) { | ||
| newHeaders.set("x-deco-page", ctx.var.matchedPageBlock); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find State type definition and related interfaces
fd -t f "\.(ts|tsx)$" runtime/ | head -20
# Look for State type definitions
rg -n "type State|interface State|interface.*State|type.*State\s*=" runtime/ --type ts | head -30Repository: deco-cx/deco
Length of output: 607
🏁 Script executed:
#!/bin/bash
# Search for matchedPageBlock usage/declaration
rg -n "matchedPageBlock" --type ts -C2
# Search for pathTemplate usage/declaration
rg -n "pathTemplate" --type ts -C2 | head -40Repository: deco-cx/deco
Length of output: 2212
🏁 Script executed:
#!/bin/bash
# Get the specific lines mentioned in the review (364-366 and 299-301)
sed -n '295,305p' runtime/middleware.ts
echo "---"
sed -n '357,370p' runtime/middleware.tsRepository: deco-cx/deco
Length of output: 947
🏁 Script executed:
#!/bin/bash
# Get the State type definition from runtime/mod.ts starting at line 52
sed -n '52,150p' runtime/mod.tsRepository: deco-cx/deco
Length of output: 2820
🏁 Script executed:
#!/bin/bash
# Search more carefully for matchedPageBlock - check all file types
rg -n "matchedPageBlock" . --type-list | head -10
# Also search without type restriction
rg -n "matchedPageBlock" . 2>/dev/null | head -20Repository: deco-cx/deco
Length of output: 623
🏁 Script executed:
#!/bin/bash
# Search for where pathTemplate is set/defined in ctx.var
rg -n "pathTemplate\s*=" runtime/middleware.ts -B2 -A2 | head -30
# Search for where matchedPageBlock might be set
rg -n "matchedPageBlock\s*=" runtime/ -B2 -A2 | head -30Repository: deco-cx/deco
Length of output: 38
🏁 Script executed:
#!/bin/bash
# Find RequestState definition
rg -n "interface RequestState|type RequestState" runtime/ --type ts -A 20
# Also search in the entire codebase
rg -n "RequestState" runtime/ --type ts | head -20Repository: deco-cx/deco
Length of output: 388
🏁 Script executed:
#!/bin/bash
# Check middleware.ts imports to understand the context structure
head -50 runtime/middleware.ts | grep -E "^import|^export|interface|type"Repository: deco-cx/deco
Length of output: 954
🏁 Script executed:
#!/bin/bash
# Search for where var properties might be set or typed
rg -n "ctx\.var\s*=" runtime/middleware.ts -B2 -A2 | head -40
# Look for type definitions of ctx.var
rg -n "var\s*:" runtime/middleware.ts -B2 -A2 | head -40Repository: deco-cx/deco
Length of output: 38
🏁 Script executed:
#!/bin/bash
# Find and read the RequestState definition
rg -n "interface RequestState|type RequestState" blocks/utils.tsx -A 50 | head -80Repository: deco-cx/deco
Length of output: 1262
🏁 Script executed:
#!/bin/bash
# Also check the full blocks/utils.tsx file structure
wc -l blocks/utils.tsx
head -100 blocks/utils.tsxRepository: deco-cx/deco
Length of output: 2793
🏁 Script executed:
#!/bin/bash
# Look for the handler function signature to see what type ctx has
rg -n "export.*handler|function.*handler" runtime/middleware.ts -B5 -A10 | head -50
# Check for any ctx typing in middleware.ts
rg -n "ctx:" runtime/middleware.ts | head -20Repository: deco-cx/deco
Length of output: 126
🏁 Script executed:
#!/bin/bash
# Search for where matchedPageBlock might be set in the codebase
rg -n "matchedPageBlock" . -B5 -A5 2>/dev/null
# Also check if this is a recent addition not yet in type definitions
git log --oneline -20 -- runtime/middleware.ts 2>/dev/null || echo "Git history not available"Repository: deco-cx/deco
Length of output: 866
🏁 Script executed:
#!/bin/bash
# Check if ctx.var has type definitions somewhere in middleware or context handling
rg -n "\.var\s*\?" runtime/middleware.ts -B3 -A3 | head -60Repository: deco-cx/deco
Length of output: 2015
🏁 Script executed:
#!/bin/bash
# Find DecoMiddlewareContext definition
rg -n "type DecoMiddlewareContext|interface DecoMiddlewareContext" runtime/ --type ts -A 30Repository: deco-cx/deco
Length of output: 7413
🏁 Script executed:
#!/bin/bash
# Check what type ctx.var has - search for var property definitions
rg -n "var\s*:" runtime/ blocks/ --type ts | grep -E "var\s*:" | head -20
# Also check Hono context types since this appears to be Hono middleware
rg -n "HttpContext|Context<" runtime/middleware.ts --type ts -A5 | head -40Repository: deco-cx/deco
Length of output: 849
🏁 Script executed:
#!/bin/bash
# Find DecoRouteState definition - this is what ctx.var should be
rg -n "type DecoRouteState|interface DecoRouteState" runtime/ --type ts -A 40Repository: deco-cx/deco
Length of output: 2467
🏁 Script executed:
#!/bin/bash
# Double-check by searching for where pathTemplate and matchedPageBlock might be assigned
rg -n "\.var\.pathTemplate\s*=" runtime/ --type ts -B2 -A2
# Also search for where these fields are assigned to ctx or context
rg -n "pathTemplate|matchedPageBlock" runtime/ --type ts -B3 -A3 | grep -E "=|set|assign" -B3 -A3Repository: deco-cx/deco
Length of output: 3932
🏁 Script executed:
#!/bin/bash
# Find DecoState definition to see if it has pathTemplate or matchedPageBlock
rg -n "type DecoState|interface DecoState" --type ts -A 30 | head -60Repository: deco-cx/deco
Length of output: 1392
🏁 Script executed:
#!/bin/bash
# Check if State type extends something else that might have these properties
rg -n "DecoState<" runtime/mod.ts --type ts -B5 -A5
# Also verify what DecoSiteState contains
rg -n "DecoSiteState|interface DecoSiteState|type DecoSiteState" --type ts -A20 | head -50Repository: deco-cx/deco
Length of output: 2525
Add matchedPageBlock property to the State type definition.
The code at lines 364-366 references ctx.var.matchedPageBlock, but this property is not declared in the State type. Unlike pathTemplate (which is defined in DecoState at types.ts:130), matchedPageBlock does not exist in DecoState, RequestState, or DecoSiteState. You need to add this property to the appropriate type definition so the code type-checks correctly.
🧰 Tools
🪛 GitHub Actions: ci
[error] 364-364: TypeScript error: Property 'matchedPageBlock' does not exist on type 'Readonly<ContextVariableMap & DecoSiteState & { deco: Deco; url: URL; correlationId?: string | undefined; ... 6 more ...; routes?: Route[] | undefined; } & RequestState>'.
[error] 365-365: TypeScript error: Property 'matchedPageBlock' does not exist on type 'Readonly<ContextVariableMap & DecoSiteState & { deco: Deco; url: URL; correlationId?: string | undefined; ... 6 more ...; routes?: Route[] | undefined; } & RequestState>'.
🤖 Prompt for AI Agents
In `@runtime/middleware.ts` around lines 364 - 366, The code references
ctx.var.matchedPageBlock but that property isn't declared on the State types;
add matchedPageBlock?: string to the appropriate state interface (e.g.,
DecoState in types.ts, alongside pathTemplate) so ctx.var.matchedPageBlock is
typed; update any related RequestState/DecoSiteState aliases if necessary to
propagate the optional string property through the state union.
1 participant
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.