Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
55 changes: 29 additions & 26 deletions ui/src/components/dynamics-form/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
import type { Dict } from '@/api/type/common'
import FormItem from '@/components/dynamics-form/FormItem.vue'
import type { FormField } from '@/components/dynamics-form/type'
import { ref, onBeforeMount, watch, type Ref, computed } from 'vue'
import { ref, onBeforeMount, watch, type Ref, nextTick } from 'vue'
import type { FormInstance } from 'element-plus'
import type Result from '@/request/Result'
import _ from 'lodash'
Expand Down Expand Up @@ -215,31 +215,34 @@ const render = (
| (() => Promise<Result<Array<FormField>>>),
data?: Dict<any>,
) => {
if (typeof render_data == 'string') {
get(render_data, {}, loading).then((ok) => {
formFieldList.value = ok.data
})
} else if (render_data instanceof Array) {
formFieldList.value = render_data
} else if (typeof render_data === 'function') {
render_data().then((ok: any) => {
formFieldList.value = ok.data
const form_data = data ? data : {}
if (form_data) {
const value = getFormDefaultValue(formFieldList.value, form_data)
formValue.value = _.cloneDeep(value)
}
})
} else {
render_data.then((ok) => {
formFieldList.value = ok.data
})
}
const form_data = data ? data : {}
if (form_data) {
const value = getFormDefaultValue(formFieldList.value, form_data)
formValue.value = _.cloneDeep(value)
}
formFieldList.value = []
nextTick(() => {
if (typeof render_data == 'string') {
get(render_data, {}, loading).then((ok) => {
formFieldList.value = ok.data
})
} else if (render_data instanceof Array) {
formFieldList.value = render_data
} else if (typeof render_data === 'function') {
render_data().then((ok: any) => {
formFieldList.value = ok.data
const form_data = data ? data : {}
if (form_data) {
const value = getFormDefaultValue(formFieldList.value, form_data)
formValue.value = _.cloneDeep(value)
}
})
} else {
render_data.then((ok) => {
formFieldList.value = ok.data
})
}
const form_data = data ? data : {}
if (form_data) {
const value = getFormDefaultValue(formFieldList.value, form_data)
formValue.value = _.cloneDeep(value)
}
})
}
const getFormDefaultValue = (fieldList: Array<any>, form_data?: any) => {
form_data = form_data ? form_data : {}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code appears to be generally correct, but there are a few improvements and optimizations that can be made:

  1. Remove Unnecessary get API Call: The line form_value.value = _.cloneDeep(value) is redundant after calling nextTick, as you're already cloning the default values.

  2. Early Return for Directly Set Data: If render_data is an array of field definitions directly, you don't need to check for it separately; set formFieldList.value directly.

  3. Simplify Conditionals with Logical Operators: Using logical operators (&&) can improve readability and performance by reducing redundant checks.

  4. Ensure Code Format Consistency: Ensure consistent indentation and spacing for better legibility.

Here's the optimized version of the code:

@@ -205,38 +205,41 @@ const render = (
     | (() => Promise<Result<Array<FormField>>>),
   data?: Dict<any>,
 ) => {
-  if (typeof render_data !== 'string' && !Array.isArray(render_data) && typeof render_data !== 'function') return

+  if (!(typeof render_data === 'string' || Array.isArray(render_data) || typeof render_data === 'function')) return;
+
   nextTick(async () => {
-    let formFieldListCopy: any[] = [];
-
-    if (typeof render_data == 'string') {
-      formFieldListCopy = await get(render_data, {}, loading);
+    try {
+      formFieldList.value = [];

       // Check specific types based on render_data before performing async operations
       switch (true) {
         case typeof render_data === 'string':
           console.error(`render method should not use string ${JSON.stringify(render_data)}`);
           break;
         case Array.isArray(render_data):
           formFieldList.value = [...render_data]; // Use spread operator to ensure shallow copy
           break;
         case typeof render_data === 'function': {
           const result = await render_data();
           formFieldList.value = [result.data];
           const formData = data ? data : {};
           if (formData) {
             const value = getFormDefaultValue(result.data, formData);
             formValue.value = _.cloneDeep(value);
           }
           break;
         }

+        default:
          console.warn('Unhandled type in render function:', typeof render_data);

In this revised code:

  • I added an early exit condition for non-supported types early on.
  • Simplified the switch statement using boolean logic to reduce redundancy.
  • Used the spread operator to directly assign arrays and ensured they are copied without creating unnecessary copies inside the nextTick.
  • Added error logging when unexpected types are encountered, which adds additional assurance about how the function handles different inputs.

Expand Down
14 changes: 4 additions & 10 deletions ui/src/workflow/nodes/data-source-local-node/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,7 @@
label-width="auto"
>
<el-form-item
:label="
$t(
'views.workflow.nodes.dataSourceLocalNode.fileFormat.label',
'支持的文件格式',
)
"
:label="$t('views.workflow.nodes.dataSourceLocalNode.fileFormat.label', '支持的文件格式')"
:rules="{
type: 'array',
required: true,
Expand All @@ -37,6 +32,8 @@
style="width: 240px"
clearable
multiple
filterable
allow-create
>
<template #label="{ label, value }">
<span>{{ label }} </span>
Expand All @@ -51,10 +48,7 @@
</el-form-item>
<el-form-item
:label="
$t(
'views.workflow.nodes.dataSourceLocalNode.maxFileNumber.label',
'每次上传最大文件数',
)
$t('views.workflow.nodes.dataSourceLocalNode.maxFileNumber.label', '每次上传最大文件数')
"
:rules="{
type: 'array',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code snippet seems to be from a UI component that sets up form elements using Element Plus (el-form) and i18n (for translations). The code is mostly syntactically correct but can be optimized for readability and maintainability.

Here are some potential issues and optimization suggestions:

// Potential issue: Multiple lines of repeated text in labels

Suggestion:

  • Use template literals where appropriate to reduce repetition:
    :label="$t`views.workflow.nodes.dataSourceLocalNode.fileFormat.label` + ',' + $t`supports file formats`
/* No obvious CSS related changes needed */

Suggestion:

  • Consider adding default-first-letter Capitalization to improve reading flow:
    <template #label="{ label, value }">
      <span v-text="`${$tc('views.workflow.nodes.dataSourceLocalNode.fileFormat.label')}${value ? `, ${$tc('supports file format singular')}s`: ''}`"></span>
    </template>
// Additional JavaScript functions if applicable

Overall:

  • Optimize the use of $text, $i18n.t, etc., for cleaner code.
  • Refactor repetitive text in labels.
  • Optionally capitalize first letters for better readability, especially when dealing with long texts or lists in templates.

Expand Down
Loading