diff --git a/lib/index.test.ts b/lib/index.test.ts index 2e7255c..f6ee9f9 100644 --- a/lib/index.test.ts +++ b/lib/index.test.ts @@ -298,6 +298,180 @@ describe('all', () => { }) }) + describe('Cycle detection', () => { + it('should detect self-referential cycle', async () => { + await expect( + all({ + async a() { + return await this.$.a + }, + }) + ).rejects.toThrow('Circular dependency detected') + }) + + it('should detect self-referential cycle with computation', async () => { + await expect( + all({ + async a() { + const aValue = await this.$.a + return aValue + 1 + }, + }) + ).rejects.toThrow('Circular dependency detected') + }) + + it('should detect simple two-task cycle (a ↔ b)', async () => { + await expect( + all({ + async a() { + return (await this.$.b) + 1 + }, + async b() { + return (await this.$.a) + 2 + }, + }) + ).rejects.toThrow('Circular dependency detected') + }) + + it('should detect three-task cycle (a → b → c → a)', async () => { + await expect( + all({ + async a() { + return (await this.$.b) + 1 + }, + async b() { + return (await this.$.c) + 2 + }, + async c() { + return (await this.$.a) + 3 + }, + }) + ).rejects.toThrow('Circular dependency detected') + }) + + it('should detect cycle with independent tasks present', async () => { + await expect( + all({ + x() { + return 100 + }, + y() { + return 200 + }, + async a() { + return (await this.$.b) + 1 + }, + async b() { + return (await this.$.a) + 2 + }, + }) + ).rejects.toThrow('Circular dependency detected') + }) + + it('should include cycle path in error message', async () => { + try { + await all({ + async a() { + return await this.$.b + }, + async b() { + return await this.$.c + }, + async c() { + return await this.$.a + }, + }) + expect.fail('Should have thrown an error') + } catch (err: any) { + expect(err.message).toMatch(/Circular dependency detected/) + // The error message should contain the tasks involved in the cycle + expect(err.message).toMatch(/a/) + expect(err.message).toMatch(/b/) + expect(err.message).toMatch(/c/) + } + }) + + it('should not throw for valid linear dependency chain', async () => { + const result = await all({ + a() { + return 1 + }, + async b() { + return (await this.$.a) + 1 + }, + async c() { + return (await this.$.b) + 1 + }, + }) + + expect(result).toEqual({ a: 1, b: 2, c: 3 }) + }) + + it('should not throw for diamond dependency pattern (no cycle)', async () => { + const result = await all({ + a() { + return 1 + }, + async b() { + return (await this.$.a) + 1 + }, + async c() { + return (await this.$.a) + 10 + }, + async d() { + return (await this.$.b) + (await this.$.c) + }, + }) + + expect(result).toEqual({ a: 1, b: 2, c: 11, d: 13 }) + }) + + it('should not throw for tasks accessing same dependency multiple times', async () => { + const result = await all({ + async base() { + return 5 + }, + async derived() { + const val = await this.$.base + return val + val + val + }, + }) + + expect(result).toEqual({ base: 5, derived: 15 }) + }) + + it('should handle cycle detection with multiple simultaneous cycles', async () => { + // If a → b → a AND c → d → c, it should detect at least one + await expect( + all({ + async a() { + return await this.$.b + }, + async b() { + return await this.$.a + }, + async c() { + return await this.$.d + }, + async d() { + return await this.$.c + }, + }) + ).rejects.toThrow('Circular dependency detected') + }) + + it('should detect cycle even with long async operation before dependency access', async () => { + await expect( + all({ + async a() { + await new Promise((resolve) => setTimeout(resolve, 10)) + return await this.$.a + }, + }) + ).rejects.toThrow('Circular dependency detected') + }) + }) + describe('Return values', () => { it('should return values of various types', async () => { const result = await all({ diff --git a/lib/index.ts b/lib/index.ts index 0f849be..7ddacb2 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -55,10 +55,77 @@ export function all>( >() const returnValue: Record = {} - const waitForDep = (depName: keyof T): Promise => { + // Track which task is waiting for which (for cycle detection) + const waitingFor = new Map>() + + // Check if there's a path from 'from' to 'to' in the waiting graph + const hasCyclePath = (from: keyof T, to: keyof T): boolean => { + const visited = new Set() + const stack: (keyof T)[] = [from] + while (stack.length > 0) { + const current = stack.pop()! + if (current === to) return true + if (visited.has(current)) continue + visited.add(current) + const deps = waitingFor.get(current) + if (deps) { + for (const dep of deps) { + stack.push(dep) + } + } + } + return false + } + + // Build cycle path for error message + const buildCyclePath = (from: keyof T, to: keyof T): string => { + const path: (keyof T)[] = [from] + const visited = new Set() + const findPath = (current: keyof T): boolean => { + if (current === to) return true + if (visited.has(current)) return false + visited.add(current) + const deps = waitingFor.get(current) + if (deps) { + for (const dep of deps) { + path.push(dep) + if (findPath(dep)) return true + path.pop() + } + } + return false + } + findPath(from) + return path.map(String).join(' → ') + } + + const waitForDep = (callerTask: keyof T, depName: keyof T): Promise => { if (!(depName in tasks)) { return Promise.reject(new Error(`Unknown task "${String(depName)}"`)) } + + // Self-reference check + if (callerTask === depName) { + return Promise.reject( + new Error(`Circular dependency detected: ${String(callerTask)} → ${String(depName)}`) + ) + } + + // Check if adding this dependency would create a cycle + // (i.e., if depName is already waiting for callerTask, directly or indirectly) + if (hasCyclePath(depName, callerTask)) { + const cyclePath = buildCyclePath(depName, callerTask) + return Promise.reject( + new Error(`Circular dependency detected: ${String(callerTask)} → ${cyclePath} → ${String(callerTask)}`) + ) + } + + // Record that callerTask is waiting for depName + if (!waitingFor.has(callerTask)) { + waitingFor.set(callerTask, new Set()) + } + waitingFor.get(callerTask)!.add(depName) + if (results.has(depName)) { return Promise.resolve(results.get(depName)) } @@ -88,16 +155,6 @@ export function all>( } } - // Create dep proxy - const depProxy = new Proxy({} as DepProxy, { - get(_, depName: string) { - return waitForDep(depName as keyof T) - }, - }) - - // Create context with $ proxy - const context: TaskContext = { $: depProxy } - // Run all tasks in parallel const promises = taskNames.map(async (name) => { try { @@ -106,7 +163,15 @@ export function all>( throw new Error(`Task "${String(name)}" is not a function`) } - const result = await taskFn.call(context) + // Create task-specific proxy that knows which task is calling + const taskDepProxy = new Proxy({} as DepProxy, { + get(_, depName: string) { + return waitForDep(name, depName as keyof T) + }, + }) + const taskContext: TaskContext = { $: taskDepProxy } + + const result = await taskFn.call(taskContext) handleResult(name, result) } catch (err) { handleError(name, err)