Skip to content

fix: Subsequent nodes in the form have lost reference data #4251

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 24, 2025
Merged

fix: Subsequent nodes in the form have lost reference data #4251

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
5 changes: 4 additions & 1 deletion ui/src/workflow/common/app-node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,14 @@ class AppNode extends HtmlResize.view {
)
}

props.model.properties.config = nodeDict[props.model.type].properties.config
props.model.properties.config = this.getConfig(props)
if (props.model.properties.height) {
props.model.height = props.model.properties.height
}
}
getConfig(props: any) {
return nodeDict[props.model.type].properties.config
}
getNodeName(nodes: Array<any>, baseName: string) {
let index = 0
let name = baseName
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Oct 24, 2025

Choose a reason for hiding this comment

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

No significant issue was found with the provided code. A couple of improvements could be made:

  1. Ensure the getConfig method is correctly exporting its results from Node.js.
  2. Add error handling in case props do not contain properties like 'type' or 'config'.
  3. Consider using strict type checking to improve readability and maintainability.

Overall, the original code seems straightforward and performs the task as intended.

Expand Down
5 changes: 4 additions & 1 deletion ui/src/workflow/nodes/form-node/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@ class FormNode extends AppNode {
constructor(props: any) {
super(props, FormNodeVue)
}
getConfig(props: any) {
return props.model.properties.config
}
}
export default {
type: 'form-node',
model: AppNodeModel,
view: FormNode
view: FormNode,
}
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Oct 24, 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 mostly correct in terms of syntax and intent. Here's a brief analysis:

  1. Getter Method: The getConfig method provides a consistent way to access the configuration from the props.model.properties.config. This is good practice if you need this functionality frequently within other methods.

  2. Export Statements: Both type, model, and view are correctly exported with their respective values.

  3. Constructor: The constructor remains straightforward, initializing the parent class (AppNode) with both props and FormNodeVue.

  4. No Immediate Issues Found: Apart from using camelCase consistently throughout (e.g., getConfig instead of GetConfig), there don't seem to be any significant problems with structure or logic.

If you have specific concerns or additional requirements not covered here, feel free to let me know!

3 changes: 3 additions & 0 deletions ui/src/workflow/nodes/parameter-extraction-node/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ class ParameterExtractionNode extends AppNode {
constructor(props: any) {
super(props, ParameterExtractionNodeVue)
}
getConfig(props: any) {
return props.model.properties.config
}
}

export default {
Expand Down
18 changes: 10 additions & 8 deletions ui/src/workflow/nodes/variable-aggregation-node/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@ import VariableAggregationNodeVue from './index.vue'
import { AppNode, AppNodeModel } from '@/workflow/common/app-node'

class VariableAggregationNode extends AppNode {
constructor(props: any) {
super(props, VariableAggregationNodeVue)
}
constructor(props: any) {
super(props, VariableAggregationNodeVue)
}
getConfig(props: any) {
return props.model.properties.config
}
}


export default {
type: 'variable-aggregation-node',
model: AppNodeModel,
view: VariableAggregationNode,
}
type: 'variable-aggregation-node',
model: AppNodeModel,
view: VariableAggregationNode,
}
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Oct 24, 2025

Choose a reason for hiding this comment

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

The provided code is mostly correct and does not contain significant irregularities or potential issues. However, there are a few optimizations you can consider:

  1. Simplify Constructors: In JavaScript/TypeScript, it's generally preferred to pass props directly to the parent class (AppNode) instead of creating a new object with them initially. This keeps things clear and concise.

    

    2. 



    

  • 

     constructor(props: any) {

    

    • 

  • 

         super(props, VariableAggregationNodeVue);

    

    • 

  • 

     }       

    

    • 



    

  • constructor(props: any) {
    • 

  • 

       super(props);

    

    • 

  • }
    • 




2. **Remove Empty Line**: The newline at the end of the last line seems unnecessary and can be removed for better maintainability.

Here’s the updated version of your code:

```javascript
import VariableAggregationNodeVue from './index.vue'
import { AppNode, AppNodeModel } from '@/workflow/common/app-node'

class VariableAggregationNode extends AppNode {
constructor(props: any) {
 super(props)
}

getConfig(props: any) {
 return props.model.properties.config;
}
}

export default {
type: 'variable-aggregation-node',
model: AppNodeModel,
view: VariableAggregationNode,
};



This should make the code cleaner and more readable while maintaining its functionality.

    

  
  




    


        

      
  
  

    
  
    
    

  

  

  



          

      
    








    


  









  


    


  		





                
              


          


    

  



    
  

    

      

        


          

            
            
          


          



          
            3 changes: 3 additions & 0 deletions
          
          

        

  ui/src/workflow/nodes/variable-splitting/index.ts

  
  
      
      
  





        
      


      

        











            


          

          

            
              

                
    

              

            
            
                            

                            


                              
   View file



                                    


                                  


                            


                            

                            

    Open in desktop



                
            
          

        

      

    



    



          

            




              
      
    
              

                
                  

                      Original file line number
                      Diff line number
                      Diff line change
                  

                
                
                      
      

    
          
            
          
          Expand Up
    
    @@ -5,6 +5,9 @@ class VariableSplittingNode extends AppNode {
  


    

    

    

  
      constructor(props: any) {






    

    

    

  
        super(props, VariableSplittingNodeVue)






    

    

    

  
      }






    

    

    

  
      getConfig(props: any) {






    

    

    

  
        return props.model.properties.config






    

    

    

  
      }






    

    

    

  
    }






    

    

    

  
    







    

    

    

  
    export default {






  

    
          
            
          
          Expand Down
    
    
  



                
              


          


    

  



  



    

  
  
    
      Loading