diff --git a/.changeset/step-bind-preserves-metadata.md b/.changeset/step-bind-preserves-metadata.md new file mode 100644 index 0000000000..ead2f02f04 --- /dev/null +++ b/.changeset/step-bind-preserves-metadata.md @@ -0,0 +1,5 @@ +--- +"@workflow/core": patch +--- + +Preserve the `this` binding of bound step proxies across workflow serialization, so passing `useStep(...).bind(thisArg)` as a step argument no longer loses the receiver. diff --git a/.changeset/swc-arguments-not-closure.md b/.changeset/swc-arguments-not-closure.md new file mode 100644 index 0000000000..02aed4b7a0 --- /dev/null +++ b/.changeset/swc-arguments-not-closure.md @@ -0,0 +1,5 @@ +--- +"@workflow/swc-plugin": patch +--- + +Fix `arguments` being incorrectly captured as a closure variable in nested `function`-form step bodies, which previously produced invalid output. diff --git a/.changeset/swc-lexical-this-capture.md b/.changeset/swc-lexical-this-capture.md new file mode 100644 index 0000000000..9661a02dcd --- /dev/null +++ b/.changeset/swc-lexical-this-capture.md @@ -0,0 +1,5 @@ +--- +"@workflow/swc-plugin": patch +--- + +Support `this` references inside nested arrow `"use step"` functions. Requires the enclosing class to have custom serialization. diff --git a/packages/core/e2e/e2e.test.ts b/packages/core/e2e/e2e.test.ts index da7a9607d2..7bc0af2d94 100644 --- a/packages/core/e2e/e2e.test.ts +++ b/packages/core/e2e/e2e.test.ts @@ -1797,6 +1797,11 @@ describe('e2e', () => { // 3. counter.multiply(3) -> 5 * 3 = 15 // 4. counter.describe('test counter') -> { label: 'test counter', value: 5 } // 5. Create Counter(100), call counter2.add(50) -> 100 + 50 = 150 + // 6. counter.makeAdder(7).add(2) -> 5 + 2 + 7 = 14 (lexical `this`, + // direct invocation — `bind(this)` carries `thisVal` to the queue) + // 7. invokeAdderFromStep(counter.makeAdder(7).add, 3) -> 5 + 3 + 7 = 15 + // (lexical `this` round-tripped through step-arg serialization — + // the reducer captures `__boundThis`, the reviver re-binds) const run = await start(await e2e('instanceMethodStepWorkflow'), [5]); const returnValue = await run.returnValue; @@ -1806,6 +1811,8 @@ describe('e2e', () => { multiplied: 15, // 5 * 3 description: { label: 'test counter', value: 5 }, added2: 150, // 100 + 50 + adderResult: 14, // 5 + 2 + 7 (lexical `this` capture) + adderViaStep: 15, // 5 + 3 + 7 (lexical `this` survives serialization) }); // Verify the run completed successfully @@ -1819,9 +1826,15 @@ describe('e2e', () => { multiplied: 15, description: { label: 'test counter', value: 5 }, added2: 150, + adderResult: 14, + adderViaStep: 15, }); - // Verify the steps were executed (should have 4 steps: add, multiply, describe, add) + // Verify the steps were executed: + // - 4 Counter instance method steps (add, multiply, describe, add) + // - 2 lexical-`this` arrow steps from `makeAdder` (direct + via-step) + // - 1 invokeAdderFromStep wrapper (which itself triggers another + // makeAdder arrow step inside it) const { json: steps } = await cliInspectJson( `steps --runId ${run.runId}` ); @@ -1836,6 +1849,32 @@ describe('e2e', () => { expect(counterSteps.every((s: any) => s.status === 'completed')).toBe( true ); + + // The lexical-`this` arrow step inside `Counter#makeAdder` is + // hoisted by the SWC plugin under an `_anonymousStep` name. It + // ran once as its own step (`adder.add(2)` invoked directly from + // the workflow). The second call (inside `invokeAdderFromStep`) + // executes inline because steps invoked from another step body + // run inline rather than queueing a new step — so we only see one + // `_anonymousStep` event in the log, even though the body executed + // twice. Asserting `=== 1` here pins down both: + // 1. the direct invocation actually creates a step (i.e. the + // `bind(this)` proxy still goes through `useStep`), and + // 2. the round-tripped proxy correctly runs inline rather than + // somehow re-queuing a duplicate step. + const adderArrowSteps = steps.filter((s: any) => + s.stepName.includes('_anonymousStep') + ); + expect(adderArrowSteps.length).toBe(1); + expect(adderArrowSteps[0].status).toBe('completed'); + + // The `invokeAdderFromStep` wrapper itself runs as its own step; + // its body invokes the round-tripped `add` proxy inline. + const invokeAdderSteps = steps.filter((s: any) => + s.stepName.includes('invokeAdderFromStep') + ); + expect(invokeAdderSteps.length).toBe(1); + expect(invokeAdderSteps[0].status).toBe('completed'); } ); diff --git a/packages/core/src/serialization.test.ts b/packages/core/src/serialization.test.ts index b2a807a1c2..51cf0018a8 100644 --- a/packages/core/src/serialization.test.ts +++ b/packages/core/src/serialization.test.ts @@ -2274,6 +2274,148 @@ describe('step function serialization', () => { expect(result).toBe('Result: 21'); }); + it('should dehydrate and hydrate step function with closureVars + boundThis combined', async () => { + // The end-to-end shape exercised by the SWC plugin's lexical-`this` + // capture: a nested arrow step closes over both lexical `this` AND a + // surrounding closure variable. After serialization, the step-bundle + // reviver must run the registered body inside the closure-vars + // AsyncLocalStorage frame *and* invoke it with `apply(boundThis, + // args)`. + const stepName = 'step//workflows/test.ts//addToInstance'; + + const { __private_getClosureVars } = await import( + './step/get-closure-vars.js' + ); + const { contextStorage } = await import('./step/context-storage.js'); + + const stepFn = async function (this: { value: number }, amount: number) { + const { delta } = __private_getClosureVars() as { delta: number }; + return this.value + amount + delta; + }; + registerStepFunction(stepName, stepFn); + + Object.defineProperty(stepFn, 'stepId', { + value: stepName, + writable: false, + enumerable: false, + configurable: false, + }); + Object.defineProperty(stepFn, '__closureVarsFn', { + value: () => ({ delta: 7 }), + writable: false, + enumerable: false, + configurable: false, + }); + // Simulate the `__boundThis` marker that the step proxy's overridden + // `.bind` (in step.ts) would attach. Plain object instead of a real + // class instance so the test focuses on the reducer/reviver plumbing. + const instance = { value: 5 }; + Object.defineProperty(stepFn, '__boundThis', { + value: instance, + writable: false, + enumerable: false, + configurable: false, + }); + + // Round-trip through the workflow→step serialization pipeline. + const dehydrated = await dehydrateStepArguments( + [stepFn, 3], + mockRunId, + noEncryptionKey, + globalThis + ); + const hydrated = (await hydrateStepArguments( + dehydrated, + 'test-run-123', + noEncryptionKey, + [] + )) as [(amount: number) => Promise, number]; + + expect(typeof hydrated[0]).toBe('function'); + expect(hydrated[1]).toBe(3); + + // Invoke the rehydrated step function inside a step-context frame + // (otherwise the closure-vars wrapper throws). The wrapper must + // bind `this` to `instance` *and* expose `delta = 7` via + // `__private_getClosureVars()`. + const result = await contextStorage.run( + { + stepMetadata: { + stepName, + stepId: 'test-step', + stepStartedAt: new Date(), + attempt: 1, + }, + workflowMetadata: { + workflowName: 'workflow//workflows/test.ts//testWorkflow', + workflowRunId: 'test-run', + workflowStartedAt: new Date(), + url: 'http://localhost:3000', + features: { encryption: false }, + }, + ops: [], + }, + () => hydrated[0](3) + ); + + // value(5) + amount(3) + delta(7) = 15 + expect(result).toBe(15); + }); + + it('should preserve `boundArgs` (partial application) across serialization', async () => { + // The step proxy's overridden `.bind` also stashes prefilled args + // (`useStep(...).bind(thisArg, x)`) so partial application survives + // the round trip. The SWC plugin only ever emits `.bind(this)` today, + // so this codifies the safety net for future hand-written callers. + const stepName = 'step//workflows/test.ts//partialApply'; + + const stepFn = async function ( + this: { tag: string }, + prefilled: number, + runtimeArg: number + ) { + return `${this.tag}:${prefilled}+${runtimeArg}`; + }; + registerStepFunction(stepName, stepFn); + + Object.defineProperty(stepFn, 'stepId', { + value: stepName, + writable: false, + enumerable: false, + configurable: false, + }); + Object.defineProperty(stepFn, '__boundThis', { + value: { tag: 'bound' }, + writable: false, + enumerable: false, + configurable: false, + }); + Object.defineProperty(stepFn, '__boundArgs', { + value: [10], + writable: false, + enumerable: false, + configurable: false, + }); + + const dehydrated = await dehydrateStepArguments( + [stepFn], + mockRunId, + noEncryptionKey, + globalThis + ); + const hydrated = (await hydrateStepArguments( + dehydrated, + 'test-run-123', + noEncryptionKey, + [] + )) as [(runtimeArg: number) => Promise]; + + // The hydrated proxy should already have `prefilled = 10` baked in, + // so the caller only supplies `runtimeArg`. + const result = await hydrated[0](32); + expect(result).toBe('bound:10+32'); + }); + it('should serialize step function to object through reducer', () => { const stepName = 'step//workflows/test.ts//anotherStep'; const stepFn = async () => 'result'; diff --git a/packages/core/src/serialization.ts b/packages/core/src/serialization.ts index 37f931026b..4ee143c9a8 100644 --- a/packages/core/src/serialization.ts +++ b/packages/core/src/serialization.ts @@ -643,6 +643,8 @@ export interface SerializableSpecial { StepFunction: { stepId: string; closureVars?: Record; + boundThis?: unknown; + boundArgs?: unknown[]; }; URL: string; URLSearchParams: string; @@ -816,14 +818,35 @@ function getCommonReducers(global: Record = globalThis) { // Check if the step function has closure variables const closureVarsFn = (value as any).__closureVarsFn; - if (closureVarsFn && typeof closureVarsFn === 'function') { - // Invoke the closure variables function and serialize along with stepId - const closureVars = closureVarsFn(); - return { stepId, closureVars }; + const closureVars = + closureVarsFn && typeof closureVarsFn === 'function' + ? closureVarsFn() + : undefined; + + // `__boundThis` / `__boundArgs` are marker properties added by the + // step proxy's overridden `.bind` (see step.ts) to record the + // bound receiver and any prefilled arguments. Use `in` for + // `__boundThis` so we round-trip even when the bound `this` is + // `undefined`/`null`. `__boundArgs` is only set when the user + // actually supplied prefilled args, so a missing property means + // "no prefilled args". + const hasBoundThis = '__boundThis' in (value as any); + const boundThis = hasBoundThis ? (value as any).__boundThis : undefined; + const boundArgs = (value as any).__boundArgs as unknown[] | undefined; + + const payload: { + stepId: string; + closureVars?: Record; + boundThis?: unknown; + boundArgs?: unknown[]; + } = { stepId }; + if (closureVars !== undefined) payload.closureVars = closureVars; + if (hasBoundThis) payload.boundThis = boundThis; + if (Array.isArray(boundArgs) && boundArgs.length > 0) { + payload.boundArgs = boundArgs; } - // No closure variables - return object with just stepId - return { stepId }; + return payload; }, URL: (value) => value instanceof global.URL && value.href, URLSearchParams: (value) => { @@ -1313,11 +1336,20 @@ export function getWorkflowRevivers( ); } - if (closureVars) { - // For step functions with closure variables, create a wrapper that provides them - return useStep(stepId, () => closureVars); + const proxy = closureVars + ? useStep(stepId, () => closureVars) + : useStep(stepId); + + // If the serialized payload includes `boundThis` (and optionally + // `boundArgs`), re-bind the freshly-created proxy so a step proxy + // that was constructed with `.bind(this, …)` in the workflow + // bundle continues to carry that receiver and any prefilled + // arguments after being deserialized in another bundle. + if ('boundThis' in value) { + const boundArgs = Array.isArray(value.boundArgs) ? value.boundArgs : []; + return (proxy as any).bind(value.boundThis, ...boundArgs); } - return useStep(stepId); + return proxy; }, Request: (value) => { Object.setPrototypeOf(value, global.Request.prototype); @@ -1391,10 +1423,29 @@ function getStepRevivers( ...getCommonRevivers(global), // StepFunction reviver for step context - returns raw step function - // with closure variable support via AsyncLocalStorage + // with closure variable support via AsyncLocalStorage. + // + // Handles four independent flags from the serialized payload: + // - `closureVars`: invoke the body inside an AsyncLocalStorage frame + // so the SWC-emitted `WORKFLOW_STEP_CONTEXT_STORAGE` IIFE in the + // hoisted body can pull the closure variables back out. + // - `boundThis`: a `this` value captured by + // `useStep(...).bind(this)` in the workflow bundle (lexical-`this` + // arrow steps). The wrapper invokes the body via + // `stepFn.apply(boundThis, args)` so the body sees the same + // `this` it would have had in the workflow bundle. Property + // presence — not truthiness — is significant because + // `bind(null)` and `bind(undefined)` are both legal and should + // round-trip faithfully. + // - `boundArgs`: prefilled args from + // `useStep(...).bind(thisArg, x, y)`. Prepended to the call args + // so partial application survives serialization. StepFunction: (value) => { const stepId = value.stepId; const closureVars = value.closureVars; + const hasBoundThis = 'boundThis' in value; + const boundThis = hasBoundThis ? value.boundThis : undefined; + const boundArgs = Array.isArray(value.boundArgs) ? value.boundArgs : []; const stepFn = getStepFunction(stepId); if (!stepFn) { @@ -1403,47 +1454,47 @@ function getStepRevivers( ); } - // If closure variables were serialized, return a wrapper function - // that sets up AsyncLocalStorage context when invoked - if (closureVars) { - const wrappedStepFn = ((...args: any[]) => { - // Get the current context from AsyncLocalStorage - const currentContext = contextStorage.getStore(); + // Fast path: nothing to wrap. + if (!closureVars && !hasBoundThis && boundArgs.length === 0) { + return stepFn; + } + const wrappedStepFn = function (this: unknown, ...args: any[]) { + const callThis = hasBoundThis ? boundThis : this; + const callArgs = boundArgs.length > 0 ? [...boundArgs, ...args] : args; + if (closureVars) { + const currentContext = contextStorage.getStore(); if (!currentContext) { throw new Error( 'Cannot call step function with closure variables outside step context' ); } - - // Create a new context with the closure variables merged in const newContext = { ...currentContext, closureVars, }; - - // Run the step function with the new context that includes closure vars - return contextStorage.run(newContext, () => stepFn(...args)); - }) as any; - - // Copy properties from original step function - Object.defineProperty(wrappedStepFn, 'name', { - value: stepFn.name, - }); - Object.defineProperty(wrappedStepFn, 'stepId', { - value: stepId, - writable: false, - enumerable: false, - configurable: false, - }); - if (stepFn.maxRetries !== undefined) { - wrappedStepFn.maxRetries = stepFn.maxRetries; + return contextStorage.run(newContext, () => + stepFn.apply(callThis, callArgs) + ); } + return stepFn.apply(callThis, callArgs); + } as any; - return wrappedStepFn; + // Copy properties from original step function + Object.defineProperty(wrappedStepFn, 'name', { + value: stepFn.name, + }); + Object.defineProperty(wrappedStepFn, 'stepId', { + value: stepId, + writable: false, + enumerable: false, + configurable: false, + }); + if (stepFn.maxRetries !== undefined) { + wrappedStepFn.maxRetries = stepFn.maxRetries; } - return stepFn; + return wrappedStepFn; }, Request: (value) => { diff --git a/packages/core/src/step.test.ts b/packages/core/src/step.test.ts index d6f7f0fb43..efd61cd738 100644 --- a/packages/core/src/step.test.ts +++ b/packages/core/src/step.test.ts @@ -258,6 +258,65 @@ describe('createUseStep', () => { }); }); + // The SWC plugin emits `useStep(stepId, closureFn).bind(this)` for nested + // arrow steps that lexically capture `this`. The runtime relies on the + // step proxy being a regular `function` (not an arrow) so that `.bind(this)` + // works and the bound `this` is recorded as `thisVal` on the queue item. + it('captures `this` via .bind(this) on the step proxy (lexical-this support)', async () => { + const ctx = setupWorkflowContext([]); + let workflowErrorReject: (err: Error) => void; + const workflowErrorPromise = new Promise((_, reject) => { + workflowErrorReject = reject; + }); + ctx.onWorkflowError = (err) => { + workflowErrorReject(err); + }; + + const useStep = createUseStep(ctx); + + // Simulate the SWC plugin output: + // globalThis[Symbol.for('WORKFLOW_USE_STEP')]("step_id").bind(this) + // executed inside an enclosing method whose `this` is `instance`. + const instance = { name: 'enclosing-this' }; + const stepProxy = useStep('step//input.js//withThis', () => ({ x: 42 })); + const boundStep = stepProxy.bind(instance); + + // The bound proxy MUST retain the `stepId` and `__closureVarsFn` + // metadata that `getStepFunctionReducer` reads when serializing step + // function references — otherwise a bound proxy that flows through + // workflow serialization (e.g. as a step argument or return value) + // would be treated as a non-serializable plain function. + expect((boundStep as any).stepId).toBe('step//input.js//withThis'); + expect((boundStep as any).__closureVarsFn).toBe( + (stepProxy as any).__closureVarsFn + ); + // `__boundThis` is the marker the reducer uses to serialize the + // captured `this`, so a deserialized proxy in another bundle can + // re-bind to the same value. + expect((boundStep as any).__boundThis).toBe(instance); + + let error: Error | undefined; + try { + await Promise.race([boundStep(7), workflowErrorPromise]); + } catch (err_) { + error = err_ as Error; + } + + expect(error).toBeInstanceOf(WorkflowSuspension); + + // The bound `this` should have been captured on the queue item so the + // step runtime can `apply(thisVal, args)` when executing the step. + expect(ctx.invocationsQueue.size).toBe(1); + const queueItem = [...ctx.invocationsQueue.values()][0]; + expect(queueItem).toMatchObject({ + type: 'step', + stepName: 'step//input.js//withThis', + args: [7], + thisVal: instance, + closureVars: { x: 42 }, + }); + }); + it('should handle empty closure variables', async () => { // Use empty events to check queue state before step completes const ctx = setupWorkflowContext([]); diff --git a/packages/core/src/step.ts b/packages/core/src/step.ts index bd45c3008c..ca186066a3 100644 --- a/packages/core/src/step.ts +++ b/packages/core/src/step.ts @@ -209,6 +209,76 @@ export function createUseStep(ctx: WorkflowOrchestratorContext) { }); } + // Override `.bind` so the bound function preserves the step proxy + // metadata that `getStepFunctionReducer` relies on for serialization. + // Without this override, `Function.prototype.bind` would return a new + // function that doesn't inherit `stepId`, `__closureVarsFn`, or any + // other own properties of the original proxy — so the StepFunction + // reducer would refuse to serialize it (it'd look like a plain + // function), and a `useStep(...).bind(this)` proxy that flowed + // through workflow serialization would silently break. + // + // The override stashes three pieces of state on the bound function so + // the round trip is faithful: + // - `stepId` — already set on the original proxy. + // - `__closureVarsFn` — only when the original proxy had one. + // - `__boundThis` — the receiver passed to `.bind(thisArg, …)`. + // Always set (even when `thisArg` is + // `null`/`undefined`) so the reducer can + // distinguish "was bound" from "wasn't". + // - `__boundArgs` — only when the user supplied prefilled + // arguments (`.bind(thisArg, x, y)`). The + // SWC plugin only ever emits `.bind(this)` + // today, so this is rare in practice; we + // still capture it so the partial args + // survive serialization rather than + // silently disappearing on the step side. + Object.defineProperty(stepFunction, 'bind', { + value: function ( + this: typeof stepFunction, + thisArg: unknown, + ...partialArgs: unknown[] + ) { + const bound = Function.prototype.bind.call( + this, + thisArg, + ...partialArgs + ); + Object.defineProperty(bound, 'stepId', { + value: stepName, + writable: false, + enumerable: false, + configurable: false, + }); + if (closureVarsFn) { + Object.defineProperty(bound, '__closureVarsFn', { + value: closureVarsFn, + writable: false, + enumerable: false, + configurable: false, + }); + } + Object.defineProperty(bound, '__boundThis', { + value: thisArg, + writable: false, + enumerable: false, + configurable: false, + }); + if (partialArgs.length > 0) { + Object.defineProperty(bound, '__boundArgs', { + value: partialArgs, + writable: false, + enumerable: false, + configurable: false, + }); + } + return bound; + }, + writable: false, + enumerable: false, + configurable: false, + }); + return stepFunction; }; } diff --git a/packages/swc-plugin-workflow/spec.md b/packages/swc-plugin-workflow/spec.md index e674ad7424..87da35340b 100644 --- a/packages/swc-plugin-workflow/spec.md +++ b/packages/swc-plugin-workflow/spec.md @@ -1065,10 +1065,90 @@ The plugin detects this pattern and correctly identifies the directive inside th --- +## Lexical `this` Capture in Nested Arrow Steps + +When a nested arrow-function step references `this` from an enclosing +function/method scope, the plugin captures that `this` so the workflow +runtime can rebind it inside the executing step body. This makes the +following pattern work — the user's class is responsible for providing +custom serialization (`WORKFLOW_SERIALIZE` / `WORKFLOW_DESERIALIZE`) so the +captured `this` can survive the workflow→step boundary: + +Input: +```javascript +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; + +export class ReadFileTool { + static [WORKFLOW_SERIALIZE](instance) { + return { service: instance.service }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new ReadFileTool(data.service); + } + constructor(service) { + this.service = service; + } + createTool(context) { + return tool({ + execute: async (input) => { + 'use step'; + return this.service.readFileContent(input, context); + }, + }); + } +} +``` + +Output (Workflow Mode) — the proxy reference is wrapped with `.bind(this)` +so the runtime's step proxy captures the caller's `this` as `thisVal` on the +invocation queue item: +```javascript +createTool(context) { + return tool({ + execute: globalThis[Symbol.for("WORKFLOW_USE_STEP")]( + "step//./input//_anonymousStep0", + () => ({ context }) + ).bind(this), + }); +} +``` + +Output (Step Mode) — the step body is hoisted as a regular `function` (not +an arrow) so the runtime's `stepFn.apply(thisVal, args)` can rebind `this` +to the value that was captured at call time: +```javascript +async function _anonymousStep0(input) { + const { context } = (function() { /* closure-var IIFE */ })(); + return this.service.readFileContent(input, context); +} +``` + +Detection rules: +- Only `this` references that are **lexically captured by an arrow** count. + An arrow function inherits `this` from its enclosing scope; a nested + `function`/method/getter/setter introduces its own `this` and is therefore + not traversed by the detector. +- The detector only flags arrows that are themselves step functions. A + `this` reference inside a non-step nested arrow inside a step does still + count, because the inner arrow inherits `this` from the step function + body, which in turn inherits from the enclosing function. + +Caveat: capturing `this` only works at runtime if the captured value is +serializable across the workflow→step boundary. Classes registered with +`WORKFLOW_SERIALIZE` / `WORKFLOW_DESERIALIZE` work; ordinary class +instances without custom serialization will fail at proxy-invocation time. + +--- + ## Notes - Arguments and return values must be serializable (JSON-compatible or using custom serialization) -- The `this` keyword and `arguments` object are not allowed in step functions +- `this` is syntactically allowed inside step bodies, but it only carries a meaningful value in two shapes that both flow through the runtime's `thisVal` plumbing: + 1. **Instance-method steps** on a class with custom serialization (e.g. `Counter#add`). Calling `instance.add(...)` captures `instance` as `thisVal` so the step body sees `this === instance`. + 2. **Nested arrow steps that lexically capture `this`** (see "Lexical `this` Capture in Nested Arrow Steps" above). The compiler emits `.bind(this)` on the proxy in workflow mode and hoists the body as a regular `function` in step mode so `stepFn.apply(thisVal, args)` rebinds correctly. + + Other shapes (a top-level `async function` step that references `this`, an arrow step assigned to a module-level variable, etc.) compile without error but `this` will be whatever the caller of the step proxy passes — typically `null`/`undefined` — so referencing it is rarely useful. +- `arguments` is allowed inside `function`-form step bodies (it reflects the positional arguments the runtime passes via `stepFn.apply(thisVal, args)`). It does **not** work inside arrow-form steps — arrows don't have their own `arguments` binding, and the compiler doesn't capture the enclosing scope's `arguments` the way it does for `this`. Use rest parameters (`...args`) instead if you need that pattern in an arrow step. - `super` calls are not allowed in step functions - Imports from the module are excluded from closure variable detection - Module-level declarations (functions, variables, classes) are excluded from closure variable detection, since they are available directly in the step bundle and should not be serialized as closure values diff --git a/packages/swc-plugin-workflow/transform/src/lib.rs b/packages/swc-plugin-workflow/transform/src/lib.rs index 97fec5125f..e4bc6ce8b4 100644 --- a/packages/swc-plugin-workflow/transform/src/lib.rs +++ b/packages/swc-plugin-workflow/transform/src/lib.rs @@ -6,7 +6,7 @@ use swc_core::{ common::{errors::HANDLER, SyntaxContext, DUMMY_SP}, ecma::{ ast::*, - visit::{noop_visit_mut_type, VisitMut, VisitMutWith}, + visit::{noop_visit_mut_type, noop_visit_type, Visit, VisitMut, VisitMutWith, VisitWith}, }, }; @@ -341,7 +341,14 @@ pub struct StepTransform { object_property_step_functions: Vec<(String, String, FnExpr, swc_core::common::Span, String, bool)>, // Track nested step functions inside workflow functions for hoisting in step mode - // (fn_name, fn_expr, span, closure_vars, was_arrow, parent_workflow_name) + // (fn_name, fn_expr, span, closure_vars, was_arrow, parent_workflow_name, references_lexical_this) + // + // `references_lexical_this` is set when the original step function was an + // arrow whose body references the enclosing `this`. In step mode such a + // step is hoisted as a regular `function` (not an arrow) so the runtime + // can rebind `this` via `stepFn.apply(thisVal, args)`. In workflow mode + // the proxy reference is wrapped with `.bind(this)` so the step proxy + // captures the caller's `this` and forwards it as `thisVal`. nested_step_functions: Vec<( String, FnExpr, @@ -349,6 +356,7 @@ pub struct StepTransform { Vec, bool, String, + bool, )>, // Counter for anonymous function names #[allow(dead_code)] @@ -1093,7 +1101,15 @@ impl ClosureVariableCollector { fn is_global_identifier(name: &str) -> bool { matches!( name, - "console" + // `arguments` is a per-function intrinsic binding, not a closure + // variable. Treat it as a global so the closure-variable collector + // doesn't try to serialize it and the hoisted body doesn't end up + // with `const { arguments } = ...` (which is a syntax error in + // strict mode anyway). Hoisted `function`-form steps see their own + // `arguments` reflecting the positional args passed via + // `stepFn.apply(thisVal, args)`. + "arguments" + | "console" | "process" | "global" | "globalThis" @@ -1227,6 +1243,99 @@ impl VisitMut for ClosureVariableNormalizer { noop_visit_mut_type!(); } +// Visitor that detects whether an arrow-function body references its lexical +// `this` (i.e. a `this` whose binding comes from an enclosing function/method). +// +// Arrow functions inherit `this` lexically; regular `function` expressions and +// methods introduce their own `this` binding. So when scanning an arrow body +// we recurse into nested arrows but stop at non-arrow function/method/class +// member boundaries (a `this` inside one of those is bound by the inner +// function, not by the outer scope). +// +// Class bodies are also boundaries: `this` inside class property initializers, +// methods, getters/setters, constructors, and static blocks is bound to the +// class instance/static class, not to the enclosing arrow. We still traverse +// `extends` expressions and computed-key expressions because those are +// evaluated in the surrounding scope. +struct LexicalThisDetector { + found: bool, +} + +impl LexicalThisDetector { + fn new() -> Self { + Self { found: false } + } + + fn detect_in_arrow(arrow: &ArrowExpr) -> bool { + let mut detector = Self::new(); + // Walk both params (default values, destructuring initializers can + // contain `this`, e.g. `(x = this.foo) => ...`) and the body. + for param in &arrow.params { + param.visit_with(&mut detector); + } + arrow.body.visit_with(&mut detector); + detector.found + } +} + +impl Visit for LexicalThisDetector { + noop_visit_type!(); + + fn visit_this_expr(&mut self, _: &ThisExpr) { + self.found = true; + } + + // Do not descend into nested non-arrow functions / methods / getters / + // setters / constructors / class static blocks — those introduce their + // own `this` binding. + fn visit_function(&mut self, _: &Function) {} + fn visit_constructor(&mut self, _: &Constructor) {} + fn visit_getter_prop(&mut self, _: &GetterProp) {} + fn visit_setter_prop(&mut self, _: &SetterProp) {} + fn visit_method_prop(&mut self, _: &MethodProp) {} + fn visit_class_method(&mut self, _: &ClassMethod) {} + fn visit_private_method(&mut self, _: &PrivateMethod) {} + fn visit_static_block(&mut self, _: &StaticBlock) {} + + // Class declarations / expressions: `this` inside a class body member is + // bound to the class instance/static class, not the enclosing arrow. We + // still need to visit `extends` clauses and computed property keys + // because those are evaluated in the outer scope. + fn visit_class(&mut self, class: &Class) { + if let Some(super_class) = &class.super_class { + super_class.visit_with(self); + } + for member in &class.body { + // For each member, only walk the parts evaluated in the outer + // scope (computed keys); the value/body is evaluated with `this` + // bound to the class. + match member { + ClassMember::ClassProp(p) => { + if let PropName::Computed(computed) = &p.key { + computed.expr.visit_with(self); + } + } + ClassMember::PrivateProp(_) => { /* private name keys are not expressions */ } + ClassMember::Method(m) => { + if let PropName::Computed(computed) = &m.key { + computed.expr.visit_with(self); + } + } + ClassMember::PrivateMethod(_) => { /* private name keys are not expressions */ } + ClassMember::Constructor(c) => { + if let PropName::Computed(computed) = &c.key { + computed.expr.visit_with(self); + } + } + ClassMember::StaticBlock(_) => { /* `this` inside is class itself */ } + ClassMember::Empty(_) + | ClassMember::TsIndexSignature(_) + | ClassMember::AutoAccessor(_) => {} + } + } + } +} + impl StepTransform { fn process_stmt(&mut self, stmt: &mut Stmt) { match stmt { @@ -1274,6 +1383,7 @@ impl StepTransform { self.current_parent_function_name .clone() .unwrap_or_default(), + false, // Regular functions have their own `this` )); // Keep the original function declaration with the directive stripped, @@ -3198,6 +3308,503 @@ impl StepTransform { }) } + // Create an inline step function registration statement (step mode). + // Instead of importing registerStepFunction from "workflow/internal/private", + // we inline the registration logic as a self-contained IIFE that has zero module dependencies. + // This is critical for 3rd-party packages that define step functions but don't depend + // on the "workflow" package directly. + // + // Generates: + // (function(__wf_fn, __wf_id) { + // var __wf_sym = Symbol.for("@workflow/core//registeredSteps"), + // __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + // __wf_reg.set(__wf_id, __wf_fn); + // __wf_fn.stepId = __wf_id; + // })(fnRef, "step//module_path//fnName"); + #[allow(dead_code)] + fn create_inline_step_registration(&self, step_id: &str, fn_ref: Expr, fn_name: &str) -> Stmt { + // Helper to create an identifier + let ident = + |name: &str| -> Ident { Ident::new(name.into(), DUMMY_SP, SyntaxContext::empty()) }; + + // Helper to create an identifier expression + let ident_expr = |name: &str| -> Box { Box::new(Expr::Ident(ident(name))) }; + + // var __wf_sym = Symbol.for("@workflow/core//registeredSteps"), + // __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + let sym_decl = VarDeclarator { + span: DUMMY_SP, + name: Pat::Ident(BindingIdent { + id: ident("__wf_sym"), + type_ann: None, + }), + init: Some(Box::new(Expr::Call(CallExpr { + span: DUMMY_SP, + ctxt: SyntaxContext::empty(), + callee: Callee::Expr(Box::new(Expr::Member(MemberExpr { + span: DUMMY_SP, + obj: ident_expr("Symbol"), + prop: MemberProp::Ident(IdentName { + span: DUMMY_SP, + sym: "for".into(), + }), + }))), + args: vec![ExprOrSpread { + spread: None, + expr: Box::new(Expr::Lit(Lit::Str(Str { + span: DUMMY_SP, + value: "@workflow/core//registeredSteps".into(), + raw: None, + }))), + }], + type_args: None, + }))), + definite: false, + }; + + let global_sym_access = Box::new(Expr::Member(MemberExpr { + span: DUMMY_SP, + obj: ident_expr("globalThis"), + prop: MemberProp::Computed(ComputedPropName { + span: DUMMY_SP, + expr: ident_expr("__wf_sym"), + }), + })); + + let reg_decl = VarDeclarator { + span: DUMMY_SP, + name: Pat::Ident(BindingIdent { + id: ident("__wf_reg"), + type_ann: None, + }), + init: Some(Box::new(Expr::Bin(BinExpr { + span: DUMMY_SP, + op: BinaryOp::LogicalOr, + left: global_sym_access.clone(), + right: Box::new(Expr::Paren(ParenExpr { + span: DUMMY_SP, + expr: Box::new(Expr::Assign(AssignExpr { + span: DUMMY_SP, + op: AssignOp::Assign, + left: AssignTarget::Simple(SimpleAssignTarget::Member(MemberExpr { + span: DUMMY_SP, + obj: ident_expr("globalThis"), + prop: MemberProp::Computed(ComputedPropName { + span: DUMMY_SP, + expr: ident_expr("__wf_sym"), + }), + })), + right: Box::new(Expr::New(NewExpr { + span: DUMMY_SP, + ctxt: SyntaxContext::empty(), + callee: ident_expr("Map"), + args: Some(vec![]), + type_args: None, + })), + })), + })), + }))), + definite: false, + }; + + // __wf_reg.set(__wf_id, __wf_fn); + let set_call = Stmt::Expr(ExprStmt { + span: DUMMY_SP, + expr: Box::new(Expr::Call(CallExpr { + span: DUMMY_SP, + ctxt: SyntaxContext::empty(), + callee: Callee::Expr(Box::new(Expr::Member(MemberExpr { + span: DUMMY_SP, + obj: ident_expr("__wf_reg"), + prop: MemberProp::Ident(IdentName { + span: DUMMY_SP, + sym: "set".into(), + }), + }))), + args: vec![ + ExprOrSpread { + spread: None, + expr: ident_expr("__wf_id"), + }, + ExprOrSpread { + spread: None, + expr: ident_expr("__wf_fn"), + }, + ], + type_args: None, + })), + }); + + // __wf_fn.stepId = __wf_id; + let step_id_assignment = Stmt::Expr(ExprStmt { + span: DUMMY_SP, + expr: Box::new(Expr::Assign(AssignExpr { + span: DUMMY_SP, + op: AssignOp::Assign, + left: AssignTarget::Simple(SimpleAssignTarget::Member(MemberExpr { + span: DUMMY_SP, + obj: ident_expr("__wf_fn"), + prop: MemberProp::Ident(IdentName { + span: DUMMY_SP, + sym: "stepId".into(), + }), + })), + right: ident_expr("__wf_id"), + })), + }); + + // Object.defineProperty(__wf_fn, "name", { value: "originalName", configurable: true }) + // This preserves the original function name in stack traces even after bundler minification. + let define_name = Stmt::Expr(ExprStmt { + span: DUMMY_SP, + expr: Box::new(Expr::Call(CallExpr { + span: DUMMY_SP, + ctxt: SyntaxContext::empty(), + callee: Callee::Expr(Box::new(Expr::Member(MemberExpr { + span: DUMMY_SP, + obj: ident_expr("Object"), + prop: MemberProp::Ident(IdentName { + span: DUMMY_SP, + sym: "defineProperty".into(), + }), + }))), + args: vec![ + ExprOrSpread { + spread: None, + expr: ident_expr("__wf_fn"), + }, + ExprOrSpread { + spread: None, + expr: Box::new(Expr::Lit(Lit::Str(Str { + span: DUMMY_SP, + value: "name".into(), + raw: None, + }))), + }, + ExprOrSpread { + spread: None, + expr: Box::new(Expr::Object(ObjectLit { + span: DUMMY_SP, + props: vec![ + PropOrSpread::Prop(Box::new(Prop::KeyValue(KeyValueProp { + key: PropName::Ident(IdentName { + span: DUMMY_SP, + sym: "value".into(), + }), + value: Box::new(Expr::Lit(Lit::Str(Str { + span: DUMMY_SP, + value: fn_name.into(), + raw: None, + }))), + }))), + PropOrSpread::Prop(Box::new(Prop::KeyValue(KeyValueProp { + key: PropName::Ident(IdentName { + span: DUMMY_SP, + sym: "configurable".into(), + }), + value: Box::new(Expr::Lit(Lit::Bool(Bool { + span: DUMMY_SP, + value: true, + }))), + }))), + ], + })), + }, + ], + type_args: None, + })), + }); + + // The function body: var decls + set + stepId assignment + name preservation + let function_body = BlockStmt { + span: DUMMY_SP, + ctxt: SyntaxContext::empty(), + stmts: vec![ + Stmt::Decl(Decl::Var(Box::new(VarDecl { + span: DUMMY_SP, + ctxt: SyntaxContext::empty(), + kind: VarDeclKind::Var, + declare: false, + decls: vec![sym_decl, reg_decl], + }))), + set_call, + step_id_assignment, + define_name, + ], + }; + + // The IIFE: (function(__wf_fn, __wf_id) { ... })(fnRef, "step_id"); + Stmt::Expr(ExprStmt { + span: DUMMY_SP, + expr: Box::new(Expr::Call(CallExpr { + span: DUMMY_SP, + ctxt: SyntaxContext::empty(), + callee: Callee::Expr(Box::new(Expr::Paren(ParenExpr { + span: DUMMY_SP, + expr: Box::new(Expr::Fn(FnExpr { + ident: None, + function: Box::new(Function { + params: vec![ + Param { + span: DUMMY_SP, + decorators: vec![], + pat: Pat::Ident(BindingIdent { + id: ident("__wf_fn"), + type_ann: None, + }), + }, + Param { + span: DUMMY_SP, + decorators: vec![], + pat: Pat::Ident(BindingIdent { + id: ident("__wf_id"), + type_ann: None, + }), + }, + ], + decorators: vec![], + span: DUMMY_SP, + ctxt: SyntaxContext::empty(), + body: Some(function_body), + is_generator: false, + is_async: false, + type_params: None, + return_type: None, + }), + })), + }))), + args: vec![ + ExprOrSpread { + spread: None, + expr: Box::new(fn_ref), + }, + ExprOrSpread { + spread: None, + expr: Box::new(Expr::Lit(Lit::Str(Str { + span: DUMMY_SP, + value: step_id.into(), + raw: None, + }))), + }, + ], + type_args: None, + })), + }) + } + + // Create an inline closure variable access expression (step mode). + // Instead of importing __private_getClosureVars from "workflow/internal/private", + // we inline the access as a self-contained IIFE that reads from the global + // AsyncLocalStorage context. + // + // Generates: + // (function() { + // var __wf_ctx = globalThis[Symbol.for("WORKFLOW_STEP_CONTEXT_STORAGE")], + // __wf_store = __wf_ctx && __wf_ctx.getStore(); + // if (!__wf_store) throw new Error("Closure variables can only be accessed inside a step function"); + // return __wf_store.closureVars || {}; + // })() + #[allow(dead_code)] + fn create_inline_get_closure_vars(&self) -> Expr { + let ident = + |name: &str| -> Ident { Ident::new(name.into(), DUMMY_SP, SyntaxContext::empty()) }; + let ident_expr = |name: &str| -> Box { Box::new(Expr::Ident(ident(name))) }; + + // var __wf_ctx = globalThis[Symbol.for("WORKFLOW_STEP_CONTEXT_STORAGE")] + let ctx_decl = VarDeclarator { + span: DUMMY_SP, + name: Pat::Ident(BindingIdent { + id: ident("__wf_ctx"), + type_ann: None, + }), + init: Some(Box::new(Expr::Member(MemberExpr { + span: DUMMY_SP, + obj: ident_expr("globalThis"), + prop: MemberProp::Computed(ComputedPropName { + span: DUMMY_SP, + expr: Box::new(Expr::Call(CallExpr { + span: DUMMY_SP, + ctxt: SyntaxContext::empty(), + callee: Callee::Expr(Box::new(Expr::Member(MemberExpr { + span: DUMMY_SP, + obj: ident_expr("Symbol"), + prop: MemberProp::Ident(IdentName { + span: DUMMY_SP, + sym: "for".into(), + }), + }))), + args: vec![ExprOrSpread { + spread: None, + expr: Box::new(Expr::Lit(Lit::Str(Str { + span: DUMMY_SP, + value: "WORKFLOW_STEP_CONTEXT_STORAGE".into(), + raw: None, + }))), + }], + type_args: None, + })), + }), + }))), + definite: false, + }; + + // __wf_store = __wf_ctx && __wf_ctx.getStore() + let store_decl = VarDeclarator { + span: DUMMY_SP, + name: Pat::Ident(BindingIdent { + id: ident("__wf_store"), + type_ann: None, + }), + init: Some(Box::new(Expr::Bin(BinExpr { + span: DUMMY_SP, + op: BinaryOp::LogicalAnd, + left: ident_expr("__wf_ctx"), + right: Box::new(Expr::Call(CallExpr { + span: DUMMY_SP, + ctxt: SyntaxContext::empty(), + callee: Callee::Expr(Box::new(Expr::Member(MemberExpr { + span: DUMMY_SP, + obj: ident_expr("__wf_ctx"), + prop: MemberProp::Ident(IdentName { + span: DUMMY_SP, + sym: "getStore".into(), + }), + }))), + args: vec![], + type_args: None, + })), + }))), + definite: false, + }; + + // if (!__wf_store) throw new Error("Closure variables can only be accessed inside a step function"); + // return __wf_store.closureVars || {}; + let throw_if_missing = Stmt::If(IfStmt { + span: DUMMY_SP, + test: Box::new(Expr::Unary(UnaryExpr { + span: DUMMY_SP, + op: UnaryOp::Bang, + arg: ident_expr("__wf_store"), + })), + cons: Box::new(Stmt::Throw(ThrowStmt { + span: DUMMY_SP, + arg: Box::new(Expr::New(NewExpr { + span: DUMMY_SP, + ctxt: SyntaxContext::empty(), + callee: ident_expr("Error"), + args: Some(vec![ExprOrSpread { + spread: None, + expr: Box::new(Expr::Lit(Lit::Str(Str { + span: DUMMY_SP, + value: "Closure variables can only be accessed inside a step function" + .into(), + raw: None, + }))), + }]), + type_args: None, + })), + })), + alt: None, + }); + + let return_stmt = Stmt::Return(ReturnStmt { + span: DUMMY_SP, + arg: Some(Box::new(Expr::Bin(BinExpr { + span: DUMMY_SP, + op: BinaryOp::LogicalOr, + left: Box::new(Expr::Member(MemberExpr { + span: DUMMY_SP, + obj: ident_expr("__wf_store"), + prop: MemberProp::Ident(IdentName { + span: DUMMY_SP, + sym: "closureVars".into(), + }), + })), + right: Box::new(Expr::Object(ObjectLit { + span: DUMMY_SP, + props: vec![], + })), + }))), + }); + + let function_body = BlockStmt { + span: DUMMY_SP, + ctxt: SyntaxContext::empty(), + stmts: vec![ + Stmt::Decl(Decl::Var(Box::new(VarDecl { + span: DUMMY_SP, + ctxt: SyntaxContext::empty(), + kind: VarDeclKind::Var, + declare: false, + decls: vec![ctx_decl, store_decl], + }))), + throw_if_missing, + return_stmt, + ], + }; + + // (function() { ... })() + Expr::Call(CallExpr { + span: DUMMY_SP, + ctxt: SyntaxContext::empty(), + callee: Callee::Expr(Box::new(Expr::Paren(ParenExpr { + span: DUMMY_SP, + expr: Box::new(Expr::Fn(FnExpr { + ident: None, + function: Box::new(Function { + params: vec![], + decorators: vec![], + span: DUMMY_SP, + ctxt: SyntaxContext::empty(), + body: Some(function_body), + is_generator: false, + is_async: false, + type_params: None, + return_type: None, + }), + })), + }))), + args: vec![], + type_args: None, + }) + } + + // Wrap an expression with `.bind(this)` so the resulting proxy captures + // the caller's lexical `this`. Used when the original step was an arrow + // function whose body referenced the enclosing function's `this`. + fn wrap_with_bind_this(expr: Expr) -> Expr { + Expr::Call(CallExpr { + span: DUMMY_SP, + ctxt: SyntaxContext::empty(), + callee: Callee::Expr(Box::new(Expr::Member(MemberExpr { + span: DUMMY_SP, + obj: Box::new(expr), + prop: MemberProp::Ident(IdentName::new("bind".into(), DUMMY_SP)), + }))), + args: vec![ExprOrSpread { + spread: None, + expr: Box::new(Expr::This(ThisExpr { span: DUMMY_SP })), + }], + type_args: None, + }) + } + + // Same as `create_step_proxy_reference` but wrapped with `.bind(this)` + // when `bind_this` is true. + fn create_step_proxy_reference_maybe_bound( + &self, + step_id: &str, + closure_vars: &[String], + bind_this: bool, + ) -> Expr { + let proxy = self.create_step_proxy_reference(step_id, closure_vars); + if bind_this { + Self::wrap_with_bind_this(proxy) + } else { + proxy + } + } + // Create a proxy reference: globalThis[Symbol.for("WORKFLOW_USE_STEP")]("step_id", closure_fn) (workflow mode) fn create_step_proxy_reference(&self, step_id: &str, closure_vars: &[String]) -> Expr { let mut args = vec![ExprOrSpread { @@ -4247,7 +4854,7 @@ impl VisitMut for StepTransform { let needs_closure_import = self .nested_step_functions .iter() - .any(|(_, _, _, closure_vars, _, _)| !closure_vars.is_empty()); + .any(|(_, _, _, closure_vars, _, _, _)| !closure_vars.is_empty()); if needs_register_import || needs_closure_import { imports_to_add.push(self.create_private_imports( @@ -4292,6 +4899,7 @@ impl VisitMut for StepTransform { closure_vars, was_arrow, parent_workflow_name, + references_lexical_this, ) in nested_functions { // Generate hoisted name including parent workflow function name @@ -4363,8 +4971,13 @@ impl VisitMut for StepTransform { } } - // Create the appropriate hoisted declaration based on original function type - let hoisted_decl = if was_arrow { + // Create the appropriate hoisted declaration based on original function type. + // + // If the original arrow body referenced lexical `this`, we + // hoist as a regular `function` (not an arrow) so that the + // workflow runtime's `stepFn.apply(thisVal, args)` can + // rebind `this` to the value captured at call time. + let hoisted_decl = if was_arrow && !references_lexical_this { // Convert back to arrow function: var name = async () => { ... }; let arrow_expr = self.convert_fn_expr_to_arrow(&fn_expr); ModuleItem::Stmt(Stmt::Decl(Decl::Var(Box::new(VarDecl { @@ -5185,23 +5798,18 @@ impl VisitMut for StepTransform { self.in_module_level = old_in_module; } - // Add forbidden expression checks - fn visit_mut_this_expr(&mut self, expr: &mut ThisExpr) { - if self.in_step_function { - emit_error(WorkflowErrorKind::ForbiddenExpression { - span: expr.span, - expr: "this", - directive: "use step", - }); - } else if self.in_workflow_function { - emit_error(WorkflowErrorKind::ForbiddenExpression { - span: expr.span, - expr: "this", - directive: "use workflow", - }); - } - } - + // Forbidden-expression check for `super` inside step/workflow bodies. + // + // Note: corresponding checks for `this` and `arguments` were removed + // because they were dead in practice (the `'use step'` / `'use workflow'` + // directives are stripped during the module-level traversal before this + // visitor runs over function bodies, so `in_step_function` / + // `in_workflow_function` are never observed as `true` here for those + // expressions). The existing `step-with-this-arguments-super` fixture + // documents that `this`, `arguments`, and `super` are allowed in step + // bodies. `super` is left flagged here for now — the same caveat applies + // — but the wording in `spec.md` reflects the actual runtime behavior + // for the other two. fn visit_mut_super(&mut self, sup: &mut Super) { if self.in_step_function { emit_error(WorkflowErrorKind::ForbiddenExpression { @@ -5218,24 +5826,6 @@ impl VisitMut for StepTransform { } } - fn visit_mut_ident(&mut self, ident: &mut Ident) { - if ident.sym == *"arguments" { - if self.in_step_function { - emit_error(WorkflowErrorKind::ForbiddenExpression { - span: ident.span, - expr: "arguments", - directive: "use step", - }); - } else if self.in_workflow_function { - emit_error(WorkflowErrorKind::ForbiddenExpression { - span: ident.span, - expr: "arguments", - directive: "use workflow", - }); - } - } - } - // Track when we're in a callee position fn visit_mut_callee(&mut self, callee: &mut Callee) { let old_in_callee = self.in_callee; @@ -7004,6 +7594,13 @@ impl VisitMut for StepTransform { // Check if we're inside any function (nested), not just workflow functions if !self.in_module_level { + // Detect whether the arrow body references the + // enclosing function's `this`. If so, we need to + // hoist as a regular function (so `apply(thisVal, + // args)` rebinds) and `.bind(this)` the workflow- + // mode proxy reference. + let references_lexical_this = + LexicalThisDetector::detect_in_arrow(arrow_expr); match self.mode { TransformMode::Step => { // Hoist arrow function to module scope @@ -7073,6 +7670,7 @@ impl VisitMut for StepTransform { self.current_parent_function_name .clone() .unwrap_or_default(), + references_lexical_this, )); // Keep the original arrow with the directive stripped, @@ -7100,10 +7698,13 @@ impl VisitMut for StepTransform { // Collect closure variables let closure_vars = ClosureVariableCollector::collect_from_arrow_expr(&arrow_expr, &self.module_imports, &self.declared_identifiers); - *init = Box::new(self.create_step_proxy_reference( - &step_id, - &closure_vars, - )); + *init = Box::new( + self.create_step_proxy_reference_maybe_bound( + &step_id, + &closure_vars, + references_lexical_this, + ), + ); } TransformMode::Client => { // In client mode for nested step functions, just remove directive @@ -7883,6 +8484,7 @@ impl VisitMut for StepTransform { self.current_parent_function_name .clone() .unwrap_or_default(), + false, // Regular functions have their own `this` )); // Keep the original function with the directive stripped, @@ -7939,6 +8541,11 @@ impl VisitMut for StepTransform { self.anonymous_fn_counter += 1; self.step_function_names.insert(name.clone()); + // Detect whether the arrow body references the + // enclosing function's `this` (lexical capture). + let references_lexical_this = + LexicalThisDetector::detect_in_arrow(arrow_expr); + match self.mode { TransformMode::Step => { // Hoist to module scope @@ -7999,6 +8606,7 @@ impl VisitMut for StepTransform { self.current_parent_function_name .clone() .unwrap_or_default(), + references_lexical_this, )); // Keep the original arrow with the directive stripped, @@ -8029,7 +8637,11 @@ impl VisitMut for StepTransform { &self.module_imports, &self.declared_identifiers, ); - *expr = self.create_step_proxy_reference(&step_id, &closure_vars); + *expr = self.create_step_proxy_reference_maybe_bound( + &step_id, + &closure_vars, + references_lexical_this, + ); return; // Don't visit children since we replaced the expr } TransformMode::Client => { @@ -8524,6 +9136,15 @@ impl VisitMut for StepTransform { self.step_function_names .insert(generated_name.clone()); + // Detect lexical `this` capture so we + // can hoist as a regular function (step + // mode) and `.bind(this)` the proxy + // (workflow mode). + let references_lexical_this = + LexicalThisDetector::detect_in_arrow( + arrow_expr, + ); + match self.mode { TransformMode::Step => { // Hoist to module scope @@ -8599,6 +9220,7 @@ impl VisitMut for StepTransform { self.current_workflow_function_name .clone() .unwrap_or_default(), + references_lexical_this, )); // Keep the original arrow with the directive stripped @@ -8628,9 +9250,10 @@ impl VisitMut for StepTransform { // Collect closure variables let closure_vars = ClosureVariableCollector::collect_from_arrow_expr(&arrow_expr, &self.module_imports, &self.declared_identifiers); *kv_prop.value = self - .create_step_proxy_reference( + .create_step_proxy_reference_maybe_bound( &step_id, &closure_vars, + references_lexical_this, ); } TransformMode::Client => { @@ -8689,6 +9312,7 @@ impl VisitMut for StepTransform { self.current_workflow_function_name .clone() .unwrap_or_default(), + false, // Regular functions have their own `this` )); // Keep the original function with the directive stripped @@ -8787,6 +9411,7 @@ impl VisitMut for StepTransform { self.current_workflow_function_name .clone() .unwrap_or_default(), + false, // Methods have their own `this` )); // Keep the original method with the directive stripped diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/input.js b/packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/input.js new file mode 100644 index 0000000000..5177c86d01 --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/input.js @@ -0,0 +1,52 @@ +// Edge cases for the lexical-`this` detector that drives `.bind(this)` and +// the arrow→function hoisting choice. +// +// - default parameter initializers see lexical `this` ⇒ should bind +// - destructuring parameter defaults see lexical `this` ⇒ should bind +// - class field initializers / methods inside the arrow body bind their own +// `this` (the class instance), so they should NOT trigger the detector +// - `extends` clauses and computed property keys inside such a class are +// evaluated in the outer scope, so `this` references there SHOULD trigger +// the detector. +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; + +export class Edge { + static [WORKFLOW_SERIALIZE](inst) { + return { value: inst.value }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new Edge(data.value); + } + constructor(value) { + this.value = value; + } + + // `this` appears only in a default parameter (no body reference). + // Detector should still flag this and emit `.bind(this)`. + withThisInDefaultParam() { + return { + execute: async (input = this.value) => { + 'use step'; + return input + 1; + }, + }; + } + + // `this` appears only inside a class body declared *inside* the arrow. + // The class field initializer's `this` is the new instance, NOT the + // outer arrow's lexical `this`. Detector should NOT flag this. + withClassBodyOnly() { + return { + execute: async () => { + 'use step'; + class Inner { + self = this; + getThis() { + return this; + } + } + return new Inner(); + }, + }; + } +} diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/output-client.js b/packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/output-client.js new file mode 100644 index 0000000000..932204b98b --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/output-client.js @@ -0,0 +1,60 @@ +// Edge cases for the lexical-`this` detector that drives `.bind(this)` and +// the arrow→function hoisting choice. +// +// - default parameter initializers see lexical `this` ⇒ should bind +// - destructuring parameter defaults see lexical `this` ⇒ should bind +// - class field initializers / methods inside the arrow body bind their own +// `this` (the class instance), so they should NOT trigger the detector +// - `extends` clauses and computed property keys inside such a class are +// evaluated in the outer scope, so `this` references there SHOULD trigger +// the detector. +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; +/**__internal_workflows{"steps":{"input.js":{"_anonymousStep0":{"stepId":"step//./input//_anonymousStep0"},"_anonymousStep1":{"stepId":"step//./input//_anonymousStep1"}}},"classes":{"input.js":{"Edge":{"classId":"class//./input//Edge"}}}}*/; +export class Edge { + static [WORKFLOW_SERIALIZE](inst) { + return { + value: inst.value + }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new Edge(data.value); + } + constructor(value){ + this.value = value; + } + // `this` appears only in a default parameter (no body reference). + // Detector should still flag this and emit `.bind(this)`. + withThisInDefaultParam() { + return { + execute: async (input = this.value)=>{ + return input + 1; + } + }; + } + // `this` appears only inside a class body declared *inside* the arrow. + // The class field initializer's `this` is the new instance, NOT the + // outer arrow's lexical `this`. Detector should NOT flag this. + withClassBodyOnly() { + return { + execute: async ()=>{ + class Inner { + self = this; + getThis() { + return this; + } + } + return new Inner(); + } + }; + } +} +(function(__wf_cls, __wf_id) { + var __wf_sym = Symbol.for("workflow-class-registry"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_cls); + Object.defineProperty(__wf_cls, "classId", { + value: __wf_id, + writable: false, + enumerable: false, + configurable: false + }); +})(Edge, "class//./input//Edge"); diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/output-step.js b/packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/output-step.js new file mode 100644 index 0000000000..9261f3824c --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/output-step.js @@ -0,0 +1,75 @@ +import { registerStepFunction } from "workflow/internal/private"; +// Edge cases for the lexical-`this` detector that drives `.bind(this)` and +// the arrow→function hoisting choice. +// +// - default parameter initializers see lexical `this` ⇒ should bind +// - destructuring parameter defaults see lexical `this` ⇒ should bind +// - class field initializers / methods inside the arrow body bind their own +// `this` (the class instance), so they should NOT trigger the detector +// - `extends` clauses and computed property keys inside such a class are +// evaluated in the outer scope, so `this` references there SHOULD trigger +// the detector. +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; +/**__internal_workflows{"steps":{"input.js":{"_anonymousStep0":{"stepId":"step//./input//_anonymousStep0"},"_anonymousStep1":{"stepId":"step//./input//_anonymousStep1"}}},"classes":{"input.js":{"Edge":{"classId":"class//./input//Edge"}}}}*/; +async function _anonymousStep0(input = this.value) { + return input + 1; +} +var _anonymousStep1 = async ()=>{ + class Inner { + self = this; + getThis() { + return this; + } + } + return new Inner(); +}; +export class Edge { + static [WORKFLOW_SERIALIZE](inst) { + return { + value: inst.value + }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new Edge(data.value); + } + constructor(value){ + this.value = value; + } + // `this` appears only in a default parameter (no body reference). + // Detector should still flag this and emit `.bind(this)`. + withThisInDefaultParam() { + return { + execute: async (input = this.value)=>{ + return input + 1; + } + }; + } + // `this` appears only inside a class body declared *inside* the arrow. + // The class field initializer's `this` is the new instance, NOT the + // outer arrow's lexical `this`. Detector should NOT flag this. + withClassBodyOnly() { + return { + execute: async ()=>{ + class Inner { + self = this; + getThis() { + return this; + } + } + return new Inner(); + } + }; + } +} +registerStepFunction("step//./input//_anonymousStep0", _anonymousStep0); +registerStepFunction("step//./input//_anonymousStep1", _anonymousStep1); +(function(__wf_cls, __wf_id) { + var __wf_sym = Symbol.for("workflow-class-registry"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_cls); + Object.defineProperty(__wf_cls, "classId", { + value: __wf_id, + writable: false, + enumerable: false, + configurable: false + }); +})(Edge, "class//./input//Edge"); diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/output-workflow.js b/packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/output-workflow.js new file mode 100644 index 0000000000..3d5b438b12 --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/output-workflow.js @@ -0,0 +1,50 @@ +// Edge cases for the lexical-`this` detector that drives `.bind(this)` and +// the arrow→function hoisting choice. +// +// - default parameter initializers see lexical `this` ⇒ should bind +// - destructuring parameter defaults see lexical `this` ⇒ should bind +// - class field initializers / methods inside the arrow body bind their own +// `this` (the class instance), so they should NOT trigger the detector +// - `extends` clauses and computed property keys inside such a class are +// evaluated in the outer scope, so `this` references there SHOULD trigger +// the detector. +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; +/**__internal_workflows{"steps":{"input.js":{"_anonymousStep0":{"stepId":"step//./input//_anonymousStep0"},"_anonymousStep1":{"stepId":"step//./input//_anonymousStep1"}}},"classes":{"input.js":{"Edge":{"classId":"class//./input//Edge"}}}}*/; +export class Edge { + static [WORKFLOW_SERIALIZE](inst) { + return { + value: inst.value + }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new Edge(data.value); + } + constructor(value){ + this.value = value; + } + // `this` appears only in a default parameter (no body reference). + // Detector should still flag this and emit `.bind(this)`. + withThisInDefaultParam() { + return { + execute: globalThis[Symbol.for("WORKFLOW_USE_STEP")]("step//./input//_anonymousStep0").bind(this) + }; + } + // `this` appears only inside a class body declared *inside* the arrow. + // The class field initializer's `this` is the new instance, NOT the + // outer arrow's lexical `this`. Detector should NOT flag this. + withClassBodyOnly() { + return { + execute: globalThis[Symbol.for("WORKFLOW_USE_STEP")]("step//./input//_anonymousStep1") + }; + } +} +(function(__wf_cls, __wf_id) { + var __wf_sym = Symbol.for("workflow-class-registry"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_cls); + Object.defineProperty(__wf_cls, "classId", { + value: __wf_id, + writable: false, + enumerable: false, + configurable: false + }); +})(Edge, "class//./input//Edge"); diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/input.js b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/input.js new file mode 100644 index 0000000000..4a251616ea --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/input.js @@ -0,0 +1,29 @@ +// A nested arrow-function step assigned to a `const` inside a method. The +// arrow body references the enclosing method's `this`, so the compiler should +// hoist as a regular function (step mode) and `.bind(this)` the proxy +// (workflow mode). +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; + +export class Counter { + static [WORKFLOW_SERIALIZE](instance) { + return { value: instance.value }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new Counter(data.value); + } + constructor(value) { + this.value = value; + } + + // The nested step is assigned to a `const`, exercising the var-declarator + // code path (separate from the in-expression arrow path used for object + // literals). + async run(amount) { + const addToValue = async (delta) => { + 'use step'; + return this.value + delta; + }; + + return addToValue(amount); + } +} diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/output-client.js b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/output-client.js new file mode 100644 index 0000000000..0d50c388aa --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/output-client.js @@ -0,0 +1,38 @@ +// A nested arrow-function step assigned to a `const` inside a method. The +// arrow body references the enclosing method's `this`, so the compiler should +// hoist as a regular function (step mode) and `.bind(this)` the proxy +// (workflow mode). +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; +/**__internal_workflows{"steps":{"input.js":{"addToValue":{"stepId":"step//./input//addToValue"}}},"classes":{"input.js":{"Counter":{"classId":"class//./input//Counter"}}}}*/; +export class Counter { + static [WORKFLOW_SERIALIZE](instance) { + return { + value: instance.value + }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new Counter(data.value); + } + constructor(value){ + this.value = value; + } + // The nested step is assigned to a `const`, exercising the var-declarator + // code path (separate from the in-expression arrow path used for object + // literals). + async run(amount) { + const addToValue = async (delta)=>{ + return this.value + delta; + }; + return addToValue(amount); + } +} +(function(__wf_cls, __wf_id) { + var __wf_sym = Symbol.for("workflow-class-registry"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_cls); + Object.defineProperty(__wf_cls, "classId", { + value: __wf_id, + writable: false, + enumerable: false, + configurable: false + }); +})(Counter, "class//./input//Counter"); diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/output-step.js b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/output-step.js new file mode 100644 index 0000000000..082bf06792 --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/output-step.js @@ -0,0 +1,43 @@ +import { registerStepFunction } from "workflow/internal/private"; +// A nested arrow-function step assigned to a `const` inside a method. The +// arrow body references the enclosing method's `this`, so the compiler should +// hoist as a regular function (step mode) and `.bind(this)` the proxy +// (workflow mode). +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; +/**__internal_workflows{"steps":{"input.js":{"addToValue":{"stepId":"step//./input//addToValue"}}},"classes":{"input.js":{"Counter":{"classId":"class//./input//Counter"}}}}*/; +async function addToValue(delta) { + return this.value + delta; +} +export class Counter { + static [WORKFLOW_SERIALIZE](instance) { + return { + value: instance.value + }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new Counter(data.value); + } + constructor(value){ + this.value = value; + } + // The nested step is assigned to a `const`, exercising the var-declarator + // code path (separate from the in-expression arrow path used for object + // literals). + async run(amount) { + const addToValue = async (delta)=>{ + return this.value + delta; + }; + return addToValue(amount); + } +} +registerStepFunction("step//./input//addToValue", addToValue); +(function(__wf_cls, __wf_id) { + var __wf_sym = Symbol.for("workflow-class-registry"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_cls); + Object.defineProperty(__wf_cls, "classId", { + value: __wf_id, + writable: false, + enumerable: false, + configurable: false + }); +})(Counter, "class//./input//Counter"); diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/output-workflow.js b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/output-workflow.js new file mode 100644 index 0000000000..3b9f85ca9a --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/output-workflow.js @@ -0,0 +1,36 @@ +// A nested arrow-function step assigned to a `const` inside a method. The +// arrow body references the enclosing method's `this`, so the compiler should +// hoist as a regular function (step mode) and `.bind(this)` the proxy +// (workflow mode). +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; +/**__internal_workflows{"steps":{"input.js":{"addToValue":{"stepId":"step//./input//addToValue"}}},"classes":{"input.js":{"Counter":{"classId":"class//./input//Counter"}}}}*/; +export class Counter { + static [WORKFLOW_SERIALIZE](instance) { + return { + value: instance.value + }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new Counter(data.value); + } + constructor(value){ + this.value = value; + } + // The nested step is assigned to a `const`, exercising the var-declarator + // code path (separate from the in-expression arrow path used for object + // literals). + async run(amount) { + const addToValue = globalThis[Symbol.for("WORKFLOW_USE_STEP")]("step//./input//addToValue").bind(this); + return addToValue(amount); + } +} +(function(__wf_cls, __wf_id) { + var __wf_sym = Symbol.for("workflow-class-registry"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_cls); + Object.defineProperty(__wf_cls, "classId", { + value: __wf_id, + writable: false, + enumerable: false, + configurable: false + }); +})(Counter, "class//./input//Counter"); diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/input.js b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/input.js new file mode 100644 index 0000000000..10a4d6be34 --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/input.js @@ -0,0 +1,31 @@ +// A nested arrow-function step that references `this` from the enclosing +// method. The class is serializable, so the workflow runtime can carry the +// captured `this` across the workflow→step boundary. +// +// The compiler should: +// - Workflow mode: emit `...WORKFLOW_USE_STEP(...).bind(this)` so the step +// proxy captures the caller's `this` and forwards it as `thisVal`. +// - Step mode: hoist the step body as a regular `function` (not an arrow) +// so the runtime's `stepFn.apply(thisVal, args)` rebinds `this` inside the +// hoisted body. +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; + +export class ReadFileTool { + static [WORKFLOW_SERIALIZE](instance) { + return { service: instance.service }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new ReadFileTool(data.service); + } + constructor(service) { + this.service = service; + } + createTool(context) { + return tool({ + execute: async (input) => { + 'use step'; + return this.service.readFileContent(input, context); + }, + }); + } +} diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/output-client.js b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/output-client.js new file mode 100644 index 0000000000..9ea8ffac78 --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/output-client.js @@ -0,0 +1,42 @@ +// A nested arrow-function step that references `this` from the enclosing +// method. The class is serializable, so the workflow runtime can carry the +// captured `this` across the workflow→step boundary. +// +// The compiler should: +// - Workflow mode: emit `...WORKFLOW_USE_STEP(...).bind(this)` so the step +// proxy captures the caller's `this` and forwards it as `thisVal`. +// - Step mode: hoist the step body as a regular `function` (not an arrow) +// so the runtime's `stepFn.apply(thisVal, args)` rebinds `this` inside the +// hoisted body. +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; +/**__internal_workflows{"steps":{"input.js":{"_anonymousStep0":{"stepId":"step//./input//_anonymousStep0"}}},"classes":{"input.js":{"ReadFileTool":{"classId":"class//./input//ReadFileTool"}}}}*/; +export class ReadFileTool { + static [WORKFLOW_SERIALIZE](instance) { + return { + service: instance.service + }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new ReadFileTool(data.service); + } + constructor(service){ + this.service = service; + } + createTool(context) { + return tool({ + execute: async (input)=>{ + return this.service.readFileContent(input, context); + } + }); + } +} +(function(__wf_cls, __wf_id) { + var __wf_sym = Symbol.for("workflow-class-registry"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_cls); + Object.defineProperty(__wf_cls, "classId", { + value: __wf_id, + writable: false, + enumerable: false, + configurable: false + }); +})(ReadFileTool, "class//./input//ReadFileTool"); diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/output-step.js b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/output-step.js new file mode 100644 index 0000000000..b754d975bd --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/output-step.js @@ -0,0 +1,48 @@ +import { __private_getClosureVars, registerStepFunction } from "workflow/internal/private"; +// A nested arrow-function step that references `this` from the enclosing +// method. The class is serializable, so the workflow runtime can carry the +// captured `this` across the workflow→step boundary. +// +// The compiler should: +// - Workflow mode: emit `...WORKFLOW_USE_STEP(...).bind(this)` so the step +// proxy captures the caller's `this` and forwards it as `thisVal`. +// - Step mode: hoist the step body as a regular `function` (not an arrow) +// so the runtime's `stepFn.apply(thisVal, args)` rebinds `this` inside the +// hoisted body. +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; +/**__internal_workflows{"steps":{"input.js":{"_anonymousStep0":{"stepId":"step//./input//_anonymousStep0"}}},"classes":{"input.js":{"ReadFileTool":{"classId":"class//./input//ReadFileTool"}}}}*/; +async function _anonymousStep0(input) { + const { context } = __private_getClosureVars(); + return this.service.readFileContent(input, context); +} +export class ReadFileTool { + static [WORKFLOW_SERIALIZE](instance) { + return { + service: instance.service + }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new ReadFileTool(data.service); + } + constructor(service){ + this.service = service; + } + createTool(context) { + return tool({ + execute: async (input)=>{ + return this.service.readFileContent(input, context); + } + }); + } +} +registerStepFunction("step//./input//_anonymousStep0", _anonymousStep0); +(function(__wf_cls, __wf_id) { + var __wf_sym = Symbol.for("workflow-class-registry"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_cls); + Object.defineProperty(__wf_cls, "classId", { + value: __wf_id, + writable: false, + enumerable: false, + configurable: false + }); +})(ReadFileTool, "class//./input//ReadFileTool"); diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/output-workflow.js b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/output-workflow.js new file mode 100644 index 0000000000..79c6a4abe9 --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/output-workflow.js @@ -0,0 +1,42 @@ +// A nested arrow-function step that references `this` from the enclosing +// method. The class is serializable, so the workflow runtime can carry the +// captured `this` across the workflow→step boundary. +// +// The compiler should: +// - Workflow mode: emit `...WORKFLOW_USE_STEP(...).bind(this)` so the step +// proxy captures the caller's `this` and forwards it as `thisVal`. +// - Step mode: hoist the step body as a regular `function` (not an arrow) +// so the runtime's `stepFn.apply(thisVal, args)` rebinds `this` inside the +// hoisted body. +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; +/**__internal_workflows{"steps":{"input.js":{"_anonymousStep0":{"stepId":"step//./input//_anonymousStep0"}}},"classes":{"input.js":{"ReadFileTool":{"classId":"class//./input//ReadFileTool"}}}}*/; +export class ReadFileTool { + static [WORKFLOW_SERIALIZE](instance) { + return { + service: instance.service + }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new ReadFileTool(data.service); + } + constructor(service){ + this.service = service; + } + createTool(context) { + return tool({ + execute: globalThis[Symbol.for("WORKFLOW_USE_STEP")]("step//./input//_anonymousStep0", ()=>({ + context + })).bind(this) + }); + } +} +(function(__wf_cls, __wf_id) { + var __wf_sym = Symbol.for("workflow-class-registry"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_cls); + Object.defineProperty(__wf_cls, "classId", { + value: __wf_id, + writable: false, + enumerable: false, + configurable: false + }); +})(ReadFileTool, "class//./input//ReadFileTool"); diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/input.js b/packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/input.js new file mode 100644 index 0000000000..5832e5d19b --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/input.js @@ -0,0 +1,16 @@ +// `arguments` inside a nested `function`-form step body must NOT be treated +// as a closure variable — it's a per-function intrinsic binding that +// reflects the positional args the runtime passes via `stepFn.apply(thisVal, +// args)`. Without the `is_global_identifier("arguments")` exclusion, the +// hoisted body would gain `const { arguments } = ...` (a syntax error in +// strict mode) and the original `arguments[0]` access would silently break. +export async function outer() { + 'use workflow'; + + async function step() { + 'use step'; + return arguments[0]; + } + + return step('hello'); +} diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/output-client.js b/packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/output-client.js new file mode 100644 index 0000000000..e9a762442c --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/output-client.js @@ -0,0 +1,11 @@ +/**__internal_workflows{"workflows":{"input.js":{"outer":{"workflowId":"workflow//./input//outer"}}}}*/; +// `arguments` inside a nested `function`-form step body must NOT be treated +// as a closure variable — it's a per-function intrinsic binding that +// reflects the positional args the runtime passes via `stepFn.apply(thisVal, +// args)`. Without the `is_global_identifier("arguments")` exclusion, the +// hoisted body would gain `const { arguments } = ...` (a syntax error in +// strict mode) and the original `arguments[0]` access would silently break. +export async function outer() { + throw new Error("You attempted to execute workflow outer function directly. To start a workflow, use start(outer) from workflow/api"); +} +outer.workflowId = "workflow//./input//outer"; diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/output-step.js b/packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/output-step.js new file mode 100644 index 0000000000..f9e186c0f5 --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/output-step.js @@ -0,0 +1,16 @@ +import { registerStepFunction } from "workflow/internal/private"; +/**__internal_workflows{"workflows":{"input.js":{"outer":{"workflowId":"workflow//./input//outer"}}},"steps":{"input.js":{"step":{"stepId":"step//./input//step"}}}}*/; +async function outer$step() { + return arguments[0]; +} +// `arguments` inside a nested `function`-form step body must NOT be treated +// as a closure variable — it's a per-function intrinsic binding that +// reflects the positional args the runtime passes via `stepFn.apply(thisVal, +// args)`. Without the `is_global_identifier("arguments")` exclusion, the +// hoisted body would gain `const { arguments } = ...` (a syntax error in +// strict mode) and the original `arguments[0]` access would silently break. +export async function outer() { + throw new Error("You attempted to execute workflow outer function directly. To start a workflow, use start(outer) from workflow/api"); +} +outer.workflowId = "workflow//./input//outer"; +registerStepFunction("step//./input//outer/step", outer$step); diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/output-workflow.js b/packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/output-workflow.js new file mode 100644 index 0000000000..dda9655df2 --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/output-workflow.js @@ -0,0 +1,13 @@ +/**__internal_workflows{"workflows":{"input.js":{"outer":{"workflowId":"workflow//./input//outer"}}},"steps":{"input.js":{"step":{"stepId":"step//./input//step"}}}}*/; +// `arguments` inside a nested `function`-form step body must NOT be treated +// as a closure variable — it's a per-function intrinsic binding that +// reflects the positional args the runtime passes via `stepFn.apply(thisVal, +// args)`. Without the `is_global_identifier("arguments")` exclusion, the +// hoisted body would gain `const { arguments } = ...` (a syntax error in +// strict mode) and the original `arguments[0]` access would silently break. +export async function outer() { + var step = globalThis[Symbol.for("WORKFLOW_USE_STEP")]("step//./input//outer/step"); + return step('hello'); +} +outer.workflowId = "workflow//./input//outer"; +globalThis.__private_workflows.set("workflow//./input//outer", outer); diff --git a/workbench/example/workflows/99_e2e.ts b/workbench/example/workflows/99_e2e.ts index 2544e9b7e0..92df6f70c4 100644 --- a/workbench/example/workflows/99_e2e.ts +++ b/workbench/example/workflows/99_e2e.ts @@ -1296,6 +1296,45 @@ export class Counter { 'use step'; return { label, value: this.value }; } + + /** + * Returns a "tool"-shaped object whose `add` property is a nested arrow + * step that lexically captures `this`. + * + * This exercises the SWC plugin's lexical-`this` capture for nested arrow + * step functions: in workflow mode the proxy is wrapped with + * `.bind(this)` (so the queue item carries `thisVal`), and in step mode + * the body is hoisted as a regular `function` (so `stepFn.apply(thisVal, + * args)` can rebind). The captured `delta` exercises the closure-vars + * path alongside it. + * + * This is structurally identical to the AI SDK `tool({ execute: async + * (input) => { 'use step'; return this.x; } })` pattern from the + * upstream issue (vercel/workflow#1865). + */ + makeAdder(delta: number): { + add: (amount: number) => Promise; + } { + return { + add: async (amount: number) => { + 'use step'; + return this.value + amount + delta; + }, + }; + } +} + +/** + * Step that takes a step-function reference and invokes it. Used by + * `instanceMethodStepWorkflow` to exercise the workflow→step→step + * round-trip serialization of a `bind(this)`-wrapped step proxy. + */ +async function invokeAdderFromStep( + add: (amount: number) => Promise, + amount: number +): Promise { + 'use step'; + return add(amount); } /** @@ -1319,12 +1358,32 @@ export async function instanceMethodStepWorkflow(initialValue: number) { const counter2 = new Counter(100); const added2 = await counter2.add(50); + // Lexical-`this` capture in a nested arrow step: + // + // `counter.makeAdder(7).add` is a step proxy that the SWC plugin wraps + // with `.bind(this)` in workflow mode. Invoking it directly should + // capture the Counter instance as `thisVal` so the step body sees + // `this.value` correctly. + const adder = counter.makeAdder(7); + const adderResult = await adder.add(2); // initialValue + 2 + 7 + + // Round-trip the bound step proxy through another step boundary: + // + // Passing `adder.add` as a step argument forces the + // `getStepFunctionReducer` to capture the bound `this` (`__boundThis`) + // alongside the `stepId`, and the step bundle's reviver must re-bind + // the freshly created proxy so the inner step still sees the original + // Counter instance via `this`. + const adderViaStep = await invokeAdderFromStep(adder.add, 3); // initialValue + 3 + 7 + return { initialValue, added, // initialValue + 10 multiplied, // initialValue * 3 description, // { label: 'test counter', value: initialValue } added2, // 100 + 50 = 150 + adderResult, // initialValue + 2 + 7 + adderViaStep, // initialValue + 3 + 7 }; }