perf: Workflow Canvas Rendering

[shaohuzhang1]

perf: Workflow Canvas Rendering

[f2c-ci-robot]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[f2c-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[shaohuzhang1]

There are several areas where improvements can be made to the provided code:

1. Consistent Method Bindings:

   • The bindings for methods like clear_next_node_field, get_up_node_field_dict, and get_up_node_field_list should only appear once in the constructor.

2. Redundant Imports:

   • Remove the duplicate import of connect from ./teleport as it's already imported at the beginning.

3. Nullish Coalescing Operator:

   • Replace || [] with ?? [] in get_node_field_list to handle cases where config?.fields might be null or undefined more gracefully.

4. Cache Management:

   • Consider adding caching strategies or memoization to improve performance, especially when the method is called frequently within loops or recursive calculations.

5. Optimize Function Calls:

   • Avoid unnecessary function calls that could lead to performance bottlenecks, such as re-retrieving properties from props.model.

6. Variable Naming Consistency:

   • Ensure variable names follow consistent naming conventions for readability and maintainability.

Here's an optimized version based on these suggestions:

@@ -10,9 +10,14 @@
 import i18n from '@/locales';
 import { WorkflowType } from '@/enums/workflow'
 import { nodeDict } from '@/workflow/common/data';
+import { t } from '@/locales';

class AppNode extends HtmlResize.view {
  isMounted: boolean;
  r?;
  component: any;
  app: any;
  root?: HTMLDivElement;
  VueNode: any;

  constructor(props: any, VueNode: any) {
    super(props);
    this.component = VueNode;
    this.isMounted = false;
    
    // Single binding statement for all event handlers
    props.model.events = {
      clearNextNodeField: this.clear_next_node_field.bind(this),
      getUpNodeFieldDict: this.get_up_node_field_dict.bind(this),
      getNodeFieldList: this.getNodeFieldList.bind(this)
    };
    
    if (props.model.properties.noRender) {
      delete props.model.properties.noRender;
    }

    function getNodesName(num: number): string {
      const name = `${props.model.properties.stepName}-${num}`;
      if (!props.graphModel.nodes.some((node: any) => node.properties.stepName === name)) {
        props.model.properties.stepName = name;
      } else {
        return getNodesName(++num);
      }
    }

    props.model.properties.config = nodeDict[props.model.type].properties.config;
    if (props.model.properties.height) {
      props.model.height = props.model.properties.height;
    }
  }

  getNodeFieldList(): Array<{ value: string; label: string; type: string; children?: Array<any> }> {
    const result: Array<{ value: string; label: string; type: string; children?: Array<any> }> = [];
    
    if (this.props.model.type === 'start-node') {
      result.push({
        value: 'global',
        label: t('views.applicationWorkflow.variable.global'),
        type: 'global',
        children: this.props.model.properties?.config?.globalFields ?? []
      });
    }

    result.push({
      value: this.props.model.id,
      label: this.props.model.properties.stepName,
      type: this.props.model.type,
      children: this.props.model.properties?.config?.fields ?? []
    });

    return result;
  }

  getUpNodeFieldDict(containSelf: boolean, useCache: boolean): Record<string, Array<{ value: string; label: string; type: string; children?: Array<any> }>> | undefined {
    let upNodeList = this.props.graphModel.getNodeIncomingNode(this.props.model.id);

    if (!upNodeList.length || upNodeList.every(n => n.id !== 'start-node')) {
      return undefined;
    }

    let upNodeFieldDict: Record<string, Array<{ value: string; label: string; type: string; children?: Array<any> }>> = {};
    for (const node of upNodeList.filter(node => node.id !== 'start-node')) {
      const fields = node.getUpNodeFieldDict(true, useCache)?.[node.id];
      upNodeFieldDict[node.id] = fields;
    }

    if (containSelf) {
      upNodeFieldDict[this.props.model.id] = this.getNodeFieldList();
    }

    return upNodeFieldDict;
  }

  getUpNodeFieldList(containSelf: boolean, useCache: boolean): Array<{ value: string; label: string; type: string; children?: Array<any> }> {
    const baseResult = Object.values(this.getUpNodeFieldDict(containSelf, useCache));
    
    if (baseResult.length > 0) {
      const startNodeFieldList = this.props.graphModel.getNodeModelById('start-node').get_nodeFieldList();
      return [...startNodeFieldList, ...baseResult.flat()];
    }

    return [];
  }

  clearNextNodeField(containSelf: boolean) {
    const nextNodeList = this.props.graphModel.getNodeOutgoingNode(this.props.model.id).filter(node => node.type !== WorkflowType.StartNode);
    nextNodeList.forEach(node => {
      node.clearNextNodeField(true);
    });

    if (containSelf) {
      this.upNodeFieldDict = undefined;
    }
  }
}

These changes ensure better organization and consistency across the codebase while improving readability and potentially enhancing performance through less redundant operations.

[shaohuzhang1]

The code appears to be handling node field changes and validation within an application workflow. It includes fetching incoming nodes, validating the current selection of fields, and handling events related to refreshing node fields.

Here are some optimizations and improvements suggested:

1. Avoiding Deep Copies: The _getIncomingNode function modifies the value array in place rather than cloning it before modifications. This approach reduces overhead but may affect performance if used improperly. Ensure that modifying arrays does not have side effects elsewhere in your application.

2. TypeScript Annotations: Adding TypeScript annotations for type safety can help catch errors early during development. For example:

   declare const t: (...args: any[]) => string;
   interface NodeModel {
       graphModel: { getNodeIncomingNode: (id: string) => any[] };
       id?: string;
       // Add more properties as needed
   }

3. Refactoring Event Listeners: Consider refactoring event listeners outside of the main logic to improve maintainability and reduce complexity. You could create separate functions or classes for managing these event subscriptions.

4. Comments and Documentation: Add comments explaining complex logic or assumptions for clarity. Also consider adding documentation comments or a README.md file for better understanding of the code's purpose and functionality.

5. Testing: Implement unit tests for specific parts of the codebase, especially those involving state management and data fetching.

6. Error Handling: Enhance error handling by adding more detailed messages returned from promises or logging exceptions for easier debugging.

By addressing these points, you can make the code cleaner, more efficient, and potentially easier to maintain and extend in the future.

[shaohuzhang1]

The provided code appears to be setting up event listeners for changes in a graph layout library (lf) and adjusting the view based on certain events. Here's a brief review and suggested improvements:

Review Points:

1. EventListener: The line lf.value.graphModel.eventCenter.on('anchor:drop', (data: any) => { correctly adds an event listener for an 'anchor drop' event. It seems intended to handle situations where nodes are dropped onto anchors.

2. Node Clearing Logic: The .clear_next_node_field(false) method call within the 'anchor:drop' handler is somewhat unclear due to missing context about next_node_field. However, it appears to clear some fields associated with nodes that might have moved as part of anchoring behavior.

3. Timer: The statement setTimeout(() => { lf.value?.fitView(); }, 0); ensures that fitView is called asynchronously after rendering has finished. This is generally good practice to prevent blocking UI updates during long-running operations.

Optimization Suggestions

4. Consistent Error Handling: Ensure there's consistent error handling when working with asynchronous actions such as clearing node fields or fitting views. For instance, you could wrap these calls in try-catch blocks to catch potential errors gracefully.

5. Event Listener Cleanup: If this logic needs to be removed or modified later, consider implementing a way to cleanly remove the event listener using off(), especially if multiple components require similar functionality. This prevents memory leaks over time.

6. Type Safety: Verify that the types passed to on() and other methods correspond with those expected by the library. Type annotations can help catch type-related issues at compile-time.

7. Performance Considerations: While not directly addressed in this snippet, ensure that clearing nodes and fitting views do not involve expensive computations or data manipulations.

Overall, the code looks functional for managing interactions and updating the graphical representation based on user input. Minor adjustments like adding clear documentation around unclear parts of the logic and ensuring robust error handling would enhance its maintenance and future reliability.