fix: infer image, cta, and rtf types in local-editor#1247
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
Walkthrough
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@packages/visual-editor/src/vite-plugin/local-editor/data.ts`:
- Around line 115-127: The template-sibling snapshot loading in
getLocalEditorDocument is too brittle because readJsonFile errors from
non-selected documents can abort the whole request. Update the templateDocuments
collection logic to handle parse failures per sibling snapshot, emit a
diagnostic for the malformed file, and continue building entityFields from only
the snapshots that were read successfully. Keep the selected document path
unchanged and use the existing getLocalEditorDocument, readJsonFile, and
inferEntityFields flow to locate the fix.
In `@packages/visual-editor/src/vite-plugin/local-editor/entityFields.ts`:
- Around line 218-223: The isRichTextValue predicate is too broad and is
reclassifying unrelated objects as rich text. Tighten the check in
isRichTextValue so it only returns true for the actual rich-text shape, such as
requiring a json object with the expected rich-text structure (for example
json.root) or a valid html string. Keep the change localized to isRichTextValue
in entityFields.ts so the type inference no longer promotes false positives to
type.rich_text_v2.
- Around line 277-279: The toDisplayName helper only strips the first “c_” from
the full path, so nested custom fields can still display incorrectly or lose
non-prefix matches. Update toDisplayName in entityFields.ts to process each path
segment separately, removing the “c_” prefix from every segment before
formatting the display name, and keep the rest of the path-to-label conversion
logic unchanged.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9128723e-fe2d-43ed-96c2-5cf9f4821fb8
📒 Files selected for processing (3)
packages/visual-editor/src/vite-plugin/local-editor/data.tspackages/visual-editor/src/vite-plugin/local-editor/entityFields.test.tspackages/visual-editor/src/vite-plugin/local-editor/entityFields.ts
|
|
||
| const isCtaValue = (value: Record<string, unknown>): boolean => { | ||
| return typeof value.label === "string" && typeof value.link === "string"; | ||
| return ( |
There was a problem hiding this comment.
What happens when this returns false? Anything detrimental?
Would it be better to do linkType && (label || link)? Or even JUST linkType?
There was a problem hiding this comment.
ooh yeah linkType && (label || link) is probably good. If it doesn't match, then it doesn't show up in type.cta dropdowns... We might eventually need to pull the full schema so that the local-editor is more functional with all data types, but this will get us the main types for now
812f513
into
2026-custom-components-templates
We infer the types of entity document fields in the local-editor, but RTF, Image, and CTA fields were just being picked up as generic objects. This means that they wouldn't show up in the correct dropdowns (particularly for createItemSource). This checks for their subfields and narrows the type if there's a match.
Also removes the
c_prefix when generating the display name.