Support nested step function definitions #317
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| // In workflow mode, transform the function body to call the runtime proxy | ||
| if self.mode == TransformMode::Workflow { | ||
| let step_id = self.create_id(Some(&fn_name), arrow_expr.span, false); | ||
| let mut proxy_call = self.create_step_proxy(&step_id); | ||
|
|
||
| // Add function arguments to the proxy call | ||
| if !arrow_expr.params.is_empty() { | ||
| for param in &arrow_expr.params { | ||
| if let Pat::Ident(ident) = param { | ||
| proxy_call = Expr::Call(CallExpr { | ||
| span: DUMMY_SP, | ||
| ctxt: SyntaxContext::empty(), | ||
| callee: Callee::Expr(Box::new(proxy_call)), | ||
| args: vec![ExprOrSpread { | ||
| spread: None, | ||
| expr: Box::new(Expr::Ident(ident.id.clone())), | ||
| }], | ||
| type_args: None, | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| arrow_expr.body = Box::new(BlockStmtOrExpr::Expr(Box::new(proxy_call))); | ||
| } | ||
|
|
There was a problem hiding this comment.
| // In workflow mode, transform the function body to call the runtime proxy | |
| if self.mode == TransformMode::Workflow { | |
| let step_id = self.create_id(Some(&fn_name), arrow_expr.span, false); | |
| let mut proxy_call = self.create_step_proxy(&step_id); | |
| // Add function arguments to the proxy call | |
| if !arrow_expr.params.is_empty() { | |
| for param in &arrow_expr.params { | |
| if let Pat::Ident(ident) = param { | |
| proxy_call = Expr::Call(CallExpr { | |
| span: DUMMY_SP, | |
| ctxt: SyntaxContext::empty(), | |
| callee: Callee::Expr(Box::new(proxy_call)), | |
| args: vec![ExprOrSpread { | |
| spread: None, | |
| expr: Box::new(Expr::Ident(ident.id.clone())), | |
| }], | |
| type_args: None, | |
| }); | |
| } | |
| } | |
| } | |
| arrow_expr.body = Box::new(BlockStmtOrExpr::Expr(Box::new(proxy_call))); | |
| } |
Unreachable dead code attempts to handle Workflow mode transformation inside a Step-mode-only conditional block, making it impossible for that code to execute.
View Details
Analysis
Unreachable dead code in nested steps hoisting
What fails: Lines 2137-2160 in packages/swc-plugin-workflow/transform/src/lib.rs contain unreachable code that attempts to handle Workflow mode transformation inside a Step-mode-only conditional block, making the code impossible to execute.
How to reproduce: The bug is a control flow issue visible in code review:
- Line 2119 contains the conditional:
if self.mode == TransformMode::Step && !self.nested_steps_to_hoist.is_empty() - Inside this block, lines 2137-2160 check:
if self.mode == TransformMode::Workflow - Since
self.modeis immutable and already verified to equalTransformMode::Stepat line 2119, the inner condition at line 2137 will never be true
Result: The Workflow mode transformation code (which attempts to transform arrow function bodies into proxy calls) never executes, leaving the dead code to be maintained without purpose.
Expected: Dead code should be removed. In Step mode, hoisted nested steps should only be:
- Collected and hoisted to module level (lines 2135-2147)
- Have their 'use step' directive removed (line 2134)
- Have registration calls created for them (lines 2156-2158)
The Workflow mode transformations that were mistakenly placed inside this Step-mode-only block were never intended to execute here - they are correctly handled in-place for nested steps during Workflow mode processing (around lines 2814-2849).
Root cause: Copy-paste error - Workflow mode handling code was accidentally included inside the Step mode hoisting block
…ted-step-function-definition
| workflow_exports_to_expand: Vec<(String, Expr, swc_core::common::Span)>, | ||
| // Track workflow functions that need workflowId property in client mode | ||
| workflow_functions_needing_id: Vec<(String, swc_core::common::Span)>, | ||
| // Track nested step functions that need to be hoisted to module scope |
There was a problem hiding this comment.
The closure variable detection feature for nested step functions is non-functional. The scope_variables field is initialized but never populated, so all closure variable checks will always pass even when closure variables are actually used.
View Details
📝 Patch Details
diff --git a/packages/swc-plugin-workflow/transform/src/lib.rs b/packages/swc-plugin-workflow/transform/src/lib.rs
index e1710cc..d81110e 100644
--- a/packages/swc-plugin-workflow/transform/src/lib.rs
+++ b/packages/swc-plugin-workflow/transform/src/lib.rs
@@ -1198,6 +1198,66 @@ impl StepTransform {
(self.has_file_workflow_directive && is_exported)
|| self.has_use_workflow_directive_arrow(&arrow_fn.body)
}
+ // Helper function to collect all identifier names from a pattern
+ fn collect_identifiers_from_pat(&self, pat: &Pat) -> HashSet<String> {
+ let mut identifiers = HashSet::new();
+ match pat {
+ Pat::Ident(binding) => {
+ identifiers.insert(binding.id.sym.to_string());
+ }
+ Pat::Object(obj_pat) => {
+ for prop in &obj_pat.props {
+ match prop {
+ ObjectPatProp::KeyValue(kv) => {
+ identifiers.extend(self.collect_identifiers_from_pat(&kv.value));
+ }
+ ObjectPatProp::Assign(assign) => {
+ identifiers.insert(assign.key.sym.to_string());
+ }
+ ObjectPatProp::Rest(rest) => {
+ identifiers.extend(self.collect_identifiers_from_pat(&rest.arg));
+ }
+ }
+ }
+ }
+ Pat::Array(arr_pat) => {
+ for elem in &arr_pat.elems {
+ if let Some(pat) = elem {
+ identifiers.extend(self.collect_identifiers_from_pat(pat));
+ }
+ }
+ }
+ Pat::Rest(rest_pat) => {
+ identifiers.extend(self.collect_identifiers_from_pat(&rest_pat.arg));
+ }
+ Pat::Assign(assign_pat) => {
+ identifiers.extend(self.collect_identifiers_from_pat(&assign_pat.left));
+ }
+ _ => {
+ // Other pattern types don't introduce identifiers in the scope
+ }
+ }
+ identifiers
+ }
+
+ // Helper function to collect all parameter names from a function
+ fn collect_function_params(&self, function: &Function) -> HashSet<String> {
+ let mut params = HashSet::new();
+ for param in &function.params {
+ params.extend(self.collect_identifiers_from_pat(¶m.pat));
+ }
+ params
+ }
+
+ // Helper function to collect all parameter names from an arrow expression
+ fn collect_arrow_params(&self, arrow: &ArrowExpr) -> HashSet<String> {
+ let mut params = HashSet::new();
+ for pat in &arrow.params {
+ params.extend(self.collect_identifiers_from_pat(pat));
+ }
+ params
+ }
+
// Check if a function uses any closure variables from the given scope
fn function_uses_closure_vars(&self, function: &Function, scope_vars: &HashSet<String>) -> Vec<String> {
@@ -1678,9 +1738,16 @@ impl VisitMut for StepTransform {
}
self.in_module_level = false;
+ // Push current function's parameters onto scope_variables
+ let func_params = self.collect_function_params(function);
+ self.scope_variables.push(func_params);
+
// Visit children
function.visit_mut_children_with(self);
+ // Pop scope_variables
+ self.scope_variables.pop();
+
// Restore context
self.in_step_function = old_in_step;
self.in_workflow_function = old_in_workflow;
@@ -1704,9 +1771,16 @@ impl VisitMut for StepTransform {
}
self.in_module_level = false;
+ // Push current arrow expression's parameters onto scope_variables
+ let arrow_params = self.collect_arrow_params(arrow);
+ self.scope_variables.push(arrow_params);
+
// Visit children
arrow.visit_mut_children_with(self);
+ // Pop scope_variables
+ self.scope_variables.pop();
+
// Restore context
self.in_step_function = old_in_step;
self.in_workflow_function = old_in_workflow;
diff --git a/packages/swc-plugin-workflow/transform/tests/errors/nested-step-closure-vars/input.js b/packages/swc-plugin-workflow/transform/tests/errors/nested-step-closure-vars/input.js
new file mode 100644
index 0000000..2aafc37
--- /dev/null
+++ b/packages/swc-plugin-workflow/transform/tests/errors/nested-step-closure-vars/input.js
@@ -0,0 +1,10 @@
+export async function workflow(a) {
+ 'use workflow';
+
+ const multiply = async (x) => {
+ 'use step';
+ return x * a; // Uses closure variable 'a' - should error
+ };
+
+ return await multiply(5);
+}
diff --git a/packages/swc-plugin-workflow/transform/tests/errors/nested-step-closure-vars/output-client.js b/packages/swc-plugin-workflow/transform/tests/errors/nested-step-closure-vars/output-client.js
new file mode 100644
index 0000000..fc8d82a
--- /dev/null
+++ b/packages/swc-plugin-workflow/transform/tests/errors/nested-step-closure-vars/output-client.js
@@ -0,0 +1,11 @@
+// Error: Nested step function uses closure variables
+export async function workflow(a) {
+ 'use workflow';
+
+ const multiply = async (x) => {
+ 'use step';
+ return x * a;
+ };
+
+ return await multiply(5);
+}
diff --git a/packages/swc-plugin-workflow/transform/tests/errors/nested-step-closure-vars/output-step.js b/packages/swc-plugin-workflow/transform/tests/errors/nested-step-closure-vars/output-step.js
new file mode 100644
index 0000000..fc8d82a
--- /dev/null
+++ b/packages/swc-plugin-workflow/transform/tests/errors/nested-step-closure-vars/output-step.js
@@ -0,0 +1,11 @@
+// Error: Nested step function uses closure variables
+export async function workflow(a) {
+ 'use workflow';
+
+ const multiply = async (x) => {
+ 'use step';
+ return x * a;
+ };
+
+ return await multiply(5);
+}
diff --git a/packages/swc-plugin-workflow/transform/tests/errors/nested-step-closure-vars/output-workflow.js b/packages/swc-plugin-workflow/transform/tests/errors/nested-step-closure-vars/output-workflow.js
new file mode 100644
index 0000000..fc8d82a
--- /dev/null
+++ b/packages/swc-plugin-workflow/transform/tests/errors/nested-step-closure-vars/output-workflow.js
@@ -0,0 +1,11 @@
+// Error: Nested step function uses closure variables
+export async function workflow(a) {
+ 'use workflow';
+
+ const multiply = async (x) => {
+ 'use step';
+ return x * a;
+ };
+
+ return await multiply(5);
+}
Analysis
Closure Variable Detection Not Working for Nested Step Functions
What fails: Nested step functions that use closure variables from enclosing scopes are not detected, allowing them to be transformed when they should be rejected with a NestedStepFunctionWithClosureVars error.
How to reproduce:
export async function workflow(a) {
'use workflow';
const multiply = async (x) => {
'use step';
return x * a; // Uses closure variable 'a' from workflow scope
};
return await multiply(5);
}This code should fail with an error: "use step" function 'multiply' cannot use closure variables: a. However, before the fix, it would transform without error and fail at runtime when trying to access the undefined closure variable a.
Root cause: The scope_variables: Vec<HashSet<String>> field was initialized with a single empty HashSet but never populated with function parameters as the AST visitor traversed nested functions. This meant:
- When checking nested step functions for closure variable usage, the code retrieved an empty set of scope variables
- The
ClosureVarCollectorvisitor compared identifiers against an empty set, so no closure variables were ever detected - Nested step functions using closure variables incorrectly passed validation
Fix implemented: Added three helper functions and updated two visitor methods to properly track function scope variables:
collect_identifiers_from_pat()- Recursively extracts all identifier names from parameter patterns (handles simple params, destructuring, rest patterns, etc.)collect_function_params()- Collects all parameter names from a Functioncollect_arrow_params()- Collects all parameter names from an ArrowExpr
Updated visit_mut_function() and visit_mut_arrow_expr() to:
- Push the collected parameters onto
scope_variableswhen entering function scopes - Pop from
scope_variableswhen exiting function scopes
This ensures closure variable detection has access to the correct scope variables at each nesting level.
Testing: Created error test case at packages/swc-plugin-workflow/transform/tests/errors/nested-step-closure-vars/ to verify the nested step function with closure variables is properly rejected.
No description provided.