feat: implement section-specific cache busting based on config hash#1052
feat: implement section-specific cache busting based on config hash#1052
Conversation
Replace global revisionId with section-specific configuration hash in cache-busting parameter (__cb). This ensures only sections with changed configuration get new cache keys, allowing unchanged sections to maintain their cache across deployments. Changes: - Add FieldResolver.resolveFromChain() utility to extract section config from resolveChain - Update useSection to hash actual section config instead of global revisionId - Refactor render.tsx to use new resolveFromChain utility (DRY) - Keep deploymentId in hash for now (to be evaluated for removal) This addresses the issue where changing one section invalidated cache for all sections site-wide. Co-authored-by: Cursor <cursoragent@cursor.com>
Tagging OptionsShould a new tag be published when this PR is merged?
|
📝 WalkthroughWalkthroughAdds a public FieldResolver.resolveFromChain helper to compute a nested value from a resolve chain and integrates it into the section hook and render flow; also updates the hook's cache hash inputs to include deterministic sectionConfig and props. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
runtime/features/render.tsx (1)
107-110:⚠️ Potential issue | 🔴 CriticalPotential
TypeErroronsection.__resolveTypewhensectionisundefined.Line 109 accesses
section.__resolveTypewithout optional chaining, butsectioncan beundefined(line 63-65 returnsundefinedwhenresolveChainis missing or contains no resolvable). If anHttpErroris thrown in this scenario, this will crash with aTypeError, masking the original error.Note the inconsistency: line 98 correctly uses
section?.__resolveType, but line 109 does not.🐛 Proposed fix
const newResolveChain = [...resolveChain ?? [], { type: "resolver", - value: section.__resolveType, + value: section?.__resolveType, }];
🤖 Fix all issues with AI agents
In `@hooks/useSection.ts`:
- Around line 91-92: Remove the extra trailing spaces on the line containing
JSON.stringify(props) that precede the inline comment to satisfy deno fmt; run
`deno fmt` (or fix the spacing so the comment aligns like the previous line) so
the two lines with JSON.stringify(sectionConfig) and JSON.stringify(props) have
consistent spacing and formatting and pass `deno fmt --check`.
- Around line 90-96: The cbString composition uses JSON.stringify(sectionConfig)
and JSON.stringify(props) which return the JS value undefined (coerced to an
empty segment) when those values are undefined (e.g., from resolveFromChain),
causing distinct undefined-reasons to collide; update the cbString construction
to deterministic string fallbacks instead of raw JSON.stringify(undefined) —
e.g., for sectionConfig and props use a nullish check or sentinel (e.g.,
sectionConfig === undefined ? "__absent_sectionConfig__" :
JSON.stringify(sectionConfig)) and the same pattern for props so cbString
reliably differentiates missing vs present-empty values while keeping the
existing vary, stableHref, ctx?.deploymentId parts.
🧹 Nitpick comments (2)
engine/core/resolver.ts (1)
134-157: Loop can start fromindex + 1instead of0to avoid unnecessary iterations.The loop at line 150 starts from
0and skips all entries beforeindexviacontinue. Starting fromindex + 1directly is clearer and avoids the redundant iterations and the multi-conditioncontinue.Also, the return type
any | undefinedcollapses to justanyin TypeScript, so the| undefinedpart is misleading — it suggests narrowing that doesn't actually exist at the type level.♻️ Suggested refactor
resolveFromChain: ( resolveChain: FieldResolver[], resolvables: Record<string, any>, - ): any | undefined => { + ): any => { if (!resolveChain || !resolvables) { return undefined; } const index = resolveChain.findLastIndex((x) => x.type === "resolvable"); if (index < 0) { return undefined; } let value = resolvables[resolveChain[index].value]; // Navigate through the resolveChain to get the specific nested value - for (let it = 0; it < resolveChain.length; it++) { + for (let it = index + 1; it < resolveChain.length; it++) { const item = resolveChain[it]; - if (it < index || item.type !== "prop") continue; + if (item.type !== "prop") continue; value = value?.[item.value]; } return value; },hooks/useSection.ts (1)
95-95: TODO-like comment left in production code.The comment
// Understand if we can remove itreads like an open question. Consider creating a tracking issue and referencing it here, or resolving the question before merging.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="hooks/useSection.ts">
<violation number="1" location="hooks/useSection.ts:91">
P3: Using `JSON.stringify` for hashing is potentially unstable because object key order is not guaranteed to be deterministic (though often consistent in V8). If the configuration object keys are reordered (e.g. by the CMS), it will cause unnecessary cache busts. Consider using a stable stringify function or ensuring the input is canonical.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Replace JSON.stringify with stableStringify to ensure deterministic hash generation. JSON.stringify doesn't guarantee object key order, which could cause unnecessary cache busts when the same configuration has keys in different order. stableStringify recursively sorts object keys, ensuring the same object structure always produces the same string. Co-authored-by: Cursor <cursoragent@cursor.com>
| const cbString = [ | ||
| revisionId, | ||
| stableStringify(sectionConfig), // Section-specific config (deterministic) | ||
| stableStringify(props), // Partial props override (deterministic) |
There was a problem hiding this comment.
this wasn't taking into consideration before, why do we need this now?
what you want is a deterministic way to check if this specific section has changed somehow, based on straight prop change or global change. this can be done once "onConfigChange" and then remove revision id
3 participants
Replace global revisionId with section-specific configuration hash in cache-busting parameter (__cb). This ensures only sections with changed configuration get new cache keys, allowing unchanged sections to maintain their cache across deployments.
Changes:
This addresses the issue where changing one section invalidated cache for all sections site-wide.
Summary by cubic
Implements section-specific cache busting by hashing the resolved section config and props, so only changed sections get new cache keys. Also makes the hash deterministic to avoid unnecessary cache busts.
New Features
Bug Fixes
Written for commit 99d59b3. Summary will update on new commits.
Summary by CodeRabbit
Refactor
Chores