Skip to content

fix: The same observable object cannot appear twice in the same tree #2812

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
wxg0103 merged 1 commit into from
Apr 7, 2025
Merged

fix: The same observable object cannot appear twice in the same tree #2812

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
17 changes: 12 additions & 5 deletions ui/src/workflow/nodes/application-node/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@
</template>

<script setup lang="ts">
import { set, groupBy, create } from 'lodash'
import { set, groupBy, create, cloneDeep } from 'lodash'
import { app } from '@/main'
import NodeContainer from '@/workflow/common/NodeContainer.vue'
import { ref, computed, onMounted, onActivated } from 'vue'
Expand Down Expand Up @@ -224,13 +224,20 @@ const update_field = () => {
applicationApi
.getApplicationById(id, props.nodeModel.properties.node_data.application_id)
.then((ok) => {
const old_api_input_field_list = props.nodeModel.properties.node_data.api_input_field_list
const old_user_input_field_list = props.nodeModel.properties.node_data.user_input_field_list
const old_api_input_field_list = cloneDeep(
props.nodeModel.properties.node_data.api_input_field_list
)
const old_user_input_field_list = cloneDeep(
props.nodeModel.properties.node_data.user_input_field_list
)
if (isWorkFlow(ok.data.type)) {
const nodeData = ok.data.work_flow.nodes[0].properties.node_data
const new_api_input_field_list = ok.data.work_flow.nodes[0].properties.api_input_field_list
const new_user_input_field_list =
const new_api_input_field_list = cloneDeep(
ok.data.work_flow.nodes[0].properties.api_input_field_list
)
const new_user_input_field_list = cloneDeep(
ok.data.work_flow.nodes[0].properties.user_input_field_list
)
const merge_api_input_field_list = new_api_input_field_list.map((item: any) => {
const find_field = old_api_input_field_list.find(
(old_item: any) => old_item.variable == item.variable
Copy link
Copy Markdown
Contributor Author

@shaohuzhang1 shaohuzhang1 Apr 7, 2025

Choose a reason for hiding this comment

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

The provided code looks generally correct, but there are a few areas where optimizations can be made:

  • Dealing with Duplicates: The cloneDeep function is used to ensure that the original array structures (api_input_field_list and user_input_field_list) are not modified within the update_field method. This can prevent unintended side effects.

  • Performance Considerations: If old_api_input_field_list, old_user_input_field_list, new_api_input_field_list, or new_user_input_field_list could be very large arrays, manually cloning them might become inefficient due to memory usage. In such cases, consider using deep-copying libraries specifically designed for performance, though this seems unnecessary unless explicitly specified in your context.

Here's the suggested enhancement to improve readability slightly by organizing the clones neatly:

const oldApiInputFieldList = cloneDeep(props.nodeModel.properties.node_data.api_input_field_list);
const oldUserInputFieldList = cloneDeep(props.nodeModel.properties.node_data.user_input_field_list);

if (isWorkFlow(ok.data.type)) {
  const nodeData = ok.data.work_flow.nodes[0].properties.node_data;
  const newApiInputFieldList = cloneDeep(ok.data.work_flow.nodes[0].properties.api_input_field_list);
  const newUserInputFieldList = cloneDeep(ok.data.work_flow.nodes[0].properties.user_input_field_list);

  const mergeApiInputFieldList = newApiInputFieldList.map((item: any) => {
    const findField = oldApiInputFieldList.find((oldItem: any) => oldItem.variable === item.variable);
    return findField || item; // Merge logic here
  });
}

In summary, maintaining clean and efficient code while handling potentially large data sets depends on specific needs and constraints. For simplicity and clarity, manual deep-cloning ensures correctness without significant overhead in many scenarios.

Expand Down