Skip to content

Add circular dependency detection to all() function #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pavi2410 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add circular dependency detection to all() function #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
174 changes: 174 additions & 0 deletions lib/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
89 changes: 77 additions & 12 deletions lib/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,77 @@ export function all<T extends Record<string, any>>(
>()
const returnValue: Record<string, any> = {}

const waitForDep = (depName: keyof T): Promise<any> => {
// Track which task is waiting for which (for cycle detection)
const waitingFor = new Map<keyof T, Set<keyof T>>()

// 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<keyof T>()
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<keyof T>()
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<any> => {
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))
}
Expand Down Expand Up @@ -88,16 +155,6 @@ export function all<T extends Record<string, any>>(
}
}

// Create dep proxy
const depProxy = new Proxy({} as DepProxy<T>, {
get(_, depName: string) {
return waitForDep(depName as keyof T)
},
})

// Create context with $ proxy
const context: TaskContext<T> = { $: depProxy }

// Run all tasks in parallel
const promises = taskNames.map(async (name) => {
try {
Expand All @@ -106,7 +163,15 @@ export function all<T extends Record<string, any>>(
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<T>, {
get(_, depName: string) {
return waitForDep(name, depName as keyof T)
},
})
const taskContext: TaskContext<T> = { $: taskDepProxy }

const result = await taskFn.call(taskContext)
handleResult(name, result)
} catch (err) {
handleError(name, err)
Expand Down