-
Notifications
You must be signed in to change notification settings - Fork 72
Add tuple type support in data-mapper #1307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feature/data-mapper-extended-types
Are you sure you want to change the base?
Add tuple type support in data-mapper #1307
Conversation
WalkthroughAdds Changes
sequenceDiagram
participant Client
participant TypeProcessor as TypeProcessor\n(processTuple)
participant Visitor as NodeVisitor
participant PortCreator as PortCreator
participant Renderer as WidgetRenderer
Client->>TypeProcessor: provide IOType(kind=Tuple)
TypeProcessor->>TypeProcessor: processTuple() -> members with ids like parent[0]
TypeProcessor-->>Visitor: return IOType with members
Visitor->>PortCreator: detect Tuple -> choose node & port routines
alt Input flow
PortCreator->>PortCreator: create OUT ports for each member (indexed ids)
PortCreator-->>Renderer: emit ports + metadata (parentFieldKind)
else Output flow
PortCreator->>PortCreator: create IN ports for each member
PortCreator-->>Renderer: emit ports + metadata (parentFieldKind)
end
Renderer->>Renderer: render fields, use bracket-notation ids and pass parentFieldKind to children
Renderer-->>Client: display tuple members as individual ports/fields
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds comprehensive support for tuple types in the data-mapper functionality, enabling the mapping of Ballerina tuple data structures alongside existing Record and Array types.
Changes:
- Added
TypeKind.Tupleenum value to the core type system - Implemented tuple processing logic in backend utilities (processTuple function with bracket notation for indexing)
- Extended UI components (Input nodes, ObjectOutput nodes, and widgets) to render and handle tuple members
- Updated search/filter utilities to support tuple member filtering
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| workspaces/ballerina/ballerina-core/src/interfaces/data-mapper.ts | Added Tuple to TypeKind enum |
| workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/utils.ts | Implemented processTuple function and TypeKind.Tuple case handling with bracket notation for member indexing |
| workspaces/ballerina/data-mapper/src/visitors/IONodeInitVisitor.ts | Extended output node creation to handle Tuple types alongside Records |
| workspaces/ballerina/data-mapper/src/components/Diagram/utils/search-utils.ts | Added tuple filtering in search functions with member-based filtering logic |
| workspaces/ballerina/data-mapper/src/components/Diagram/utils/node-utils.ts | Enhanced field parsing to handle bracket notation for tuple member access |
| workspaces/ballerina/data-mapper/src/components/Diagram/Node/commons/DataMapperNode.ts | Added tuple field processing methods and FQN handling for tuple members with bracket notation |
| workspaces/ballerina/data-mapper/src/components/Diagram/Node/ObjectOutput/ObjectOutputNode.ts | Added tuple member port creation in initPorts |
| workspaces/ballerina/data-mapper/src/components/Diagram/Node/ObjectOutput/ObjectOutputWidget.tsx | Extended to handle both record fields and tuple members |
| workspaces/ballerina/data-mapper/src/components/Diagram/Node/ObjectOutput/ObjectOutputFieldWidget.tsx | Added tuple-specific port naming and member rendering logic |
| workspaces/ballerina/data-mapper/src/components/Diagram/Node/Input/InputNode.ts | Added tuple member handling in port initialization |
| workspaces/ballerina/data-mapper/src/components/Diagram/Node/Input/InputNodeWidget.tsx | Extended to display tuple members |
| workspaces/ballerina/data-mapper/src/components/Diagram/Node/Input/InputNodeTreeItemWidget.tsx | Added tuple member ID construction with bracket notation |
| workspaces/ballerina/data-mapper/src/components/Diagram/Node/Input/InputNodeFactory.tsx | Added Tuple type check for node rendering |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
workspaces/ballerina/data-mapper/src/components/Diagram/Node/commons/DataMapperNode.ts
Outdated
Show resolved
Hide resolved
workspaces/ballerina/data-mapper/src/components/Diagram/Node/commons/DataMapperNode.ts
Outdated
Show resolved
Hide resolved
workspaces/ballerina/data-mapper/src/components/Diagram/utils/search-utils.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/utils.ts`:
- Around line 798-822: The bug is that processTuple reuses the same visitedRefs
Set across all tupleMembers causing later members that reference the same record
to be incorrectly considered recursive; update processTuple so each tuple member
gets its own copy of the visitedRefs when calling processTypeKind (e.g., pass
new Set(visitedRefs) or otherwise clone the set for the call to processTypeKind
inside the map for each member using memberFieldId), ensuring the original
visitedRefs isn't mutated/shared across members.
In
`@workspaces/ballerina/data-mapper/src/components/Diagram/Node/commons/DataMapperNode.ts`:
- Line 249: The tuple OUT header persistence is using collapsedFields while
non-tuple OUT headers use expandedFields, causing restore mismatch; update the
logic in DataMapperNode where collapsed is computed (call to
isHeaderPortCollapsed) so that for OUT ports with tuple types it reads from
expandedFields (same as non-tuple OUT headers) instead of collapsedFields,
ensuring both tuple and non-tuple OUT headers use expandedFields for persistence
and restore via the existing expandedFields handling.
🧹 Nitpick comments (1)
workspaces/ballerina/data-mapper/src/components/Diagram/Node/commons/DataMapperNode.ts (1)
116-117: Tuple memberIOType.idis correctly constructed as a full, parent-qualified path.Verification confirms that
processTuple()in utils.ts (line 805) constructs tuple member IDs as${parentId}[${index}], guaranteeing they are always parent-qualified (e.g.,"person[0]", not just"[0]"). The bracket-notation check ingetInputFieldFQNandgetUnsafeFieldFQNis therefore safe and correct.The suggested defensive guard adding
field?.kind === TypeKind.Tupleand parent path validation would strengthen consistency withgetOutputFieldFQNand add safety against future code changes, but is not required for correctness.
workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/utils.ts
Show resolved
Hide resolved
workspaces/ballerina/data-mapper/src/components/Diagram/Node/commons/DataMapperNode.ts
Outdated
Show resolved
Hide resolved
8bcba9e to
a21c30d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@workspaces/ballerina/data-mapper/src/components/Diagram/Node/ObjectOutput/ObjectOutputFieldWidget.tsx`:
- Around line 95-113: ObjectOutputFieldWidget builds tuple port names by taking
only the root prefix of parentId (substring before first dot) which drops
intermediate path segments; change its logic to mirror
DataMapperNode.getOutputFieldFQN by treating field.id as a potentially full
path: when field.id contains bracket notation (tuple members), prefer using
field.id directly if it already includes the parent path (e.g.,
field.id.startsWith(updatedParentId or parentId)), otherwise concatenate the
full parent path (updatedParentId or parentId) with field.id to preserve
nesting; update the branch that sets portName (variables: parentId,
updatedParentId, field.id, fieldName, portName) so nested prefixes like
output.record1.record2 are retained rather than reduced to only the root.
🧹 Nitpick comments (1)
workspaces/ballerina/data-mapper/src/components/Diagram/Node/ObjectOutput/ObjectOutputFieldWidget.tsx (1)
125-125: Add parentheses for clarity.The operator precedence makes this evaluate correctly, but adding explicit parentheses improves readability and prevents future maintenance errors.
♻️ Suggested improvement
- const fields = (isRecord && field?.fields?.filter(f => f !== null)) || isTuple && field?.members?.filter(m => m !== null); + const fields = (isRecord && field?.fields?.filter(f => f !== null)) || (isTuple && field?.members?.filter(m => m !== null));
...s/ballerina/data-mapper/src/components/Diagram/Node/ObjectOutput/ObjectOutputFieldWidget.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
workspaces/ballerina/data-mapper/src/components/Diagram/Node/commons/DataMapperNode.ts (1)
319-325: Comment contradicts the actual collapse logic.The comment says input headers check
expandedFields, but the code checkscollapsedFields. Either update the comment or adjust the logic to match the intended behavior.✏️ Comment-only fix (if logic is intended)
- // Hence we explicitly check expandedFields for input header ports. + // Hence we explicitly check collapsedFields for input header ports.
🤖 Fix all issues with AI agents
In
`@workspaces/ballerina/data-mapper/src/components/Diagram/Node/commons/DataMapperNode.ts`:
- Around line 273-306: The tuple-branch FQN concatenations can produce
"undefined" when a field name is missing; update getInputFieldFQN,
getUnsafeFieldFQN and getOutputFieldFQN to default the name to an empty string
in their Tuple branches (e.g., use fieldName || '' or field?.name || '') so IDs
never include the literal "undefined" while preserving existing concatenation
logic.
workspaces/ballerina/data-mapper/src/components/Diagram/Node/commons/DataMapperNode.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...s/ballerina/data-mapper/src/components/Diagram/Node/ObjectOutput/ObjectOutputFieldWidget.tsx
Outdated
Show resolved
Hide resolved
...s/ballerina/data-mapper/src/components/Diagram/Node/ObjectOutput/ObjectOutputFieldWidget.tsx
Outdated
Show resolved
Hide resolved
...s/ballerina/data-mapper/src/components/Diagram/Node/ObjectOutput/ObjectOutputFieldWidget.tsx
Outdated
Show resolved
Hide resolved
...s/ballerina/data-mapper/src/components/Diagram/Node/ObjectOutput/ObjectOutputFieldWidget.tsx
Outdated
Show resolved
Hide resolved
| // Use dmType.id directly for tuple members (bracket notation) | ||
| let fieldId: string; | ||
| if (parentFieldKind === TypeKind.Tuple) { | ||
| fieldId = parentId + fieldName; | ||
| } else if (dmType.isFocused) { | ||
| fieldId = fieldName; | ||
| } else { | ||
| fieldId = `${parentId}.${fieldName}`; | ||
| } |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment states "Use dmType.id directly for tuple members (bracket notation)" but the code below doesn't actually use dmType.id. For tuple members, the code constructs fieldId as parentId + fieldName, but since dmType.id already contains the full path with bracket notation (e.g., "parentId[0]"), it should be used directly for consistency. Consider using: fieldId = dmType.id when parentFieldKind === TypeKind.Tuple.
workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/utils.ts
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
workspaces/ballerina/data-mapper/src/components/Diagram/Node/Input/InputNodeTreeItemWidget.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@workspaces/ballerina/data-mapper/src/components/Diagram/Node/ObjectOutput/ObjectOutputFieldWidget.tsx`:
- Around line 97-101: Update the comment above the tuple-handling logic in
ObjectOutputFieldWidget.tsx to reflect the actual implementation: the port name
is built using parentId concatenated with field.name (not field.id);
specifically adjust the comment that currently mentions “field.id” to say
“field.name” and ensure it references the use of parentFieldKind ===
TypeKind.Tuple and the portName construction using parentId and field.name so
the comment matches the code.
...s/ballerina/data-mapper/src/components/Diagram/Node/ObjectOutput/ObjectOutputFieldWidget.tsx
Outdated
Show resolved
Hide resolved
...s/ballerina/data-mapper/src/components/Diagram/Node/ObjectOutput/ObjectOutputFieldWidget.tsx
Outdated
Show resolved
Hide resolved
…nput/InputNodeTreeItemWidget.tsx
…bjectOutput/ObjectOutputFieldWidget.tsx
Purpose
Addresses : wso2/product-ballerina-integrator#2254
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.