Sync release/bi-1/8.x with the feature/persist-improvements branch#1486
Merged
sachiniSam merged 12 commits intorelease/bi-1.8.xfrom Feb 19, 2026
Merged
Sync release/bi-1/8.x with the feature/persist-improvements branch#1486sachiniSam merged 12 commits intorelease/bi-1.8.xfrom
sachiniSam merged 12 commits intorelease/bi-1.8.xfrom
Conversation
…into persistImprovements
Introduce Dependent Type Editor related to persist forms
Contributor
📝 WalkthroughWalkthroughAdds a record-field selector feature: new type definitions, a DependentTypeEditor React component for hierarchical record-field selection, EditorFactory routing for the new input type, and small UI toolkit enhancements (checkbox indeterminate and ClickAwayListener sx prop). Changes
Sequence DiagramsequenceDiagram
participant User
participant DependentTypeEditor
participant TypeResolver
participant FormContext
User->>DependentTypeEditor: Mount with RecordSelectorType value
DependentTypeEditor->>TypeResolver: Resolve rootType and referencedTypes -> members
TypeResolver-->>DependentTypeEditor: Return hierarchical members
User->>DependentTypeEditor: Toggle field checkbox
DependentTypeEditor->>DependentTypeEditor: toggleSelection (recursive, cycle-guarded)
DependentTypeEditor->>DependentTypeEditor: compute indeterminate / full-selected states
DependentTypeEditor->>FormContext: Sync updated selection payload
FormContext-->>User: Form value updated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
workspaces/ballerina/ballerina-side-panel/src/components/editors/DependentTypeEditor.tsx (3)
310-315: Mutable ref + forceUpdate is a recognized pattern, but consider documenting the rationale.The "mutate ref then force re-render" pattern works but is non-obvious to future maintainers. A brief comment explaining why mutable data + forceUpdate was chosen over
useStatewith immutable updates (presumably for performance with deep tree mutations) would help.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-side-panel/src/components/editors/DependentTypeEditor.tsx` around lines 310 - 315, The code uses a mutable ref plus a dummy state re-render pattern (see forceUpdate, triggerUpdate, useState and syncToForm) which is non-obvious to future readers; add a concise comment above the useState/triggerUpdate block that explains the rationale (e.g., using a mutable ref for in-place deep-tree updates for performance and calling triggerUpdate to force a lightweight re-render and then syncToForm), mention any trade-offs and that this is intentional (not a bug), and reference the mutable ref usage so maintainers know why useState immutable updates were avoided.
274-276: Avoidas any— use the discriminated union type.The
recordSelectorEntrycan be typed usingRecordFieldSelectorTypefrom the core interfaces instead ofas any, which would provide type-safe access torecordSelectorType.Proposed fix
+ import type { RecordFieldSelectorType } from "@wso2/ballerina-core"; + const recordSelectorEntry = field.types?.find( - (t: any) => t.fieldType === "RECORD_FIELD_SELECTOR" - ) as any; - - const initialData = recordSelectorEntry?.recordSelectorType as RecordSelectorType | undefined; + (t): t is RecordFieldSelectorType => t.fieldType === "RECORD_FIELD_SELECTOR" + ); + + const initialData = recordSelectorEntry?.recordSelectorType;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-side-panel/src/components/editors/DependentTypeEditor.tsx` around lines 274 - 276, The code uses a loose cast for recordSelectorEntry — replace the `as any` with the proper discriminated-union type so TypeScript can narrow the shape; locate the `recordSelectorEntry` assignment in DependentTypeEditor (the find over `field.types` that checks `t.fieldType === "RECORD_FIELD_SELECTOR"`) and cast/annotate the result as `RecordFieldSelectorType` (importing it from the core interfaces) so you can safely access `recordSelectorType` without `any`.
291-295:syncToFormis called inside the effect but excluded from its dependency array.The
eslint-disablesuppresses the warning, but this means the effect captures a stale closure ofsyncToFormifdata,field, orsetValuechange between mount and the next render. Since the effect is intended as mount-only initialization, confirm thatsetValueandfieldare stable references at mount time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-side-panel/src/components/editors/DependentTypeEditor.tsx` around lines 291 - 295, The effect calls register(field.key) and syncToForm() but excludes syncToForm from dependencies, risking a stale closure; make syncToForm stable (wrap it in useCallback capturing setValue and field) and then include that memoized syncToForm in the useEffect dependency array (remove the eslint-disable). Ensure the useCallback for syncToForm references only stable values (e.g., setValue and field.key) so the effect remains effectively mount-only while avoiding stale closures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@workspaces/ballerina/ballerina-side-panel/src/components/editors/DependentTypeEditor.tsx`:
- Around line 358-360: The current filter on members in DependentTypeEditor.tsx
only keeps FIELD members whose own name matches search, so children that match
while their parent does not are dropped; update the filter logic to perform a
hierarchical match: implement a helper (e.g., memberMatchesSearch(member,
search) or hasDescendantMatch(member, search)) that returns true if the member's
name matches OR any descendant's name matches recursively, and use that helper
in place of the second .filter to ensure parents are kept when any nested child
matches the search term.
- Around line 383-392: The TypeTag currently renders even when m?.typeName is
undefined, producing an empty styled badge; update the JSX in
DependentTypeEditor to conditionally render the TypeTag only when m.typeName is
present (or render a small fallback string like "-" if you prefer), leaving the
CheckBox, FieldLabel, and their handlers (handleToggleField, m.name,
selected/isRequired/isPartial) unchanged so the layout and click behavior remain
consistent.
---
Nitpick comments:
In
`@workspaces/ballerina/ballerina-side-panel/src/components/editors/DependentTypeEditor.tsx`:
- Around line 310-315: The code uses a mutable ref plus a dummy state re-render
pattern (see forceUpdate, triggerUpdate, useState and syncToForm) which is
non-obvious to future readers; add a concise comment above the
useState/triggerUpdate block that explains the rationale (e.g., using a mutable
ref for in-place deep-tree updates for performance and calling triggerUpdate to
force a lightweight re-render and then syncToForm), mention any trade-offs and
that this is intentional (not a bug), and reference the mutable ref usage so
maintainers know why useState immutable updates were avoided.
- Around line 274-276: The code uses a loose cast for recordSelectorEntry —
replace the `as any` with the proper discriminated-union type so TypeScript can
narrow the shape; locate the `recordSelectorEntry` assignment in
DependentTypeEditor (the find over `field.types` that checks `t.fieldType ===
"RECORD_FIELD_SELECTOR"`) and cast/annotate the result as
`RecordFieldSelectorType` (importing it from the core interfaces) so you can
safely access `recordSelectorType` without `any`.
- Around line 291-295: The effect calls register(field.key) and syncToForm() but
excludes syncToForm from dependencies, risking a stale closure; make syncToForm
stable (wrap it in useCallback capturing setValue and field) and then include
that memoized syncToForm in the useEffect dependency array (remove the
eslint-disable). Ensure the useCallback for syncToForm references only stable
values (e.g., setValue and field.key) so the effect remains effectively
mount-only while avoiding stale closures.
Comment on lines
+358
to
+360
| return members | ||
| .filter(m => m.kind === "FIELD" && m.name) | ||
| .filter(m => !search || m.name!.toLowerCase().includes(search.toLowerCase())) |
Contributor
There was a problem hiding this comment.
Search filter doesn't propagate to nested children.
The search filter on line 360 only matches immediate members by name. If a child field matches the search term but its parent does not, the child will be invisible because the parent is filtered out first. This is a common tree-search limitation.
If hierarchical search is expected (show parent when child matches), this needs adjustment to check descendants recursively before filtering.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@workspaces/ballerina/ballerina-side-panel/src/components/editors/DependentTypeEditor.tsx`
around lines 358 - 360, The current filter on members in DependentTypeEditor.tsx
only keeps FIELD members whose own name matches search, so children that match
while their parent does not are dropped; update the filter logic to perform a
hierarchical match: implement a helper (e.g., memberMatchesSearch(member,
search) or hasDescendantMatch(member, search)) that returns true if the member's
name matches OR any descendant's name matches recursively, and use that helper
in place of the second .filter to ensure parents are kept when any nested child
matches the search term.
Comment on lines
+383
to
+392
| <CheckBox | ||
| label="" | ||
| checked={m.selected || isRequired} | ||
| indeterminate={isPartial} | ||
| disabled={isRequired} | ||
| onChange={() => handleToggleField(m)} | ||
| /> | ||
|
|
||
| <FieldLabel onClick={() => handleToggleField(m)}>{m.name}</FieldLabel> | ||
| <TypeTag>{m?.typeName}</TypeTag> |
Contributor
There was a problem hiding this comment.
Empty TypeTag rendered when typeName is undefined.
Line 392 renders <TypeTag>{m?.typeName}</TypeTag> unconditionally. When typeName is undefined, this renders an empty styled badge (with background color and padding but no text).
Proposed fix
- <TypeTag>{m?.typeName}</TypeTag>
+ {m?.typeName && <TypeTag>{m.typeName}</TypeTag>}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <CheckBox | |
| label="" | |
| checked={m.selected || isRequired} | |
| indeterminate={isPartial} | |
| disabled={isRequired} | |
| onChange={() => handleToggleField(m)} | |
| /> | |
| <FieldLabel onClick={() => handleToggleField(m)}>{m.name}</FieldLabel> | |
| <TypeTag>{m?.typeName}</TypeTag> | |
| <CheckBox | |
| label="" | |
| checked={m.selected || isRequired} | |
| indeterminate={isPartial} | |
| disabled={isRequired} | |
| onChange={() => handleToggleField(m)} | |
| /> | |
| <FieldLabel onClick={() => handleToggleField(m)}>{m.name}</FieldLabel> | |
| {m?.typeName && <TypeTag>{m.typeName}</TypeTag>} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@workspaces/ballerina/ballerina-side-panel/src/components/editors/DependentTypeEditor.tsx`
around lines 383 - 392, The TypeTag currently renders even when m?.typeName is
undefined, producing an empty styled badge; update the JSX in
DependentTypeEditor to conditionally render the TypeTag only when m.typeName is
present (or render a small fallback string like "-" if you prefer), leaving the
CheckBox, FieldLabel, and their handlers (handleToggleField, m.name,
selected/isRequired/isPartial) unchanged so the layout and click behavior remain
consistent.
Make parent type selected when fields are selected
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
workspaces/ballerina/ballerina-side-panel/src/components/editors/DependentTypeEditor.tsx (1)
326-329:dataRefis never re-initialized iffieldchanges after mount.The guard
if (!dataRef.current && initialData)at lines 326–328 ensures the clone is created only once. If the parent re-renders the component with a differentfield, the ref keeps the original snapshot. Although the component likely renders one field per form, adding a defensive reset mechanism would make the invariant explicit and prevent silent staleness.The
useEffectwatchesfield.keybut doesn't resetdataRef, so the component lacks proper lifecycle handling. Either track field identity withprevFieldKeyRefto clear the ref on change, or addkey={field.key}at the usage site so React remounts the component when field identity changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-side-panel/src/components/editors/DependentTypeEditor.tsx` around lines 326 - 329, dataRef is only initialized once and will hold a stale snapshot when the parent passes a different field; update the component to reset/re-initialize dataRef when the field identity changes by watching field.key (the same value used in the existing useEffect) and clearing or re-cloning initialData into dataRef.current; implement this by adding a useEffect that compares field.key (or tracks prevFieldKeyRef) and sets dataRef.current = undefined (or = JSON.parse(JSON.stringify(initialData))) when the key changes, or alternatively ensure the component is remounted by adding key={field.key} where the editor is used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@workspaces/ballerina/ballerina-side-panel/src/components/editors/DependentTypeEditor.tsx`:
- Around line 155-181: The cycle-guard bug is that hasPartialSelection (and
renderTree) build newVisited (adding member.refs[0]) but then call
resolveChildren with the old visited set, so self-referential types bypass the
guard; fix by passing the updated set (newVisited) into resolveChildren instead
of visited, ensuring resolveChildren sees the added ref for cycle detection —
update calls in hasPartialSelection (and the analogous call in renderTree) to
use newVisited when resolving children.
- Around line 489-497: The render uses rootType.members directly while other
functions (e.g., handleSelectAll) guard against missing members; to fix,
normalize and guard members once (e.g., compute a validated members array from
rootType like const members = rootType?.members || []) and use that variable in
the JSX and calls to renderTree, areAllSelected, and any other places (instead
of rootType.members), and/or add a short early return when rootType or
rootType.members is falsy so renderTree and TreeContainer never receive
undefined.
---
Duplicate comments:
In
`@workspaces/ballerina/ballerina-side-panel/src/components/editors/DependentTypeEditor.tsx`:
- Line 444: The TypeTag badge is rendered even when m?.typeName is undefined,
producing an empty badge; update the JSX in DependentTypeEditor (the <TypeTag>
usage showing <TypeTag>{m?.typeName}</TypeTag>) to render the TypeTag only when
m?.typeName is a non-empty string (e.g., guard with a conditional or &&) so that
no empty styled badge is output when typeName is undefined or empty.
- Around line 410-412: The current filter on members (in
DependentTypeEditor.tsx) only checks m.name for matches and thus hides matching
descendant members when their parent doesn't match; modify the predicate to
perform a recursive/recursive-like check that returns true if the member's name
matches OR any descendant matches. Create/use a helper (e.g.,
memberMatchesRecursive(member, search)) that walks child collections (e.g.,
m.children or m.members) and checks names case-insensitively, then replace the
current .filter(...) with one that calls that helper so parents are kept when
any child matches.
---
Nitpick comments:
In
`@workspaces/ballerina/ballerina-side-panel/src/components/editors/DependentTypeEditor.tsx`:
- Around line 326-329: dataRef is only initialized once and will hold a stale
snapshot when the parent passes a different field; update the component to
reset/re-initialize dataRef when the field identity changes by watching
field.key (the same value used in the existing useEffect) and clearing or
re-cloning initialData into dataRef.current; implement this by adding a
useEffect that compares field.key (or tracks prevFieldKeyRef) and sets
dataRef.current = undefined (or = JSON.parse(JSON.stringify(initialData))) when
the key changes, or alternatively ensure the component is remounted by adding
key={field.key} where the editor is used.
workspaces/ballerina/ballerina-side-panel/src/components/editors/DependentTypeEditor.tsx
Show resolved
Hide resolved
Comment on lines
+489
to
+497
| label="" | ||
| checked={areAllSelected(rootType.members, referencedTypes)} | ||
| onChange={handleSelectAll} | ||
| /> | ||
| <SelectAllText>Select All Fields</SelectAllText> | ||
| </SelectAllRow> | ||
|
|
||
| <TreeContainer> | ||
| {renderTree(rootType.members, "", 0)} |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -i dependenttypeeditorRepository: wso2/vscode-extensions
Length of output: 154
🏁 Script executed:
find . -name "DependentTypeEditor.tsx" -o -name "DependentTypeEditor.ts"Repository: wso2/vscode-extensions
Length of output: 156
🏁 Script executed:
# Check the file contents around the suspected lines
cat -n workspaces/ballerina/ballerina-side-panel/src/components/editors/DependentTypeEditor.tsx | head -n 500 | tail -n 100Repository: wso2/vscode-extensions
Length of output: 4591
🏁 Script executed:
# Find Type definition to check if members is optional
rg -n "interface Type|type Type" --type tsRepository: wso2/vscode-extensions
Length of output: 9093
🏁 Script executed:
# Check the resolveChildren and handleSelectAll functions referenced in the review
rg -n "resolveChildren|handleSelectAll" workspaces/ballerina/ballerina-side-panel/src/components/editors/DependentTypeEditor.tsxRepository: wso2/vscode-extensions
Length of output: 664
🏁 Script executed:
cat -n workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.ts | sed -n '1439,1460p'Repository: wso2/vscode-extensions
Length of output: 787
🏁 Script executed:
# Get more context around Type interface
cat -n workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.ts | sed -n '1439,1500p'Repository: wso2/vscode-extensions
Length of output: 2092
🏁 Script executed:
# Also check the Member interface referenced in renderTree
rg -n "interface Member" --type tsRepository: wso2/vscode-extensions
Length of output: 270
🏁 Script executed:
# Look for areAllSelected function to understand its expectations
rg -n "areAllSelected" workspaces/ballerina/ballerina-side-panel/src/components/editors/DependentTypeEditor.tsx -B3 -A8Repository: wso2/vscode-extensions
Length of output: 1903
🏁 Script executed:
cat -n workspaces/ballerina/ballerina-side-panel/src/components/editors/DependentTypeEditor.tsx | sed -n '37,60p'Repository: wso2/vscode-extensions
Length of output: 1246
🏁 Script executed:
# View more of resolveChildren context
cat -n workspaces/ballerina/ballerina-side-panel/src/components/editors/DependentTypeEditor.tsx | sed -n '1,100p'Repository: wso2/vscode-extensions
Length of output: 4464
Inconsistent handling of rootType.members — add guard for consistency.
The code inconsistently handles rootType.members:
- Line 383 (
handleSelectAll): guards withif (!rootType?.members) return; - Lines 490 and 497 (main render): directly use
rootType.memberswithout checking
Although Type.members is typed as required (members: Member[]), the defensive checks throughout the codebase (e.g., line 51 in resolveChildren: if (!typeObj.members) return []) suggest members can be undefined in practice.
To align with the guard at line 383 and prevent potential crashes if a Type object lacks members:
if (!rootType) {
return ( /* error UI */ );
}
+
+ if (!rootType.members) {
+ return ( /* error UI */ );
+ }Or consolidate the check before use and reference a validated variable:
- if (!rootType) {
+ if (!rootType?.members) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@workspaces/ballerina/ballerina-side-panel/src/components/editors/DependentTypeEditor.tsx`
around lines 489 - 497, The render uses rootType.members directly while other
functions (e.g., handleSelectAll) guard against missing members; to fix,
normalize and guard members once (e.g., compute a validated members array from
rootType like const members = rootType?.members || []) and use that variable in
the JSX and calls to renderTree, areAllSelected, and any other places (instead
of rootType.members), and/or add a short early return when rootType or
rootType.members is falsy so renderTree and TreeContainer never receive
undefined.
kanushka
approved these changes
Feb 19, 2026
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
2 participants
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.
Purpose
$Subject
This includes the feature of multiple persist connection support and Dependent Type editor support
Goals
Approach
UI Component Development
npm run storybookfrom the root directory to view current components.Manage Icons
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit
New Features
Improvements
API