Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ export class ServiceDesignerRpcManager implements ServiceDesignerAPI {
this.ensureFileExists(targetFile);
params.filePath = targetFile;
const res: ListenerSourceCodeResponse = await context.langClient.updateListenerSourceCode(params);
const artifacts = await updateSourceCode({ textEdits: res.textEdits, artifactData: { artifactType: DIRECTORY_MAP.LISTENER }, description: params.listener.name + ' Update' });
const artifacts = await updateSourceCode({ textEdits: res.textEdits, description: params.listener.name + ' Update' });
const result: UpdatedArtifactsResponse = {
artifacts: artifacts
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ export function ServiceConfigureView(props: ServiceConfigureProps) {
const [currentIdentifier, setCurrentIdentifier] = useState<string | null>(null);

const [isSaving, setIsSaving] = useState<boolean>(false);
const [position, setPosition] = useState<NodePosition>(props.position);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Find where ServiceConfigureView is instantiated/rendered
rg -n "ServiceConfigureView" --type=tsx --type=ts -B3 -A3 | head -100

Repository: 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.tsx

Repository: 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.tsx

Repository: 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 -20

Repository: wso2/vscode-extensions

Length of output: 93


🏁 Script executed:

# Use generic file search without specifying tsx
rg "ServiceConfigureView" -B3 -A3 --type=js | head -100

Repository: 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.tsx

Repository: 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.tsx

Repository: 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 -A5

Repository: 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 -80

Repository: 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.tsx

Repository: 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 -A2

Repository: 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.tsx

Repository: 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.

Suggested change
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.

const [existingListenerType, setExistingListenerType] = useState<string>(""); // Example: "Listener", "CdcListener"

const [selectedListener, setSelectedListener] = useState<string | null>(null);
Expand Down Expand Up @@ -296,8 +297,8 @@ export function ServiceConfigureView(props: ServiceConfigureProps) {
}, []);

useEffect(() => {
fetchService(props.position);
}, [props.position]);
fetchService(position);
}, [position]);
Comment on lines 299 to +301
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

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 directly

Also 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.


useEffect(() => {
if (props.listenerName) {
Expand Down Expand Up @@ -475,6 +476,10 @@ export function ServiceConfigureView(props: ServiceConfigureProps) {
// Set the service model
setServiceModel(res.service);
setConfigTitle(`${getDisplayServiceName(res.service)} Configuration`);
// Set the current identifier from the service name
if (res.service.name && !currentIdentifier) {
setCurrentIdentifier(res.service.name);
}
// Set the service listeners
setServiceListeners(res.service);
// Find the listener type
Expand Down Expand Up @@ -656,6 +661,7 @@ export function ServiceConfigureView(props: ServiceConfigureProps) {
console.error("No artifact returned after detaching listener");
return;
}
setPosition(updatedArtifact.position);
setCurrentIdentifier(updatedArtifact.name);
await fetchService(updatedArtifact.position);
setChangeMap({});
Expand Down Expand Up @@ -698,8 +704,16 @@ export function ServiceConfigureView(props: ServiceConfigureProps) {
const serviceChanges = changes.filter((c) => c.isService);
// Listeners first, then service last
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);
}
Comment on lines 706 to 714
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Typo in variable name: listnerResponselistenerResponse.

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.

Suggested change
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).


// 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);
Expand All @@ -708,6 +722,7 @@ export function ServiceConfigureView(props: ServiceConfigureProps) {
continue;
}
setCurrentIdentifier(updatedArtifact.name);
setPosition(updatedArtifact.position);
await fetchService(updatedArtifact.position);
}
setChangeMap({});
Expand Down Expand Up @@ -1013,14 +1028,6 @@ function ServiceConfigureListenerEditView(props: ServiceConfigureListenerEditVie
onDirtyChange?.(isDirty, filePath, position);
}

// Check if this is a legacy listener (legacy FTP listeners carry path/folderPath in listener properties)
const isLegacyListener =
listenerModel?.properties?.folderPath !== undefined ||
listenerModel?.properties?.path !== undefined;

// For attached listeners in new system (no folderPath in listener), show only monitoring path
const showMinimalConfig = isAttachedListener && !isLegacyListener;

return (
<ServiceConfigureListenerEditViewContainer>
{!listenerModel &&
Expand All @@ -1029,7 +1036,7 @@ function ServiceConfigureListenerEditView(props: ServiceConfigureListenerEditVie
<Typography variant="h3" sx={{ marginTop: '16px' }}>Loading...</Typography>
</LoadingContainer>
}
{listenerModel && !showMinimalConfig &&
{listenerModel &&
<ListenerConfigForm
listenerModel={listenerModel}
filePath={filePath}
Expand All @@ -1041,14 +1048,6 @@ function ServiceConfigureListenerEditView(props: ServiceConfigureListenerEditVie
onValidityChange={onValidityChange}
/>
}
{listenerModel && showMinimalConfig &&
<AttachedListenerMinimalConfig
listenerName={listenerName}
onSave={onSubmit}
isSaving={saving}
savingText={savingText}
/>
}
</ServiceConfigureListenerEditViewContainer>
);
};
Expand Down
Loading