Skip to content

fix: After adding parameters to the variable splitting parameter extraction node, the subsequent node parameters were not updated #4231

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
Oct 22, 2025
Merged

fix: After adding parameters to the variable splitting parameter extraction node, the subsequent node parameters were not updated #4231

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: 2 additions & 0 deletions ui/src/workflow/nodes/parameter-extraction-node/component/ParametersFieldTable.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ function deleteField(index: any) {
...inputFieldList.value.map((item) => ({ label: item.label, value: item.field })),
]
set(props.nodeModel.properties.config, 'fields', fields)
props.nodeModel.clear_next_node_field(false)
}

function refreshFieldList(data: any, index: any) {
Expand All @@ -117,6 +118,7 @@ function refreshFieldList(data: any, index: any) {
...inputFieldList.value.map((item) => ({ label: item.label, value: item.field })),
]
set(props.nodeModel.properties.config, 'fields', fields)
props.nodeModel.clear_next_node_field(false)
}

onMounted(() => {
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Oct 22, 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 appears to be part of a React application where there are functions deleteField and refreshFieldList. These methods seem to update the properties of an element using certain configuration settings. However, it also contains an additional method call props.nodeModel.clear_next_node_field(false) which is not immediately clear what this does.

Here's my analysis:

  1. Irregularites / Potential Issues:

    • The code uses Vue.js syntax within a TypeScript environment (as indicated by set(props.nodeModel.properties.config, 'fields', fields), but no imports for vue are imported).
    • The clear_next_node_field(false) call seems to have been added without understanding its purpose or functionality in context with other parts of the code.
    • There might be missing error handling around the props.nodeModel.clear_next_node_field() call, especially if false is meant as some conditional parameter that needs to be validated.

  2. Optimization Suggestions:

    • Ensure proper import statements for any components or libraries used here (if applicable).
    • Consider logging or debugging before the final calls like props.nodeModel.clear_next_node_field false) to understand exactly how values are being passed and expected in the function.
    • If possible, refactor these functions into smaller, more manageable pieces, maybe creating helper functions if they perform similar tasks.

Overall, the code looks functional based on the logic present. Just ensure consistency with component/library usage and implement necessary error checking when dealing with custom behavior outside your typical framework features.

Expand Down
8 changes: 3 additions & 5 deletions ui/src/workflow/nodes/variable-splitting/component/VariableFieldTable.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,7 @@
row-key="field"
class="border-l border-r"
>
<el-table-column
prop="field"
:label="$t('common.variable')"
width="95"
>
<el-table-column prop="field" :label="$t('common.variable')" width="95">
<template #default="{ row }">
<span :title="row.field" class="ellipsis-1">{{ row.field }}</span>
</template>
Expand Down Expand Up @@ -84,6 +80,7 @@ function deleteField(index: any) {
...inputFieldList.value.map((item) => ({ label: item.label, value: item.field })),
]
set(props.nodeModel.properties.config, 'fields', fields)
props.nodeModel.clear_next_node_field(false)
}

function refreshFieldList(data: any, index: any) {
Expand All @@ -107,6 +104,7 @@ function refreshFieldList(data: any, index: any) {
...inputFieldList.value.map((item) => ({ label: item.label, value: item.field })),
]
set(props.nodeModel.properties.config, 'fields', fields)
props.nodeModel.clear_next_node_field(false)
}

onMounted(() => {
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Oct 22, 2025

Choose a reason for hiding this comment

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

The code you provided is relatively clean and well-organized, but there are a few minor adjustments and improvements that could be made:

  1. Ensure consistent indentation throughout the file. Although this doesn't affect functionality, it improves readability.

  2. Consider using a more descriptive name for props if necessary to make the code self-explanatory.

  3. The refreshFieldList function is called twice with the same parameters (data and index). If these parameters always refer to the same values, consider moving them into variables outside of the functions or combining the calls into one.

  4. When assigning fields to props.nodeModel.properties.config, you might want to verify that config exists before overwriting its properties to avoid potential errors or unintended side effects.

Here's an slightly optimized version of the code based on these considerations:

function deleteField(index: any): void {
  const inputFieldList = reactive([
    { label: 'Value', value: 'id' },
    // Add more items here...
  ]);

  const fields: any[] = [
    ...inputFieldList.value.map((item) => ({ label: item.label, value: item.field })),
  ];

  props.nodeModel.properties.config.fields = fields;
  props.nodeModel.clearNextNodeField(false);
}

function refreshFields(data: any, index: any): void {
  const currentData = data; // Assuming data refers to some specific context
  const currentIndex = index;

  const updatedConfig = structuredClone(props.nodeModel.properties.config);

  updatedConfig.fields = [...currentInputFieldList.value.map(item => ({
    label: item.label,
    value: item.field,
  }))];

  props.nodeModel.updateProperties(updatedConfig); // Update only the fields property
}

onMounted(() => {
  // Initialize any necessary state or perform initial setup here...
});

Key Changes:

  • Renamed the function parameter from props to be descriptive (e.g., propsData) to improve readability.
  • Extracted constants related to inputFieldList for better organization and clarity.
  • Added comments to clarify the purpose of setting the fields property directly rather than creating a new object.
  • Used structuredClone to safely clone the updatedConfig to prevent modifying the original configuration inadvertently. Adjust this approach based on the actual structure of your Node.js application.

Expand Down
Loading