wzhudev / redi

💉 A dependency injection library for TypeScript & JavaScript, along with a binding for React.
https://redi.wendell.fun
MIT License
104 stars 17 forks source link

循环依赖问题的解决方案有 bug #21

Closed hehos closed 4 months ago

hehos commented 4 months ago

检查循环依赖的实现方案感觉有 bug ?因为如果 _resolveClass 方法实例化次数确实大于等于 MAX_RESOLUTIONS_QUEUED 次呢,当然这种情况实际上几乎可能不会有,但是就是不考虑这种情况,那么该实现方案在解决循环依赖问题的本质上也不应该是记录 _resolveClass 方法被调用的次数。通过认为正常情况 _resolveClass 方法被调用不可能大于 MAX_RESOLUTIONS_QUEUED 定义的 300 次来侧面判断循环依赖问题,我认为是有缺陷的,没有严谨的解决该问题。

wzhudev commented 4 months ago

你好 @hehos 请提供一个无法检测到循环依赖的复现

严谨的方法是引入 DAG 来判断是否成环,这样做的开发成本比较高,目前基于递归的方法已经能够比较好的解决问题;在我有空闲时间之前应该不会改成基于 DAG 的方案

hehos commented 4 months ago
it('detect circular dependency bug', () => {
      const j = new Injector()

      class Base {
        constructor(base: any) { }
      }

      let A = Base

      for (let i = 0; i < 400; i++) {
        const aI = createIdentifier('aI' + i)

        class B {
          constructor(@Inject(aI) private readonly a: any) { }
        }

        j.add([aI, { useClass: A }])

        A = B
      }

      j.add([A])

      expectToThrow(
        () => j.get(A),
        `[redi]: Detecting cyclic dependency. The last identifier is "B".`
      )
    })

上面的代码就能导致检查循环依赖时抛异常,但事实上这只是一个单向的超长依赖,并没有产生循环依赖。这个超长依赖只是为了证明检查循环依赖实现的bug,只是实际情况几乎不太可能出现这种超长依赖场景!

wzhudev commented 4 months ago

正是因为实际生产中不会有上百层的依赖关系,所以设计如此。