Resolve the listeners not showing issue#1554
Resolve the listeners not showing issue#1554samithkavishke merged 4 commits intowso2:release/bi-1.8.xfrom
Conversation
📝 WalkthroughWalkthroughThe changes introduce local position state and refactor ServiceConfigureView rendering to always use the listener config form; they also remove the explicit Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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}
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 (2)
workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsx (2)
699-731:⚠️ Potential issue | 🟡 MinorDouble
fetchServicecalls in the service-save and detach paths.When saving service changes (lines 725–726),
setPosition(...)triggers theuseEffectat line 299 which callsfetchService(position), butfetchServiceis also called directly on line 726. Same pattern inhandleOnDetachListener(lines 664–666). This results in two identical network round-trips.The listener-save loop (lines 706–714) does it correctly — it only calls
setPositionand lets the effect handle the fetch.Pick one pattern consistently. The simplest fix is to remove the direct
fetchServicecalls wheresetPositionalready triggers it:Suggested fix
// Update service changes for (const change of serviceChanges) { const res = await rpcClient.getServiceDesignerRpcClient().updateServiceSourceCode({ filePath: change.filePath, service: change.data as ServiceModel }); const updatedArtifact = res.artifacts.at(0); if (!updatedArtifact) { console.error("No artifact returned after saving service changes"); continue; } setCurrentIdentifier(updatedArtifact.name); setPosition(updatedArtifact.position); - await fetchService(updatedArtifact.position); }And similarly in
handleOnDetachListener:setPosition(updatedArtifact.position); setCurrentIdentifier(updatedArtifact.name); - await fetchService(updatedArtifact.position);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsx` around lines 699 - 731, handleSave currently calls fetchService(...) directly after setPosition(...) for service changes, causing duplicate network calls because the useEffect watching position already invokes fetchService; likewise handleOnDetachListener has the same double-call pattern. Remove the direct calls to fetchService(...) in these places (specifically the await fetchService(updatedArtifact.position) in handleSave and the analogous fetchService call in handleOnDetachListener) and rely on setPosition(...) + the existing useEffect to perform the fetch; keep the listener-save loop unchanged since it already uses setPosition only.
888-896:⚠️ Potential issue | 🟠 Major
ServiceEditViewstill receivesprops.positioninstead of the localpositionstate.After a listener update shifts the service's position,
ServiceEditView(line 891) will still hold the originalprops.position. This means the change-map key generated viahandleFormDirtyChangewill reference the stale position, potentially causing mismatches when looking up dirty/change state.Consider passing the local
positionstate:<ServiceEditView filePath={props.filePath} - position={props.position} + position={position} onChange={handleServiceChange}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsx` around lines 888 - 896, ServiceEditView is being given props.position (the original prop) instead of the component's local position state, so after position updates the child still renders with stale position causing handleFormDirtyChange and change-map keys to mismatch; update the JSX to pass the local position state (the variable named position) into ServiceEditView instead of props.position and ensure any callbacks like onDirtyChange/onValidityChange continue to use that local position when invoking handleFormDirtyChange/handleFormValidityChange so keys reflect the current position.
🤖 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-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsx`:
- Around line 706-714: In the loop over listenerChanges, fix the typo variable
name listnerResponse to listenerResponse to keep naming consistent: replace all
uses of listnerResponse (assigned from
rpcClient.getServiceDesignerRpcClient().updateListenerSourceCode(...)) with
listenerResponse and ensure updatedServiceArtifact is obtained from
listenerResponse.artifacts before calling
setPosition(updatedServiceArtifact.position).
- Line 217: The local state position in ServiceConfigureView is initialized from
props.position but never updated when the prop changes; add a useEffect in
ServiceConfigureView that watches props.position (or props.position.x/y) and
calls setPosition(props.position) when they differ so the internal position
state stays in sync and ServiceEditView (which reads the original
props.position) receives up-to-date coordinates; reference the
position/setPosition state pair and props.position in the effect to detect
changes and update accordingly.
---
Outside diff comments:
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsx`:
- Around line 699-731: handleSave currently calls fetchService(...) directly
after setPosition(...) for service changes, causing duplicate network calls
because the useEffect watching position already invokes fetchService; likewise
handleOnDetachListener has the same double-call pattern. Remove the direct calls
to fetchService(...) in these places (specifically the await
fetchService(updatedArtifact.position) in handleSave and the analogous
fetchService call in handleOnDetachListener) and rely on setPosition(...) + the
existing useEffect to perform the fetch; keep the listener-save loop unchanged
since it already uses setPosition only.
- Around line 888-896: ServiceEditView is being given props.position (the
original prop) instead of the component's local position state, so after
position updates the child still renders with stale position causing
handleFormDirtyChange and change-map keys to mismatch; update the JSX to pass
the local position state (the variable named position) into ServiceEditView
instead of props.position and ensure any callbacks like
onDirtyChange/onValidityChange continue to use that local position when invoking
handleFormDirtyChange/handleFormValidityChange so keys reflect the current
position.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
workspaces/ballerina/ballerina-extension/src/rpc-managers/service-designer/rpc-manager.tsworkspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsx
| const [currentIdentifier, setCurrentIdentifier] = useState<string | null>(null); | ||
|
|
||
| const [isSaving, setIsSaving] = useState<boolean>(false); | ||
| const [position, setPosition] = useState<NodePosition>(props.position); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find where ServiceConfigureView is instantiated/rendered
rg -n "ServiceConfigureView" --type=tsx --type=ts -B3 -A3 | head -100Repository: wso2/vscode-extensions
Length of output: 93
🏁 Script executed:
# Check the component file structure to understand line numbers better
wc -l workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsxRepository: wso2/vscode-extensions
Length of output: 166
🏁 Script executed:
# Look at the listener/service save logic around lines 706-727
sed -n '700,730p' workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsxRepository: wso2/vscode-extensions
Length of output: 1678
🏁 Script executed:
# Check ServiceEditView and how it uses position
rg -n "ServiceEditView" --type=tsx --type=ts -B2 -A2 | grep -A5 -B5 "position"Repository: wso2/vscode-extensions
Length of output: 93
🏁 Script executed:
# Examine the parent component passing position to ServiceConfigureView
rg -n "ServiceConfigureView" --type=tsx --type=ts | head -20Repository: wso2/vscode-extensions
Length of output: 93
🏁 Script executed:
# Use generic file search without specifying tsx
rg "ServiceConfigureView" -B3 -A3 --type=js | head -100Repository: wso2/vscode-extensions
Length of output: 48
🏁 Script executed:
# Look for ServiceEditView usage and position prop around line 891
sed -n '880,920p' workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsxRepository: wso2/vscode-extensions
Length of output: 4313
🏁 Script executed:
# Find all imports and exports to understand component relationship
sed -n '1,50p' workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsxRepository: wso2/vscode-extensions
Length of output: 2103
🏁 Script executed:
# Search for where ServiceConfigureView is used in the project
find workspaces/ballerina/ballerina-visualizer/src -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.js" -o -name "*.jsx" \) -exec grep -l "ServiceConfigureView" {} \;Repository: wso2/vscode-extensions
Length of output: 221
🏁 Script executed:
# Find how ServiceConfigureView is instantiated in MainPanel.tsx
sed -n '1,100p' workspaces/ballerina/ballerina-visualizer/src/MainPanel.tsx | grep -A10 "ServiceConfigureView"Repository: wso2/vscode-extensions
Length of output: 649
🏁 Script executed:
# Full context of ServiceConfigureView instantiation in MainPanel.tsx
rg "ServiceConfigureView" workspaces/ballerina/ballerina-visualizer/src/MainPanel.tsx -B5 -A5Repository: wso2/vscode-extensions
Length of output: 1304
🏁 Script executed:
# Check if ServiceEditView uses the position prop it receives
sed -n '1,100p' workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceEditView.tsx | head -80Repository: wso2/vscode-extensions
Length of output: 2352
🏁 Script executed:
# Look at the fetchService function to understand if double-fetch matters
sed -n '200,300p' workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsx | grep -A20 "fetchService"Repository: wso2/vscode-extensions
Length of output: 97
🏁 Script executed:
# Examine ServiceEditView to see how it uses the position prop
sed -n '1,150p' workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceEditView.tsxRepository: wso2/vscode-extensions
Length of output: 5107
🏁 Script executed:
# Check if position prop is used beyond initial state
rg "props\.position" workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceEditView.tsx -B2 -A2Repository: wso2/vscode-extensions
Length of output: 215
🏁 Script executed:
# Look at the full context around line 217 in ServiceConfigureView
sed -n '210,230p' workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsxRepository: wso2/vscode-extensions
Length of output: 1278
Sync local position state with props.position changes.
The local position state is initialized from props.position but never updated when the prop changes. Since ServiceEditView receives the original props.position (line 891) and depends on it in its effect hook to fetch service data, any internal position updates from setPosition won't propagate to ServiceEditView, causing it to work with stale position data.
Add a sync effect:
Suggested sync effect
const [position, setPosition] = useState<NodePosition>(props.position);
+
+ useEffect(() => {
+ setPosition(props.position);
+ }, [props.position]);📝 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 [position, setPosition] = useState<NodePosition>(props.position); | |
| const [position, setPosition] = useState<NodePosition>(props.position); | |
| useEffect(() => { | |
| setPosition(props.position); | |
| }, [props.position]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsx`
at line 217, The local state position in ServiceConfigureView is initialized
from props.position but never updated when the prop changes; add a useEffect in
ServiceConfigureView that watches props.position (or props.position.x/y) and
calls setPosition(props.position) when they differ so the internal position
state stays in sync and ServiceEditView (which reads the original
props.position) receives up-to-date coordinates; reference the
position/setPosition state pair and props.position in the effect to detect
changes and update accordingly.
| for (const change of listenerChanges) { | ||
| await rpcClient.getServiceDesignerRpcClient().updateListenerSourceCode({ filePath: change.filePath, listener: change.data as ListenerModel }); | ||
| const listnerResponse = await rpcClient.getServiceDesignerRpcClient().updateListenerSourceCode({ filePath: change.filePath, listener: change.data as ListenerModel }); | ||
| const updatedServiceArtifact = listnerResponse.artifacts.filter(artifact => artifact.name ===currentIdentifier).at(0); | ||
| if (!updatedServiceArtifact) { | ||
| console.error("No service artifact returned after updating listener"); | ||
| continue; | ||
| } | ||
| setPosition(updatedServiceArtifact.position); | ||
| } |
There was a problem hiding this comment.
Typo in variable name: listnerResponse → listenerResponse.
Minor naming inconsistency.
Suggested fix
- const listnerResponse = await rpcClient.getServiceDesignerRpcClient().updateListenerSourceCode({ filePath: change.filePath, listener: change.data as ListenerModel });
- const updatedServiceArtifact = listnerResponse.artifacts.filter(artifact => artifact.name ===currentIdentifier).at(0);
+ const listenerResponse = await rpcClient.getServiceDesignerRpcClient().updateListenerSourceCode({ filePath: change.filePath, listener: change.data as ListenerModel });
+ const updatedServiceArtifact = listenerResponse.artifacts.filter(artifact => artifact.name === currentIdentifier).at(0);📝 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.
| for (const change of listenerChanges) { | |
| await rpcClient.getServiceDesignerRpcClient().updateListenerSourceCode({ filePath: change.filePath, listener: change.data as ListenerModel }); | |
| const listnerResponse = await rpcClient.getServiceDesignerRpcClient().updateListenerSourceCode({ filePath: change.filePath, listener: change.data as ListenerModel }); | |
| const updatedServiceArtifact = listnerResponse.artifacts.filter(artifact => artifact.name ===currentIdentifier).at(0); | |
| if (!updatedServiceArtifact) { | |
| console.error("No service artifact returned after updating listener"); | |
| continue; | |
| } | |
| setPosition(updatedServiceArtifact.position); | |
| } | |
| for (const change of listenerChanges) { | |
| const listenerResponse = await rpcClient.getServiceDesignerRpcClient().updateListenerSourceCode({ filePath: change.filePath, listener: change.data as ListenerModel }); | |
| const updatedServiceArtifact = listenerResponse.artifacts.filter(artifact => artifact.name === currentIdentifier).at(0); | |
| if (!updatedServiceArtifact) { | |
| console.error("No service artifact returned after updating listener"); | |
| continue; | |
| } | |
| setPosition(updatedServiceArtifact.position); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsx`
around lines 706 - 714, In the loop over listenerChanges, fix the typo variable
name listnerResponse to listenerResponse to keep naming consistent: replace all
uses of listnerResponse (assigned from
rpcClient.getServiceDesignerRpcClient().updateListenerSourceCode(...)) with
listenerResponse and ensure updatedServiceArtifact is obtained from
listenerResponse.artifacts before calling
setPosition(updatedServiceArtifact.position).
There was a problem hiding this comment.
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 (2)
workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsx (2)
619-625:⚠️ Potential issue | 🟠 Major
handleOnAttachListenernever callssetPosition, leaving local position state stale after attach.
handleOnDetachListenerandhandleSaveboth callsetPosition(updatedArtifact.position)after a mutation, buthandleOnAttachListeneronly callssetCurrentIdentifierandfetchServicedirectly (line 624), neversetPosition. After an attach, the localpositionstate still holds the pre-attach value. Any subsequent operation that readsposition(e.g., theuseEffectre-triggering, or the stale position being passed toServiceEditViewvia the sync effect once it is added) would use the wrong coordinates.🐛 Proposed fix
setCurrentIdentifier(updatedArtifact.name); + setPosition(updatedArtifact.position); await fetchService(updatedArtifact.position);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsx` around lines 619 - 625, handleOnAttachListener currently updates current identifier and refetches the service but never updates the local position state, leaving position stale; after verifying updatedArtifact is present, call setPosition(updatedArtifact.position) (in the same place where setCurrentIdentifier and fetchService are invoked) so the local position state is updated before calling fetchService and closeModal(POPUP_IDS.ATTACH_LISTENER).
573-578:⚠️ Potential issue | 🟡 MinorFallback position in
setServiceListenersstill usesprops.positioninstead of localpositionstate.The fallback branch for inline listeners (when there is no
codedata.lineRange) constructs aProjectStructureArtifactResponseusing the originalprops.positioncoordinates. After any mutation that updates the localpositionstate, this fallback will produce a stale position for those listeners.🐛 Proposed fix
listenersToSet.push({ id: listenerName, name: listenerName, path: props.filePath, type: DIRECTORY_MAP.LISTENER, position: { - startLine: props.position.startLine, - startColumn: props.position.startColumn, - endLine: props.position.endLine, - endColumn: props.position.endColumn, + startLine: position.startLine, + startColumn: position.startColumn, + endLine: position.endLine, + endColumn: position.endColumn, }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsx` around lines 573 - 578, In setServiceListeners, the fallback branch that builds the ProjectStructureArtifactResponse still uses props.position (startLine/startColumn/endLine/endColumn) which can be stale after local position state updates; change that construction to use the component's local position state (the position variable/state referenced in this file) instead of props.position so inline listeners get the current coordinates; update any occurrences in ServiceConfigureView (where position: { startLine: props.position.startLine, ... } is created) to reference position.startLine/position.startColumn/position.endLine/position.endColumn.
♻️ Duplicate comments (2)
workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsx (2)
217-217: Localpositionstate is not synced whenprops.positionchanges.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsx` at line 217, The local state variable position initialized via useState(props.position) in ServiceConfigureView is not updated when props.position changes; add an effect that watches props.position and calls setPosition(props.position) to sync local state (useEffect(() => setPosition(props.position), [props.position])) so the component reflect external updates to props.position while keeping local edits via setPosition.
707-714: TypolistnerResponse→listenerResponse.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsx` around lines 707 - 714, The local variable name listnerResponse is misspelled; rename it to listenerResponse wherever declared and used in this block (the await call to rpcClient.getServiceDesignerRpcClient().updateListenerSourceCode and the subsequent access to .artifacts), so the assignment from updateListenerSourceCode({ filePath: change.filePath, listener: change.data as ListenerModel }) is stored in listenerResponse and you then use listenerResponse.artifacts.filter(...).at(0) to find updatedServiceArtifact before calling setPosition(updatedServiceArtifact.position).
🧹 Nitpick comments (1)
workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsx (1)
1075-1086:AttachedListenerMinimalConfigis dead code after the rendering simplification.The component is defined at lines 1075-1086 but is never instantiated anywhere in the file following the removal of the
showMinimalConfigbranch. It can be deleted.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsx` around lines 1075 - 1086, Remove the unused component AttachedListenerMinimalConfig and any related unused references (e.g., the now-obsolete showMinimalConfig branch) from ServiceConfigureView.tsx; specifically delete the AttachedListenerMinimalConfig function declaration and ensure no imports or variables remain that only served that component so there are no unused-symbol warnings.
🤖 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-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsx`:
- Around line 299-301: The duplicate RPC happens because setPosition(...)
triggers the useEffect([position]) which calls fetchService, while code also
calls await fetchService(updatedArtifact.position) immediately after; remove the
immediate/direct fetchService calls (the awaits following setPosition in
handleSave and the other places that call
fetchService(updatedArtifact.position)) and rely on the existing useEffect that
watches position to perform the fetchService call after setPosition; keep
setPosition as-is so the effect runs, and delete the redundant fetchService
calls (and their awaits) adjacent to setPosition to eliminate the double fetch.
---
Outside diff comments:
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsx`:
- Around line 619-625: handleOnAttachListener currently updates current
identifier and refetches the service but never updates the local position state,
leaving position stale; after verifying updatedArtifact is present, call
setPosition(updatedArtifact.position) (in the same place where
setCurrentIdentifier and fetchService are invoked) so the local position state
is updated before calling fetchService and
closeModal(POPUP_IDS.ATTACH_LISTENER).
- Around line 573-578: In setServiceListeners, the fallback branch that builds
the ProjectStructureArtifactResponse still uses props.position
(startLine/startColumn/endLine/endColumn) which can be stale after local
position state updates; change that construction to use the component's local
position state (the position variable/state referenced in this file) instead of
props.position so inline listeners get the current coordinates; update any
occurrences in ServiceConfigureView (where position: { startLine:
props.position.startLine, ... } is created) to reference
position.startLine/position.startColumn/position.endLine/position.endColumn.
---
Duplicate comments:
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsx`:
- Line 217: The local state variable position initialized via
useState(props.position) in ServiceConfigureView is not updated when
props.position changes; add an effect that watches props.position and calls
setPosition(props.position) to sync local state (useEffect(() =>
setPosition(props.position), [props.position])) so the component reflect
external updates to props.position while keeping local edits via setPosition.
- Around line 707-714: The local variable name listnerResponse is misspelled;
rename it to listenerResponse wherever declared and used in this block (the
await call to rpcClient.getServiceDesignerRpcClient().updateListenerSourceCode
and the subsequent access to .artifacts), so the assignment from
updateListenerSourceCode({ filePath: change.filePath, listener: change.data as
ListenerModel }) is stored in listenerResponse and you then use
listenerResponse.artifacts.filter(...).at(0) to find updatedServiceArtifact
before calling setPosition(updatedServiceArtifact.position).
---
Nitpick comments:
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsx`:
- Around line 1075-1086: Remove the unused component
AttachedListenerMinimalConfig and any related unused references (e.g., the
now-obsolete showMinimalConfig branch) from ServiceConfigureView.tsx;
specifically delete the AttachedListenerMinimalConfig function declaration and
ensure no imports or variables remain that only served that component so there
are no unused-symbol warnings.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsx
| useEffect(() => { | ||
| fetchService(props.position); | ||
| }, [props.position]); | ||
| fetchService(position); | ||
| }, [position]); |
There was a problem hiding this comment.
setPosition + direct fetchService call causes a double fetch on every mutation.
After each setPosition(...) call (lines 664, 713, 737), the useEffect at lines 299-301 will fire — because position changed — and invoke fetchService a second time for the same position. The immediately-following await fetchService(updatedArtifact.position) call then becomes a duplicate RPC round-trip. Over multiple listener changes in handleSave's loop this multiplies further.
There are two clean options:
Option A — drop the direct fetchService calls and rely solely on the effect:
🐛 Option A: let the effect drive fetching
// handleOnDetachListener
setPosition(updatedArtifact.position);
setCurrentIdentifier(updatedArtifact.name);
- await fetchService(updatedArtifact.position);
setChangeMap({});
setDirtyFormMap({});
// handleSave – listener loop
setPosition(updatedServiceArtifact.position);
+ // useEffect will re-fetch once position changes
// handleSave – service loop
setCurrentIdentifier(updatedArtifact.name);
setPosition(updatedArtifact.position);
- await fetchService(updatedArtifact.position);Option B — keep direct fetchService calls, remove position from the effect dependency:
🐛 Option B: effect only for initial mount
useEffect(() => {
fetchService(position);
-}, [position]);
+}, []); // run once on mount; mutations call fetchService directlyAlso applies to: 664-666, 713-713, 737-738
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceConfigureView.tsx`
around lines 299 - 301, The duplicate RPC happens because setPosition(...)
triggers the useEffect([position]) which calls fetchService, while code also
calls await fetchService(updatedArtifact.position) immediately after; remove the
immediate/direct fetchService calls (the awaits following setPosition in
handleSave and the other places that call
fetchService(updatedArtifact.position)) and rely on the existing useEffect that
watches position to perform the fetchService call after setPosition; keep
setPosition as-is so the effect runs, and delete the redundant fetchService
calls (and their awaits) adjacent to setPosition to eliminate the double fetch.
2 participants
Purpose
Fix: wso2/product-ballerina-integrator#2568, wso2/product-ballerina-integrator#2533
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
Bug Fixes
Refactor