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 e10b0f3ab5..085b4f96cd 100644 --- a/packages/core/e2e/e2e.test.ts +++ b/packages/core/e2e/e2e.test.ts @@ -2112,6 +2112,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; @@ -2121,6 +2126,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 @@ -2134,9 +2141,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}` ); @@ -2151,6 +2164,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 4c86d66739..d8ac2b4a13 100644 --- a/packages/core/src/serialization.test.ts +++ b/packages/core/src/serialization.test.ts @@ -2299,6 +2299,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 1ec66d0cb7..ec67ef5140 100644 --- a/packages/core/src/serialization.ts +++ b/packages/core/src/serialization.ts @@ -1001,10 +1001,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) { @@ -1016,47 +1035,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 WorkflowRuntimeError( '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; }, WorkflowFunction: (value) => diff --git a/packages/core/src/serialization/reducers/step-function.ts b/packages/core/src/serialization/reducers/step-function.ts index 8b2f521ed1..59a2e31ff0 100644 --- a/packages/core/src/serialization/reducers/step-function.ts +++ b/packages/core/src/serialization/reducers/step-function.ts @@ -4,10 +4,18 @@ * In workflow mode, step functions are replaced by the SWC plugin with * proxies created by `globalThis[Symbol.for("WORKFLOW_USE_STEP")]("stepId")`. * These proxies have a `.stepId` property and optionally a `.__closureVarsFn` - * for captured closure variables. + * for captured closure variables. They may additionally have `.__boundThis` + * (and rarely `.__boundArgs`) when the SWC plugin emitted + * `useStep(...).bind(this)` for a nested arrow step that lexically + * captured `this` (see `packages/swc-plugin-workflow/spec.md` → "Lexical + * `this` Capture in Nested Arrow Steps"). * - * The reducer serializes them as `{ stepId, closureVars? }`. - * The reviver reconstructs them by calling WORKFLOW_USE_STEP. + * The reducer serializes them as + * `{ stepId, closureVars?, boundThis?, boundArgs? }`. + * The reviver reconstructs them by calling WORKFLOW_USE_STEP and, when + * `boundThis` (or `boundArgs`) is present, re-binding the resulting + * proxy so the caller's captured `this` (and prefilled args) survive the + * round trip. */ import type { Reducers, Revivers } from '../types.js'; @@ -22,12 +30,35 @@ export function getStepFunctionReducer(): Partial { if (typeof stepId !== 'string') return false; const closureVarsFn = (value as any).__closureVarsFn; - if (closureVarsFn && typeof closureVarsFn === 'function') { - 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; } - return { stepId }; + return payload; }, }; } @@ -38,7 +69,13 @@ export function getStepFunctionReducer(): Partial { * Create the StepFunction reviver for workflow context. * * The reviver calls WORKFLOW_USE_STEP to create the step proxy, - * restoring the ability to call the step from workflow code. + * restoring the ability to call the step from workflow code. If the + * serialized payload includes `boundThis` (and optionally `boundArgs`), + * the reviver also re-binds 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 (e.g. when passed as a step + * argument). */ export function getStepFunctionReviver( global: Record = globalThis @@ -61,10 +98,15 @@ export function getStepFunctionReviver( ); } - if (closureVars) { - return useStep(stepId, () => closureVars); + const proxy = closureVars + ? useStep(stepId, () => closureVars) + : useStep(stepId); + + 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; }, }; } diff --git a/packages/core/src/serialization/serialization.test.ts b/packages/core/src/serialization/serialization.test.ts index ace5b0f8e3..c28af97b0d 100644 --- a/packages/core/src/serialization/serialization.test.ts +++ b/packages/core/src/serialization/serialization.test.ts @@ -755,6 +755,45 @@ describe('step function reducer', () => { it('should return false for functions without stepId', () => { expect(reducers.StepFunction!(() => {})).toBe(false); }); + + it('should include `boundThis` when the proxy has the marker property', () => { + // Simulates a step proxy that was created via `useStep(...).bind(thisArg)`. + // The actual `.bind` override on real proxies (in step.ts) attaches + // `__boundThis` so the reducer can serialize the captured `this`. + const instance = { kind: 'workflow-side' }; + const fn = Object.defineProperties(() => {}, { + stepId: { value: 'step//test//withBoundThis' }, + __boundThis: { value: instance }, + }); + const result = reducers.StepFunction!(fn); + expect(result).toEqual({ + stepId: 'step//test//withBoundThis', + boundThis: instance, + }); + }); + + it('should round-trip a `null`/`undefined` bound `this`', () => { + // `Function.prototype.bind(null)` and `bind(undefined)` are valid; the + // reducer must use property presence rather than truthiness so the + // distinction round-trips faithfully. + const fnNull = Object.defineProperties(() => {}, { + stepId: { value: 'step//test//boundNull' }, + __boundThis: { value: null }, + }); + expect(reducers.StepFunction!(fnNull)).toEqual({ + stepId: 'step//test//boundNull', + boundThis: null, + }); + + const fnUndef = Object.defineProperties(() => {}, { + stepId: { value: 'step//test//boundUndef' }, + __boundThis: { value: undefined }, + }); + expect(reducers.StepFunction!(fnUndef)).toEqual({ + stepId: 'step//test//boundUndef', + boundThis: undefined, + }); + }); }); describe('step function reviver', () => { @@ -798,6 +837,38 @@ describe('step function reviver', () => { revivers.StepFunction!({ stepId: 'step//test//myStep' }) ).toThrow(/WORKFLOW_USE_STEP not found/); }); + + it('should re-bind the proxy when `boundThis` is present in the payload', () => { + // Simulates the round trip for a step proxy that was created via + // `useStep(...).bind(thisArg)` in the workflow bundle and then passed + // as a step argument: the reducer captures `boundThis`, and the + // reviver in the step bundle must re-bind the freshly created proxy + // so the original `this` survives the serialization round trip. + const recordedThisVals: unknown[] = []; + const proxy = function (this: unknown) { + recordedThisVals.push(this); + return 'result'; + }; + const mockUseStep = vi.fn().mockReturnValue(proxy); + const global = { + [Symbol.for('WORKFLOW_USE_STEP')]: mockUseStep, + }; + + const instance = { restored: true }; + const revivers = getStepFunctionReviver(global); + const result = revivers.StepFunction!({ + stepId: 'step//test//withBoundThis', + boundThis: instance, + }) as () => unknown; + + expect(typeof result).toBe('function'); + expect(mockUseStep).toHaveBeenCalledWith('step//test//withBoundThis'); + + // Invoking the revived proxy without an explicit receiver should + // observe the bound `this`, not `undefined`. + result(); + expect(recordedThisVals).toEqual([instance]); + }); }); // ============================================================================ diff --git a/packages/core/src/serialization/types.ts b/packages/core/src/serialization/types.ts index 4c14f82bff..3ec14b07ef 100644 --- a/packages/core/src/serialization/types.ts +++ b/packages/core/src/serialization/types.ts @@ -107,6 +107,24 @@ export interface SerializableSpecial { StepFunction: { stepId: string; closureVars?: Record; + /** + * Captured lexical `this` for step proxies that were created via + * `useStep(...).bind(thisArg)` (the SWC plugin emits this for nested + * arrow steps that close over their enclosing function's `this`). + * The reviver re-binds the freshly-created proxy to this value so the + * binding survives serialization round-trips. + */ + boundThis?: unknown; + /** + * Prefilled arguments captured when the user (rather than the SWC + * plugin) called `useStep(...).bind(thisArg, x, y)`. The reviver + * re-applies these alongside `boundThis` so partial application + * survives serialization. The SWC plugin only ever emits + * `.bind(this)` with no extra args today; this slot exists so a + * hand-written `.bind(thisArg, x)` doesn't silently lose `x` after + * round-tripping through the reducer/reviver. + */ + boundArgs?: unknown[]; }; TypeError: { message: string; stack?: string; cause?: unknown }; URIError: { message: string; stack?: string; cause?: unknown }; diff --git a/packages/core/src/step.test.ts b/packages/core/src/step.test.ts index 3baea6f355..4152562b66 100644 --- a/packages/core/src/step.test.ts +++ b/packages/core/src/step.test.ts @@ -284,6 +284,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 5ee265beb6..9a03a2c0d9 100644 --- a/packages/core/src/step.ts +++ b/packages/core/src/step.ts @@ -221,6 +221,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 3237a746de..b4f71f6eab 100644 --- a/packages/swc-plugin-workflow/spec.md +++ b/packages/swc-plugin-workflow/spec.md @@ -1158,10 +1158,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 f299d1904a..8557e4750c 100644 --- a/packages/swc-plugin-workflow/transform/src/lib.rs +++ b/packages/swc-plugin-workflow/transform/src/lib.rs @@ -3,10 +3,10 @@ mod naming; use serde::Deserialize; use std::collections::{HashMap, HashSet}; use swc_core::{ - common::{errors::HANDLER, SyntaxContext, DUMMY_SP}, + common::{DUMMY_SP, SyntaxContext, errors::HANDLER}, ecma::{ ast::*, - visit::{noop_visit_mut_type, noop_visit_type, Visit, VisitMut, VisitMutWith, VisitWith}, + visit::{Visit, VisitMut, VisitMutWith, VisitWith, noop_visit_mut_type, noop_visit_type}, }, }; @@ -360,7 +360,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, @@ -368,6 +375,7 @@ pub struct StepTransform { Vec, bool, String, + bool, )>, // Counter for anonymous function names #[allow(dead_code)] @@ -1319,7 +1327,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" @@ -1453,6 +1469,97 @@ 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 { @@ -1498,6 +1605,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, @@ -3872,6 +3980,42 @@ impl StepTransform { }) } + // 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 { @@ -4852,6 +4996,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 @@ -4914,8 +5059,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 { @@ -6066,23 +6216,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 { @@ -6099,24 +6244,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; @@ -7308,9 +7435,9 @@ impl VisitMut for StepTransform { if let Some(body) = &mut fn_expr.function.body { let error_msg = format!( - "You attempted to execute workflow {} function directly. To start a workflow, use start({}) from workflow/api", - name, name - ); + "You attempted to execute workflow {} function directly. To start a workflow, use start({}) from workflow/api", + name, name + ); let error_expr = Expr::New(NewExpr { span: DUMMY_SP, ctxt: SyntaxContext::empty(), @@ -7657,6 +7784,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 @@ -7727,6 +7861,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, @@ -7759,10 +7894,13 @@ impl VisitMut for StepTransform { &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::Detect => {} } @@ -8611,6 +8749,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, @@ -8659,6 +8798,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 @@ -8719,6 +8863,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, @@ -8749,7 +8894,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::Detect => {} @@ -9211,6 +9360,13 @@ impl VisitMut for StepTransform { self.anonymous_fn_counter += 1; 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 @@ -9280,6 +9436,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 @@ -9309,9 +9466,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::Detect => {} @@ -9357,6 +9515,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 @@ -9442,6 +9601,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-step.js b/packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/output-step.js new file mode 100644 index 0000000000..228a2c793f --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/output-step.js @@ -0,0 +1,90 @@ +// 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; +} +(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; + Object.defineProperty(__wf_fn, "name", { + value: "_anonymousStep0", + configurable: true + }); +})(_anonymousStep0, "step//./input//_anonymousStep0"); +var _anonymousStep1 = async ()=>{ + class Inner { + self = this; + getThis() { + return this; + } + } + return new Inner(); +}; +(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; + Object.defineProperty(__wf_fn, "name", { + value: "_anonymousStep1", + configurable: true + }); +})(_anonymousStep1, "step//./input//_anonymousStep1"); +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-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-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..6db5d11ee9 --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/output-step.js @@ -0,0 +1,50 @@ +// 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; +} +(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; + Object.defineProperty(__wf_fn, "name", { + value: "addToValue", + configurable: true + }); +})(addToValue, "step//./input//addToValue"); +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-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-step.js b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/output-step.js new file mode 100644 index 0000000000..bbcafef2c1 --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/output-step.js @@ -0,0 +1,59 @@ +// 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 } = 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 || {}; + }(); + return this.service.readFileContent(input, context); +} +(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; + Object.defineProperty(__wf_fn, "name", { + value: "_anonymousStep0", + configurable: true + }); +})(_anonymousStep0, "step//./input//_anonymousStep0"); +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-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-step.js b/packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/output-step.js new file mode 100644 index 0000000000..d1759676ad --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/output-step.js @@ -0,0 +1,23 @@ +/**__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]; +} +(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; + Object.defineProperty(__wf_fn, "name", { + value: "outer$step", + configurable: true + }); +})(outer$step, "step//./input//outer/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() { + 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-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 54c46fe725..ad3826b826 100644 --- a/workbench/example/workflows/99_e2e.ts +++ b/workbench/example/workflows/99_e2e.ts @@ -1543,6 +1543,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); } /** @@ -1566,12 +1605,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 }; }