Fix external form model updates without remounting - #120
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (8)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe form now refreshes from external models without remounting fields. It separates external updates from local edits, preserves drafts and focus-related field state, avoids refresh callbacks, and synchronizes map entries, RAW values, numeric input, and ChangesExternal form refresh
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant Parent
participant KaotoForm
participant Field
Parent->>KaotoForm: provide updated model
KaotoForm->>Field: refresh displayed value
Field-->>KaotoForm: emit local edit
KaotoForm-->>Parent: notify local model change
Merge Risk: ⚪ Minimal · up to The form refresh changes are covered across model synchronization, local drafts, maps, RAW values, and oneOf selection behavior. No actionable merge risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Git: CodeRabbit could not clone the repository, so clone-backed analysis was skipped and this review may be incomplete. Verify repository clone access, such as SSH credentials, before requesting another full review. If clone access is intentionally unavailable, use 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. A rabbit sees the fields refresh, Comment |
|
lordrip
left a comment
There was a problem hiding this comment.
I don't think this is the way to move forward with this issue.
The problem is that we're fighting and providing workarounds to an architectural flaw from my initial design, I started it as a controlled component, but along the way, I wanted to add additional features, like supporting String and Number in the StringField component and that derived into having a mix between controlled and uncontrolled behaviors.
This is clear when we see we receive data from the parent, plus keep track of the internal state and now we need to capture what each component emitted themselves.
To unblock the current issues, let's move forward with this approach, and then move the main branch of this library to Kaoto directly, that will allow faster iteration cycles and we can fix the issues differently there.
For more information: https://legacy.reactjs.org/docs/uncontrolled-components.html



Closes #119
Updating
KaotoForm's model now refreshes the existing fields without remounting the form or emitting another change event. This lets Kaoto's properties panel follow source-editor changes while keeping nested expression forms and input focus intact.The fix synchronizes string/RAW values, key/value entries and the selected
oneOfvariant. Local numeric drafts and temporary duplicate map keys remain editable, including when a parent immediately echoes the updated model. Expression selection also handles clearing, subsequent external updates and refreshed schemas.Targets the
1.xmaintenance branch used by Kaoto. This moves the workaround currently carried as a Yarn patch into the library's TypeScript sources, with regression coverage.Validation
yarn buildpasses.yarn test --runInBand: 407 tests and 21 snapshots pass, including 9 added regression tests.git diff --checkpasses.1.xbranch; the output is unchanged.Related: KaotoIO/kaoto#3938
Summary by CodeRabbit
NaN, are preserved correctly during controlled updates.