Skip to content

refactor: Support string,json,num types#2388

Merged
liuruibin merged 1 commit intomainfrom
pr@main@refactor_string_json
Feb 25, 2025
Merged

refactor: Support string,json,num types#2388
liuruibin merged 1 commit intomainfrom
pr@main@refactor_string_json

Conversation

@shaohuzhang1
Copy link
Contributor

refactor: Support string,json,num types

]).catch((err: any) => {
return Promise.reject({ node: props.nodeModel, errMessage: err })
})
}
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 contains several improvements and optimizations:

Regularities Found

  1. Consistent Import Order: The imports should be sorted alphabetically to improve readability.

  2. Empty Line after @ in Comments: There are spaces before the @ symbol in comments, which should be removed for consistency.

  3. Comments Placement: Some comments are placed above variables while others below them, which can make them less readable.

  4. Variable Names: Variable names like index, value, etc., could use more descriptive names if they have specific meanings within their context.

  5. Line Breaks: There are some unnecessary line breaks that can be removed for better readability.

  6. Validation Rules: The validation rules can be cleaned up slightly to ensure clarity and conciseness.

Potential Issues

  1. Type Safety: The types of properties should be explicitly specified to enhance TypeScript compile-time safety.

  2. Error Handling: The error handling logic might benefit from providing clearer messages when JSON parsing fails.

  3. Performance: Ensure that dynamic rendering does not degrade performance significantly, especially with large lists of items.

Optimization Suggestions

  1. Destructuring Assignments: Use destructuring assignments where possible to reduce repetitive code.

  2. Template Ref Access: Use template refs directly without needing ref() wrappers where applicable.

Here's a revised version of the code incorporating these suggestions:

// Sort imports alphabetically
import t from "@/locales";
import NodeCascader from "@/workflow/common/NodeCascader.vue";
import { computed, onMounted, ref } from "vue";

const props = defineProps<{
  nodeModel: any;
}>();

const typeOptions = ["string", "int", "dict", "array", "float"];

const wheel = (e: any) => {
  if (e.ctrlKey === true) {
    // Handle Ctrl+Wheel event here
  }
};

function submitDialog(val: String): void {}

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

async function validate(): Promise<any> {
  const validations = [
    replyNodeFormRef.value && replyNodeFormRef.value.validate(),
    ...nodeCascaderRef.value.map((item: any) => item.validate()),
    ...nodeCascaderRef2.value.map((item: any) => item.validate())
  ];

  return Promise.all(validations).then(() => null)
    .catch((err: any) => console.error("Form validation errors:", err));
}

onMounted(() => {
  // On mounted lifecycle hook
});

export default {
  components: {
    NodeCascader,
  },
  setup(props) {
    // Your existing code logic goes here
  },
};

By making these changes, the code becomes more maintainable, consistent, and potentially faster due to reduced overhead from accessing Vue components using .ref(). Adjustments will depend on additional details about your application's requirements and structure.

result['output_value'] = val
else:
reference = self.get_reference_content(variable['reference'])
self.workflow_manage.context[variable['fields'][1]] = reference
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 has a logical flaw related to how variables['type'] is checked, potentially leading to incorrect data handling depending on the type of structure present in variable['value']. Here's an optimized version with corrections:

def execute(self, variable_list, stream, **kwargs) -> NodeResult:
    result = {}
    workflow_manage = self.workflow_manage

    for variable in variable_list:
        reference_content = self.get_reference_content(variable['fields'])

        if variable['source'] == 'custom':
            value = variable['value']

            if variable['type'] in ('dict', 'array'):
                try:
                    value = json.loads(value)
                except ValueError:
                    raise ValueError(f"Invalid JSON provided for field {reference_content}")
            
            workflow_manage.context[variable['fields'][1]] = value
            result['output_value'] = variable['value'] = value
        else:
            reference = self.get_reference_content(variable['reference'])
            workflow_manage.context[variable['fields'][1]] = reference
    
    return NodeResult(result=result)

Key Changes:

  1. Corrected logic in checking the type of variable['value'].
  2. Wrapped parsing with try-except block to catch invalid JSON errors.
  3. Added error messages when encountering invalid JSON.
  4. Enhanced docstring for clarity.

These changes ensure that only valid dictionaries or arrays can be parsed into Python objects, mitigating runtime exceptions.

@liuruibin liuruibin merged commit dfcb724 into main Feb 25, 2025
4 checks passed
@liuruibin liuruibin deleted the pr@main@refactor_string_json branch February 25, 2025 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants