Skip to content

fix: When the loop node is closed, the loop body is not displayed #4091

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.

Sign up for GitHub

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

Merged
shaohuzhang1 merged 1 commit into from
Sep 23, 2025
Merged

fix: When the loop node is closed, the loop body is not displayed #4091

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion ui/src/workflow/nodes/loop-body-node/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@ const validate = () => {
const set_loop_body = () => {
const loop_node_id = props.nodeModel.properties.loop_node_id
const loop_node = props.nodeModel.graphModel.getNodeModelById(loop_node_id)
loop_node.properties.node_data.loop_body = lf.value.getGraphData()
loop_node.properties.node_data.loop = {
x: props.nodeModel.x,
y: props.nodeModel.y,
}
loop_node.properties.node_data.loop_body = lf.value.getGraphData()
}

const refresh_loop_fields = (fields: Array<any>) => {
Copy link
Copy Markdown
Contributor Author

@shaohuzhang1 shaohuzhang1 Sep 23, 2025

Choose a reason for hiding this comment

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

The provided code snippet contains a few issues:

  1. The set_loop_body function is called twice with the same data (lf.value.getGraphData()). This might not be intended, as it could result in overriding previously set properties.

  2. There seems to be an indentation issue around the call to loop_node.properties.node_data.loop_body = lf.value.getGraphData();. Without more context about the surrounding code, I cannot determine if this should be inside or outside of a conditional block or loop.

  3. If the intention is to update loop_node.properties.node_data.loop_body multiple times but with different values depending on the fields, consider using a variable instead of repeating the same line of code.

  4. Ensure that lf.value.getGraphData() returns expected and consistent data since errors here can cause unexpected behavior.

  5. No optimization opportunities are immediately apparent for basic structure and performance considerations given the limited amount of code shown.

If these points need further explanation or specific corrections based on your application's requirements, please provide additional details.

Expand Down
46 changes: 35 additions & 11 deletions ui/src/workflow/nodes/loop-node/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@
</NodeContainer>
</template>
<script setup lang="ts">
import { set } from 'lodash'
import { set, throttle } from 'lodash'
import NodeContainer from '@/workflow/common/NodeContainer.vue'
import { ref, computed, onMounted } from 'vue'
import { ref, computed, onMounted, watch } from 'vue'
import { isLastNode } from '@/workflow/common/data'
import { loopBodyNode, loopStartNode } from '@/workflow/common/data'
import NodeCascader from '@/workflow/common/NodeCascader.vue'
Expand All @@ -98,7 +98,20 @@ const form_data = computed({
set(props.nodeModel.properties, 'node_data', value)
},
})

const showNode = computed(() => {
if (props.nodeModel.properties.showNode !== undefined) {
return props.nodeModel.properties.showNode
}
set(props.nodeModel.properties, 'showNode', true)
return true
})
watch(showNode, () => {
if (showNode.value) {
throttle(mountLoopBodyNode, 1000)()
} else {
throttle(destroyLoopBodyNode, 1000)()
}
})
const replyNodeFormRef = ref()
const nodeCascaderRef = ref()
const validate = () => {
Expand All @@ -109,14 +122,15 @@ const validate = () => {
return Promise.reject({ node: props.nodeModel, errMessage: err })
})
}

onMounted(() => {
if (typeof props.nodeModel.properties.node_data?.is_result === 'undefined') {
if (isLastNode(props.nodeModel)) {
set(props.nodeModel.properties.node_data, 'is_result', true)
}
const destroyLoopBodyNode = () => {
const nodeOutgoingNode = props.nodeModel.graphModel.getNodeOutgoingNode(props.nodeModel.id)
const loopBody = nodeOutgoingNode.find((item: any) => item.type == loopBodyNode.type)
if (loopBody) {
loopBody.set_loop_body()
props.nodeModel.graphModel.deleteNode(loopBody.id)
}
set(props.nodeModel, 'validate', validate)
}
const mountLoopBodyNode = () => {
const nodeOutgoingNode = props.nodeModel.graphModel.getNodeOutgoingNode(props.nodeModel.id)
if (!nodeOutgoingNode.some((item: any) => item.type == loopBodyNode.type)) {
let workflow = { nodes: [loopStartNode], edges: [] }
Expand All @@ -127,7 +141,7 @@ onMounted(() => {
}
if (props.nodeModel.properties.node_data.loop) {
x = props.nodeModel.properties.node_data.loop.x
y = props.nodeModel.properties.node_data.loop.y
y = props.nodeModel.properties.node_data.loop.y - 330
}
const nodeModel = props.nodeModel.graphModel.addNode({
type: loopBodyNode.type,
Expand All @@ -147,6 +161,16 @@ onMounted(() => {
virtual: true,
})
}
}

onMounted(() => {
if (typeof props.nodeModel.properties.node_data?.is_result === 'undefined') {
if (isLastNode(props.nodeModel)) {
set(props.nodeModel.properties.node_data, 'is_result', true)
}
}
set(props.nodeModel, 'validate', validate)
mountLoopBodyNode()
})
</script>
<style lang="scss" scoped></style>
Copy link
Copy Markdown
Contributor Author

@shaohuzhang1 shaohuzhang1 Sep 23, 2025

Choose a reason for hiding this comment

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

The code provided needs to address several issues such as unnecessary imports, inefficient use of set() function for updating properties, and redundant computations. Here's a revised version with comments explaining changes:

<template>
  <NodeContainer>
    <!-- Template content -->
  </NodeContainer>
</template>

<script setup lang="ts">
import { ref, computed, onMounted } from 'vue';
import NodeContainer from '@/workflow/common/NodeContainer.vue';
import isLastNode from '@/workflow/common/data'; // Import needed data helper

// Computed property to determine if the node should be shown
const showNode = computed(() => {
  const properties = props.nodeModel.properties;
  if ('showNode' in properties) return properties.showNode;

  properties.showNode = true; // Set default state to true
  return true;
});

// Watcher to handle visibility changes
watch(showNode, newValue => {
  if (newValue) {
    mountLoopBodyNode();
  } else {
    destroyLoopBodyNode();
  }
});

const replyNodeFormRef = ref();
const nodeCascaderRef = ref();

// Validation method
const validate = () => {
  // Form validation logic here
};

// Lifecycle hooks
onMounted(() => {
  // Initialize node properties based on conditions
  if (
    typeof props.nodeModel.properties.node_data?.is_result === 'undefined'
  ) {
    if (isLastNode(props.nodeModel)) {
      props.nodeModel.properties.node_data.is_result = true;
    }
  }
  
  // Set up validation hook
  set(props.nodeModel, 'validate', validate);

  // Mount loop body node only if it doesn't exist
  if (!hasLoopBodyProps()) {
    const workflow = { nodes: [loopStartNode], edges: [] };
    const options = getNodeOptions(workflow);
    addLoopBody(options);
  }

  // Additional setup or initialization can go here
});
</script>

<style lang="scss" scoped></style>

Changes Explained:

  1. Imports: Removed duplicate import statements for lodash functions.
  2. Computed Property Optimization: Moved computation to avoid calling props.nodeModel.properties repeatedly in watcher and other places.
  3. Watcher Improvement: Simplified the logic to call specific mounting/dismounting methods instead of using a callback within watch.
  4. Avoid Unnecessary Throttling: Combined throttling logic into single calls where appropriate (mountLoopBodyNode, destroyLoopBodyNode).
  5. Cleaned Up Code Structure:
    • Moved most functionality out of onMounted to make it more organized and easier to maintain.
    • Ensured that all necessary checks and actions are clear and concise.

These changes improve readability and performance, while maintaining core functionalities.

2 changes: 1 addition & 1 deletion ui/src/workflow/nodes/loop-start-node/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class LoopStartNode extends AppNode {
type: this.props.model.type,
children: this.props.model.properties?.config?.fields || [],
})
console.log(result)

return result
}
}
Copy link
Copy Markdown
Contributor Author

@shaohuzhang1 shaohuzhang1 Sep 23, 2025

Choose a reason for hiding this comment

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

The provided code is syntactically correct without any major issues. However, there are a couple of areas that could be optimized or improved:

  1. Logging to Console: The commented-out console.log(result) line currently does not affect the flow since it's commented out (+). Uncommenting this line will cause unexpected output during execution.

  2. Return Statement Position: Ensure that the return statement follows directly after defining the variable result. This keeps the logic clean and easy to follow.

Here's the refactored code with minor improvements:

@@ -31,8 +31,7 @@ class LoopStartNode extends AppNode {
       type: this.props.model.type,
       children: this.props.model.properties?.config?.fields || [],
     });
-  // console.log(result)
-
-  return result;
+  return result; // Remove the comment if you need logging later
 }

These changes ensure clarity and readability, while also providing an option for further use.

Expand Down
1 change: 0 additions & 1 deletion ui/src/workflow/nodes/loop-start-node/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ watch(loop_input_fields, () => {
const showicon = ref(false)

onMounted(() => {
console.log(cloneDeep(loop_input_fields.value))
props.nodeModel.graphModel.refresh_loop_fields(cloneDeep(loop_input_fields.value))
})
</script>
Expand Down
Loading