Conversation
…nager and ExtendedLangClient
… DataMapper components
There was a problem hiding this comment.
Pull Request Overview
This PR improves completion support for the Data Mapper's clause editor form, introducing support for join clauses and enhancing the frontend architecture for query expression handling.
- Adds support for JOIN clause type in query expressions with dedicated form fields (lhsExpression, rhsExpression, isOuter)
- Introduces
ClauseConnectorNodecomponent for visualizing clause connections in the diagram - Refactors View interface to support multiple source fields (sourceFields array) instead of single sourceField
- Adds new RPC methods
getFieldPropertyandgetClausePositionfor improved clause positioning and property retrieval
Reviewed Changes
Copilot reviewed 53 out of 53 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| workspaces/ballerina/data-mapper/src/visitors/IntermediateNodeInitVisitor.ts | Adds ClauseConnectorNode initialization in visitor pattern |
| workspaces/ballerina/data-mapper/src/visitors/BaseVisitor.ts | Extends visitor interface with query visit methods |
| workspaces/ballerina/data-mapper/src/utils/model-utils.ts | Implements query traversal logic in model utilities |
| workspaces/ballerina/data-mapper/src/utils/DataMapperContext/DataMapperContext.ts | Reorders constructor parameters for better organization |
| workspaces/ballerina/data-mapper/src/store/store.ts | Adds clauseToAdd state for managing join clause creation |
| workspaces/ballerina/data-mapper/src/index.tsx | Adds getClausePosition prop to DataMapperEditorProps |
| workspaces/ballerina/data-mapper/src/components/Diagram/utils/port-utils.ts | Extends getInputPort to support SubMappingNode |
| workspaces/ballerina/data-mapper/src/components/Diagram/utils/node-utils.ts | Refactors findInputNode to use focusInputRootMap for better input resolution |
| workspaces/ballerina/data-mapper/src/components/Diagram/utils/modification-utils.ts | Adds mapWithJoin function and updates expandArrayFn calls |
| workspaces/ballerina/data-mapper/src/components/Diagram/utils/focus-positioning-utils.ts | Adds ClauseConnectorNode type to focus positioning logic |
| workspaces/ballerina/data-mapper/src/components/Diagram/utils/common-utils.ts | Refactors expandArrayFn to accept multiple input IDs and adds ArrayJoin mapping type |
| workspaces/ballerina/data-mapper/src/components/Diagram/hooks/useDiagramModel.ts | Adds query inputs/outputs to dependency array for model regeneration |
| workspaces/ballerina/data-mapper/src/components/Diagram/Port/model/InputOutputPortModel.ts | Adds ArrayJoin mapping type handling in link creation |
| workspaces/ballerina/data-mapper/src/components/Diagram/Node/index.ts | Exports ClauseConnector components |
| workspaces/ballerina/data-mapper/src/components/Diagram/Node/commons/DataMapperNode.ts | Simplifies array member field resolution |
| workspaces/ballerina/data-mapper/src/components/Diagram/Node/QueryOutput/QueryOutputNode.ts | Removes unused query input/output link creation code |
| workspaces/ballerina/data-mapper/src/components/Diagram/Node/QueryExprConnector/QueryExprConnectorNodeWidget.tsx | Updates expandArrayFn call with array parameter |
| workspaces/ballerina/data-mapper/src/components/Diagram/Node/Input/InputNode.ts | Changes to use sourceFields array and removes focusedMemberId logic |
| workspaces/ballerina/data-mapper/src/components/Diagram/Node/ClauseConnector/index.ts | New file exporting ClauseConnector components |
| workspaces/ballerina/data-mapper/src/components/Diagram/Node/ClauseConnector/ClauseConnectorNodeWidget.tsx | New component for rendering clause connector nodes in diagram |
| workspaces/ballerina/data-mapper/src/components/Diagram/Node/ClauseConnector/ClauseConnectorNodeFactory.tsx | Factory for creating ClauseConnectorNode instances |
| workspaces/ballerina/data-mapper/src/components/Diagram/Node/ClauseConnector/ClauseConnectorNode.ts | Core logic for clause connector nodes with port and link management |
| workspaces/ballerina/data-mapper/src/components/Diagram/LinkState/CreateLinkState.ts | Updates link state to support ClauseConnectorNode |
| workspaces/ballerina/data-mapper/src/components/Diagram/Link/DataMapperLink/DataMapperLink.ts | Adds ArrayJoin mapping type enum |
| workspaces/ballerina/data-mapper/src/components/Diagram/Label/QueryExprLabelWidget.tsx | Removed file - query expression label widget deleted |
| workspaces/ballerina/data-mapper/src/components/Diagram/Label/ExpressionLabelModel.ts | Removes unused isQuery property |
| workspaces/ballerina/data-mapper/src/components/Diagram/Label/ExpressionLabelFactory.tsx | Updates label widget generation to handle ArrayJoin |
| workspaces/ballerina/data-mapper/src/components/Diagram/Diagram.tsx | Registers ClauseConnectorNodeFactory and updates node positioning |
| workspaces/ballerina/data-mapper/src/components/Diagram/Actions/utils.ts | Adds ClauseConnectorNode to intermediate nodes list |
| workspaces/ballerina/data-mapper/src/components/Diagram/Actions/IONodesScrollCanvasAction.ts | Includes ClauseConnectorNode in scroll action logic |
| workspaces/ballerina/data-mapper/src/components/DataMapper/Views/DataMapperView.ts | Changes sourceField to sourceFields array |
| workspaces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClausesPanel.tsx | Adds getClausePosition prop and clauseToAdd state management |
| workspaces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClauseItem.tsx | Passes getClausePosition to ClauseEditor components |
| workspaces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClauseEditor.tsx | Implements JOIN clause support with new form fields and position fetching |
| workspaces/ballerina/data-mapper/src/components/DataMapper/DataMapperEditor.tsx | Passes getClausePosition to ClausesPanel |
| workspaces/ballerina/ballerina-visualizer/src/views/DataMapper/DataMapperView.tsx | Implements getClausePosition and updates completion API calls |
| workspaces/ballerina/ballerina-visualizer/src/views/BI/Forms/FormGeneratorNew/index.tsx | Adds isDataMapperEditor flag to route completions correctly |
| workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionEditor.tsx | Adds DataMapperJoinClauseRhsEditor for join condition completions |
| workspaces/ballerina/ballerina-side-panel/src/components/editors/EditorFactory.tsx | Registers DM_JOIN_CLAUSE_RHS_EXPRESSION field type |
| workspaces/ballerina/ballerina-rpc-client/src/rpc-clients/data-mapper/rpc-client.ts | Adds getFieldProperty and getClausePosition RPC methods |
| workspaces/ballerina/ballerina-rpc-client/src/rpc-clients/bi-diagram/rpc-client.ts | Adds getDataMapperCompletions RPC method |
| workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/utils.ts | Implements focusInputRootMap tracking for input field resolution |
| workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/rpc-manager.ts | Implements getFieldProperty and getClausePosition handlers |
| workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/rpc-handler.ts | Registers new RPC handlers |
| workspaces/ballerina/ballerina-extension/src/rpc-managers/bi-diagram/rpc-manager.ts | Implements getDataMapperCompletions method |
| workspaces/ballerina/ballerina-extension/src/rpc-managers/bi-diagram/rpc-handler.ts | Registers getDataMapperCompletions handler |
| workspaces/ballerina/ballerina-extension/src/core/extended-language-client.ts | Adds language server API methods for clause position and field properties |
| workspaces/ballerina/ballerina-core/src/rpc-types/data-mapper/rpc-type.ts | Defines RPC request types for new methods |
| workspaces/ballerina/ballerina-core/src/rpc-types/data-mapper/index.ts | Exports new API interfaces |
| workspaces/ballerina/ballerina-core/src/rpc-types/bi-diagram/rpc-type.ts | Defines getDataMapperCompletions RPC type |
| workspaces/ballerina/ballerina-core/src/rpc-types/bi-diagram/index.ts | Exports getDataMapperCompletions in API interface |
| workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.ts | Adds PropertyRequest, FieldPropertyRequest, ClausePositionRequest/Response interfaces |
| workspaces/ballerina/ballerina-core/src/interfaces/data-mapper.ts | Adds JOIN clause type and new properties for join clauses |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...aces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClausesPanel.tsx
Show resolved
Hide resolved
...ces/ballerina/data-mapper/src/components/Diagram/Node/ClauseConnector/ClauseConnectorNode.ts
Show resolved
Hide resolved
...aces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClausesPanel.tsx
Show resolved
Hide resolved
...aces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClauseEditor.tsx
Show resolved
Hide resolved
...lerina/data-mapper/src/components/Diagram/Node/ClauseConnector/ClauseConnectorNodeWidget.tsx
Outdated
Show resolved
Hide resolved
...ces/ballerina/data-mapper/src/components/Diagram/Node/ClauseConnector/ClauseConnectorNode.ts
Outdated
Show resolved
Hide resolved
...ces/ballerina/data-mapper/src/components/Diagram/Node/ClauseConnector/ClauseConnectorNode.ts
Outdated
Show resolved
Hide resolved
...ces/ballerina/data-mapper/src/components/Diagram/Node/ClauseConnector/ClauseConnectorNode.ts
Outdated
Show resolved
Hide resolved
...ces/ballerina/data-mapper/src/components/Diagram/Node/ClauseConnector/ClauseConnectorNode.ts
Outdated
Show resolved
Hide resolved
…ompletion-impr-clause
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe PR introduces improved completion support for clause editors by adding RPC endpoints to fetch field properties and clause positions dynamically. New request/response types ( Changes
Sequence DiagramsequenceDiagram
participant UI as Clause Editor UI
participant React as React/Query
participant RPC as RPC Manager
participant Lang as Language Client
UI->>React: Query getClausePosition(targetField, index)
Note over UI: Show ProgressRing
React->>RPC: getClausePosition(ClausePositionRequest)
RPC->>Lang: getClausePosition(params)
Lang-->>RPC: ClausePositionResponse {position: LinePosition}
RPC-->>React: LinePosition
React-->>UI: Cache result
Note over UI: Render form with startLine/endLine<br/>Update ExpressionEditor context
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 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 |
![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)
workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/rpc-manager.ts (1)
20-52: AvoidasyncPromise executors in new RPC helpers (BiomenoAsyncPromiseExecutor)Both new methods wrap an
asyncexecutor insidenew Promise, which Biome flags and which is unnecessary givenlangClient()already returns Promises:async getFieldProperty(params: FieldPropertyRequest): Promise<PropertyResponse> { return new Promise(async (resolve) => { const property = await StateMachine .langClient() .getFieldProperty(params) as PropertyResponse; resolve(property); }); } async getClausePosition(params: ClausePositionRequest): Promise<ClausePositionResponse> { return new Promise(async (resolve) => { const position: any = await StateMachine .langClient() .getClausePosition(params); resolve(position); }); }You can simplify and satisfy the lint by returning the underlying Promise directly:
- async getFieldProperty(params: FieldPropertyRequest): Promise<PropertyResponse> { - return new Promise(async (resolve) => { - const property = await StateMachine - .langClient() - .getFieldProperty(params) as PropertyResponse; - - resolve(property); - }); - } - - async getClausePosition(params: ClausePositionRequest): Promise<ClausePositionResponse> { - return new Promise(async (resolve) => { - const position: any = await StateMachine - .langClient() - .getClausePosition(params); - - resolve(position); - }); - } + async getFieldProperty(params: FieldPropertyRequest): Promise<PropertyResponse> { + return StateMachine + .langClient() + .getFieldProperty(params) as Promise<PropertyResponse>; + } + + async getClausePosition(params: ClausePositionRequest): Promise<ClausePositionResponse> { + return StateMachine + .langClient() + .getClausePosition(params) as Promise<ClausePositionResponse>; + }This removes the lint error and keeps behavior unchanged.
Also applies to: 239-257
♻️ Duplicate comments (1)
workspaces/ballerina/data-mapper/src/index.tsx (1)
27-27: Verify whetherPropertyimport is needed.A past review flagged
Propertyas an unused import. AlthoughLinePositionis now actively used in the newgetClausePositionprop signature (line 71),Propertydoes not appear to be referenced anywhere in this file. Consider removing it if it's not required for type exports or re-exports.
🧹 Nitpick comments (1)
workspaces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClauseItem.tsx (1)
40-55: Index semantics forgetClausePositionvsonAddwhen inserting after a clauseFor edits you pass
indexconsistently to bothClauseEditorandonEdit, but for inserts-after you passindex + 1toClauseEditor(forgetClausePosition) whileonAddreceives the originalindex. This assumes the clause-position service interprets the index used for position lookup as the insertion index, not the source clause index.If that’s the intended contract with
getClausePosition, this wiring is fine; otherwise, you may want to align the index you use for position lookup with the index you pass intoaddClausesfor new clauses.Also applies to: 107-129
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.ts(1 hunks)workspaces/ballerina/ballerina-core/src/rpc-types/data-mapper/index.ts(2 hunks)workspaces/ballerina/ballerina-core/src/rpc-types/data-mapper/rpc-type.ts(2 hunks)workspaces/ballerina/ballerina-extension/src/core/extended-language-client.ts(3 hunks)workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/rpc-handler.ts(4 hunks)workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/rpc-manager.ts(4 hunks)workspaces/ballerina/ballerina-rpc-client/src/rpc-clients/data-mapper/rpc-client.ts(4 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/EditorFactory.tsx(2 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionEditor.tsx(2 hunks)workspaces/ballerina/ballerina-visualizer/src/views/DataMapper/DataMapperView.tsx(5 hunks)workspaces/ballerina/data-mapper/src/components/DataMapper/DataMapperEditor.tsx(2 hunks)workspaces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClauseEditor.tsx(4 hunks)workspaces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClauseItem.tsx(3 hunks)workspaces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClausesPanel.tsx(4 hunks)workspaces/ballerina/data-mapper/src/index.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-20T11:04:33.712Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 898
File: workspaces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClausesPanel.tsx:38-38
Timestamp: 2025-11-20T11:04:33.712Z
Learning: In workspaces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClausesPanel.tsx, the use of `useDMQueryClausesPanelStore.getState()` to access `clauseToAdd` and `setClauseToAdd` (instead of the hook subscription pattern) is intentional to prevent re-renders when these state values change.
Applied to files:
workspaces/ballerina/data-mapper/src/index.tsxworkspaces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClausesPanel.tsxworkspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionEditor.tsxworkspaces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClauseEditor.tsxworkspaces/ballerina/ballerina-visualizer/src/views/DataMapper/DataMapperView.tsxworkspaces/ballerina/ballerina-side-panel/src/components/editors/EditorFactory.tsxworkspaces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClauseItem.tsxworkspaces/ballerina/data-mapper/src/components/DataMapper/DataMapperEditor.tsx
🧬 Code graph analysis (14)
workspaces/ballerina/ballerina-core/src/rpc-types/data-mapper/index.ts (1)
workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.ts (4)
FieldPropertyRequest(480-482)PropertyResponse(484-486)ClausePositionRequest(488-493)ClausePositionResponse(495-497)
workspaces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClausesPanel.tsx (3)
workspaces/ballerina/ballerina-core/src/interfaces/data-mapper.ts (2)
IntermediateClause(216-219)DMFormProps(249-258)workspaces/ballerina/ballerina-extension/src/core/extended-language-client.ts (1)
getClausePosition(857-859)workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/rpc-manager.ts (1)
getClausePosition(249-257)
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionEditor.tsx (2)
workspaces/ballerina/ballerina-side-panel/src/context/form.tsx (1)
useFormContext(70-70)workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.ts (1)
ExpressionProperty(1090-1090)
workspaces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClauseEditor.tsx (3)
workspaces/ballerina/ballerina-core/src/interfaces/data-mapper.ts (2)
IntermediateClause(216-219)DMFormProps(249-258)workspaces/ballerina/ballerina-extension/src/core/extended-language-client.ts (1)
getClausePosition(857-859)workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/rpc-manager.ts (1)
getClausePosition(249-257)
workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.ts (1)
workspaces/ballerina/ballerina-core/src/interfaces/bi.ts (2)
Property(124-144)CodeData(170-190)
workspaces/ballerina/ballerina-extension/src/core/extended-language-client.ts (1)
workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.ts (4)
FieldPropertyRequest(480-482)PropertyResponse(484-486)ClausePositionRequest(488-493)ClausePositionResponse(495-497)
workspaces/ballerina/ballerina-visualizer/src/views/DataMapper/DataMapperView.tsx (2)
workspaces/ballerina/ballerina-extension/src/core/extended-language-client.ts (1)
getClausePosition(857-859)workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/rpc-manager.ts (1)
getClausePosition(249-257)
workspaces/ballerina/ballerina-side-panel/src/components/editors/EditorFactory.tsx (1)
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionEditor.tsx (1)
DataMapperJoinClauseRhsEditor(307-331)
workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/rpc-handler.ts (4)
workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/rpc-manager.ts (2)
getFieldProperty(239-247)getClausePosition(249-257)workspaces/ballerina/ballerina-core/src/rpc-types/data-mapper/rpc-type.ts (2)
getFieldProperty(71-71)getClausePosition(72-72)workspaces/ballerina/ballerina-rpc-client/src/rpc-clients/data-mapper/rpc-client.ts (2)
getFieldProperty(147-149)getClausePosition(151-153)workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.ts (2)
FieldPropertyRequest(480-482)ClausePositionRequest(488-493)
workspaces/ballerina/ballerina-core/src/rpc-types/data-mapper/rpc-type.ts (3)
workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/rpc-manager.ts (2)
getFieldProperty(239-247)getClausePosition(249-257)workspaces/ballerina/ballerina-rpc-client/src/rpc-clients/data-mapper/rpc-client.ts (2)
getFieldProperty(147-149)getClausePosition(151-153)workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.ts (4)
FieldPropertyRequest(480-482)PropertyResponse(484-486)ClausePositionRequest(488-493)ClausePositionResponse(495-497)
workspaces/ballerina/ballerina-rpc-client/src/rpc-clients/data-mapper/rpc-client.ts (3)
workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.ts (4)
FieldPropertyRequest(480-482)PropertyResponse(484-486)ClausePositionRequest(488-493)ClausePositionResponse(495-497)workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/rpc-manager.ts (2)
getFieldProperty(239-247)getClausePosition(249-257)workspaces/ballerina/ballerina-core/src/rpc-types/data-mapper/rpc-type.ts (2)
getFieldProperty(71-71)getClausePosition(72-72)
workspaces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClauseItem.tsx (4)
workspaces/ballerina/ballerina-core/src/interfaces/data-mapper.ts (1)
DMFormProps(249-258)workspaces/ballerina/ballerina-extension/src/core/extended-language-client.ts (1)
getClausePosition(857-859)workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/rpc-manager.ts (1)
getClausePosition(249-257)workspaces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClauseEditor.tsx (1)
ClauseEditor(38-204)
workspaces/ballerina/data-mapper/src/components/DataMapper/DataMapperEditor.tsx (2)
workspaces/ballerina/ballerina-extension/src/core/extended-language-client.ts (1)
getClausePosition(857-859)workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/rpc-manager.ts (1)
getClausePosition(249-257)
workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/rpc-manager.ts (1)
workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.ts (4)
FieldPropertyRequest(480-482)PropertyResponse(484-486)ClausePositionRequest(488-493)ClausePositionResponse(495-497)
🪛 Biome (2.1.2)
workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/rpc-manager.ts
[error] 240-246: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
[error] 250-256: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
🔇 Additional comments (15)
workspaces/ballerina/ballerina-side-panel/src/components/editors/EditorFactory.tsx (1)
27-27: LGTM! Clean integration of the new editor type.The
DataMapperJoinClauseRhsEditoris properly imported and integrated into the factory pattern, following the same prop-forwarding convention as other expression editors.Also applies to: 221-233
workspaces/ballerina/data-mapper/src/index.tsx (1)
71-71: LGTM! Clear and well-typed API extension.The new
getClausePositionprop signature is concise and correctly typed withLinePositionas the return value.workspaces/ballerina/data-mapper/src/components/DataMapper/DataMapperEditor.tsx (1)
142-142: LGTM! Proper prop threading.The
getClausePositionprop is correctly destructured from props and forwarded toClausesPanel, consistent with the existing pattern foraddClausesanddeleteClause.Also applies to: 365-365
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionEditor.tsx (1)
307-331: LGTM! Well-structured completion context wrapper.The
DataMapperJoinClauseRhsEditorcorrectly wrapsExpressionEditorand provides contextual completions by prepending a join clause prefix (from var ${varName} in ${expression} select). The offset adjustment accounts for the prefix length, ensuring accurate cursor positioning for completion requests.The implementation assumes
varNameandexpressionfrom the form are valid; ensure upstream validation handles malformed or empty values to prevent invalid syntax from being sent to the completion provider.workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/rpc-handler.ts (1)
27-27: LGTM! Correct RPC handler registration.The new
getFieldPropertyandgetClausePositionRPC endpoints are properly wired into the messenger, following the existing pattern. Imports and handler delegation torpcMangerare consistent with other endpoints.Also applies to: 40-42, 83-84
workspaces/ballerina/ballerina-core/src/rpc-types/data-mapper/index.ts (1)
45-48: LGTM! Consistent API extension.The
DataMapperAPIinterface is correctly extended withgetFieldPropertyandgetClausePositionmethods, using the appropriate request/response types. The signatures are consistent with other API methods.Also applies to: 68-69
workspaces/ballerina/ballerina-core/src/rpc-types/data-mapper/rpc-type.ts (1)
47-50: LGTM! Well-defined RPC types.The new RPC endpoints
getFieldPropertyandgetClausePositionare properly defined with correct method paths and request/response type mappings, consistent with the existing RPC type definitions.Also applies to: 71-72
workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.ts (1)
480-497: LGTM! Clear interface definitions for clause positioning.The new interfaces are well-structured:
FieldPropertyRequestextendsPropertyRequestwith afieldIdfield for field-specific property retrieval.ClausePositionRequestcaptures the necessary context (filePath, codedata, targetField, index) to locate a specific clause.ClausePositionResponsereturns aLinePositionfor position-aware editing.These interfaces enable the completion support improvements described in the PR objectives.
workspaces/ballerina/ballerina-rpc-client/src/rpc-clients/data-mapper/rpc-client.ts (1)
20-72: New DataMapper RPC client methods look consistent
getFieldPropertyandgetClausePositionare wired identically to the existing client methods: they use the correct request types from@wso2/ballerina-coreand delegate viasendRequesttoHOST_EXTENSION. This keeps the client in sync with the extendedDataMapperAPI.Also applies to: 147-153
workspaces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClausesPanel.tsx (1)
26-35: Propagation ofgetClausePositionthrough ClausesPanel is coherentExtending
ClausesPanelPropswithgetClausePositionand threading it (along withtargetFieldandindex) into both the top-levelClauseEditorand eachClauseItemcleanly exposes clause-position lookup to all clause editors without changing the existing side-panel state management. The data flow looks consistent with the new RPC API.Also applies to: 40-41, 106-115, 121-137
workspaces/ballerina/ballerina-visualizer/src/views/DataMapper/DataMapperView.tsx (3)
287-287: LGTM! Context flag added for DataMapper editor.The
isDataMapperEditorflag properly marks this form as a DataMapper editor, enabling context-aware completion behavior downstream.
509-514: LGTM! Successfully migrated to the new field-property API.The migration from
getPropertytogetFieldPropertycorrectly adds thefieldIdparameter and enables field-specific property retrieval for navigation.
703-703: LGTM! Clause position capability properly wired to DataMapper.The
getClausePositionfunction is correctly passed to the DataMapper component, enabling dynamic clause positioning for context-aware completions.workspaces/ballerina/ballerina-extension/src/core/extended-language-client.ts (2)
377-378: LGTM! API endpoint constants properly defined.The new
DATA_MAPPER_FIELD_PROPERTYandDATA_MAPPER_CLAUSE_POSITIONconstants follow the established naming convention and namespace pattern for data-mapper endpoints.
853-859: LGTM! RPC methods properly implemented.Both
getFieldPropertyandgetClausePositionfollow the established RPC wrapper pattern and correctly use the new API endpoint constants. The return type difference (NOT_SUPPORTED_TYPE union forgetFieldPropertybut not forgetClausePosition) is consistent with similar data-mapper methods.
| const getClausePosition = async (targetField: string, index: number) => { | ||
| try { | ||
| const { position } = await rpcClient.getDataMapperRpcClient().getClausePosition({ | ||
| filePath, | ||
| codedata: viewState.codedata, | ||
| targetField: targetField, | ||
| index: index | ||
| }); | ||
| return position; | ||
| } catch (error) { | ||
| console.error(error); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Improve error handling and add explicit return type.
The function silently swallows errors and returns undefined implicitly, making it difficult for callers to distinguish between "position not found" and "error occurred" scenarios.
Apply this diff:
- const getClausePosition = async (targetField: string, index: number) => {
+ const getClausePosition = async (targetField: string, index: number): Promise<NodePosition | undefined> => {
try {
const { position } = await rpcClient.getDataMapperRpcClient().getClausePosition({
filePath,
codedata: viewState.codedata,
- targetField: targetField,
- index: index
+ targetField,
+ index
});
return position;
} catch (error) {
console.error(error);
+ return undefined;
}
}📝 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.
| const getClausePosition = async (targetField: string, index: number) => { | |
| try { | |
| const { position } = await rpcClient.getDataMapperRpcClient().getClausePosition({ | |
| filePath, | |
| codedata: viewState.codedata, | |
| targetField: targetField, | |
| index: index | |
| }); | |
| return position; | |
| } catch (error) { | |
| console.error(error); | |
| } | |
| } | |
| const getClausePosition = async (targetField: string, index: number): Promise<NodePosition | undefined> => { | |
| try { | |
| const { position } = await rpcClient.getDataMapperRpcClient().getClausePosition({ | |
| filePath, | |
| codedata: viewState.codedata, | |
| targetField, | |
| index | |
| }); | |
| return position; | |
| } catch (error) { | |
| console.error(error); | |
| return undefined; | |
| } | |
| } |
🤖 Prompt for AI Agents
In
workspaces/ballerina/ballerina-visualizer/src/views/DataMapper/DataMapperView.tsx
around lines 363-375, the async getClausePosition swallows errors and returns
undefined implicitly; change its signature to an explicit return type (e.g.,
Promise<number | null>), and in the catch block log the error and re-throw it
(or reject) instead of returning/ignoring, so callers can distinguish a real
error from a "not found" null/number result; update any callers to handle the
thrown error or the explicit null return as needed.
| import { EditorContainer, ProgressRingWrapper } from "./styles"; | ||
| import { Divider, Dropdown, OptionProps, ProgressRing, Typography } from "@wso2/ui-toolkit"; | ||
| import { DMFormProps, DMFormField, DMFormFieldValues, IntermediateClauseType, IntermediateClause, IntermediateClauseProps, LinePosition } from "@wso2/ballerina-core"; | ||
| import { useDMQueryClausesPanelStore } from "../../../../store/store"; | ||
| import { useQuery } from "@tanstack/react-query"; | ||
|
|
||
| export interface ClauseEditorProps { | ||
| index: number; | ||
| targetField: string; | ||
| clause?: IntermediateClause; | ||
| onSubmitText?: string; | ||
| isSaving: boolean; | ||
| onSubmit: (clause: IntermediateClause) => void; | ||
| onCancel: () => void; | ||
| getClausePosition: (targetField: string, index: number) => Promise<LinePosition>; | ||
| generateForm: (formProps: DMFormProps) => JSX.Element; | ||
| } |
There was a problem hiding this comment.
Guard against missing clausePosition when building targetLineRange
The new useQuery integration and ProgressRing gating align well with the clause-position RPC and should help anchor completions to the right place. One minor corner case: if getClausePosition fails or returns undefined, isFetchingTargetLineRange will eventually be false but clausePosition will still be undefined, and you’ll pass that through as both startLine and endLine:
const formProps: DMFormProps = {
targetLineRange: { startLine: clausePosition, endLine: clausePosition },
...
};
...
{isFetchingTargetLineRange ?
<ProgressRingWrapper> ... </ProgressRingWrapper> :
generateForm(formProps)
}To make this more robust, consider also checking clausePosition before rendering the form:
- const formProps: DMFormProps = {
- targetLineRange: { startLine: clausePosition, endLine: clausePosition },
- ...
- }
+ const formProps: DMFormProps | undefined = clausePosition && {
+ targetLineRange: { startLine: clausePosition, endLine: clausePosition },
+ ...
+ };
...
- {isFetchingTargetLineRange ?
+ {isFetchingTargetLineRange || !formProps ?
<ProgressRingWrapper>
<ProgressRing sx={{ height: "20px", width: "20px" }} />
</ProgressRingWrapper> :
- generateForm(formProps)
+ generateForm(formProps)
}This keeps the UI safe even if the position lookup fails, while preserving the intended loading behavior. The new "DM_JOIN_CLAUSE_RHS_EXPRESSION" field type for the RHS join expression also looks appropriate for distinguishing that field in the form.
Also applies to: 38-40, 114-124, 159-200
2 participants
Purpose
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.