feat: Restore cycle detection

Cycle detection is no longer vulnerable for "stale context" from previous initialization
of eg. a singleton.

packages/injectable/core/src/dependency-injection-container/createContainer.js

@@ -22,6 +22,7 @@ export default containerId => {
   const instancesByInjectableMap = new Map();
   const injectablesByInjectionToken = new Map();
   const namespacedIdByInjectableMap = new Map();
+  const dependersMap = new Map();
 
   const getNamespacedId = getNamespacedIdFor(injectableAndRegistrationContext);
 
@@ -37,6 +38,7 @@ export default containerId => {
       containerRootContextItem,
       getRelatedInjectables,
       getInject: () => privateInject,
+      dependersMap,
     });
 
   const nonDecoratedPrivateInjectMany =
@@ -51,6 +53,8 @@ export default containerId => {
 
   const withInjectionDecorators = withInjectionDecoratorsFor({
     injectMany: nonDecoratedPrivateInjectMany,
+    dependersMap,
+    getNamespacedId,
   });
 
   const getSideEffectsArePrevented = injectable =>
@@ -68,6 +72,7 @@ export default containerId => {
     getSideEffectsArePrevented,
     getDi: () => privateDi,
     getNamespacedId,
+    dependersMap,
   });
 
   const decoratedPrivateInjectMany = withInjectionDecorators(
@@ -158,6 +163,7 @@ export default containerId => {
         customContextItem
           ? [containerRootContextItem, customContextItem]
           : [containerRootContextItem],
+        containerRootContextItem.injectable,
       ),
 
     injectMany: (alias, parameter, customContextItem) =>
@@ -167,10 +173,23 @@ export default containerId => {
         customContextItem
           ? [containerRootContextItem, customContextItem]
           : [containerRootContextItem],
+        containerRootContextItem.injectable,
       ),
 
     register: (...injectables) => {
-      privateDi.register({ injectables, context: [containerRootContextItem] });
+      privateDi.register({
+        injectables,
+        context: [containerRootContextItem],
+        source: containerRootContextItem.injectable,
+      });
+    },
+
+    deregister: (...injectables) => {
+      privateDi.deregister({
+        injectables,
+        context: [containerRootContextItem],
+        source: containerRootContextItem.injectable,
+      });
     },
 
     injectManyWithMeta: (alias, parameter, customContextItem) =>
@@ -180,6 +199,7 @@ export default containerId => {
         customContextItem
           ? [containerRootContextItem, customContextItem]
           : [containerRootContextItem],
+        containerRootContextItem.injectable,
       ),
   };
 

packages/injectable/core/src/dependency-injection-container/deregister.js

@@ -13,8 +13,13 @@ export const deregisterFor =
     // Todo: get rid of function usage.
     getDi,
   }) =>
-  (...injectables) => {
-    const callbacks = injectMany(deregistrationCallbackToken);
+  ({ injectables, context, source }) => {
+    const callbacks = injectMany(
+      deregistrationCallbackToken,
+      undefined,
+      context,
+      source,
+    );
 
     injectables.forEach(injectable => {
       callbacks.forEach(callback => {
@@ -65,7 +70,11 @@ export const deregisterSingleFor =
       .map(x => x[0])
       .forEach(injectable => {
         injectableAndRegistrationContext.delete(injectable);
-        di.deregister(injectable);
+        di.deregister({
+          injectables: [injectable],
+          context: [],
+          source: { id: 'lol' },
+        });
       });
 
     purgeInstances(injectable);

packages/injectable/core/src/dependency-injection-container/getCycleFor.js

@@ -0,0 +1,44 @@
+export const getCycleFor = dependencyMap => {
+  const visited = new Set();
+
+  const getCycle = (
+    reference,
+    rootReference = reference,
+    path = [reference],
+  ) => {
+    if (visited.has(reference)) {
+      return;
+    }
+
+    visited.add(reference);
+
+    if (rootReference.cannotCauseCycles) {
+      return;
+    }
+
+    const dependers = dependencyMap.get(reference);
+
+    // Todo: this should never be possible?
+    if (!dependers) {
+      return;
+    }
+
+    if (dependers.size === 0) {
+      return;
+    }
+
+    if (dependers.has(rootReference)) {
+      return [rootReference, ...path];
+    }
+
+    for (const depender of dependers) {
+      const cycle = getCycle(depender, rootReference, [depender, ...path]);
+
+      if (cycle) {
+        return cycle;
+      }
+    }
+  };
+
+  return reference => getCycle(reference);
+};

packages/injectable/core/src/dependency-injection-container/inject.js

@@ -15,8 +15,9 @@ export const privateInjectFor = ({
   getSideEffectsArePrevented,
   getDi,
   getNamespacedId,
+  dependersMap,
 }) =>
-  withInjectionDecoratorsFor({ injectMany })(
+  withInjectionDecoratorsFor({ injectMany, dependersMap, getNamespacedId })(
     (alias, instantiationParameter, context = []) => {
       const checkForNoMatches = checkForNoMatchesFor(getNamespacedId);
 
@@ -104,21 +105,37 @@ const getInstance = ({
   );
 
   const minimalDi = {
-    inject: (alias, parameter) => di.inject(alias, parameter, newContext),
+    inject: (alias, parameter) =>
+      di.inject(alias, parameter, newContext, injectableToBeInstantiated),
 
     injectMany: (alias, parameter) =>
-      di.injectMany(alias, parameter, newContext),
+      di.injectMany(alias, parameter, newContext, injectableToBeInstantiated),
 
     injectManyWithMeta: (alias, parameter) =>
-      di.injectManyWithMeta(alias, parameter, newContext),
+      di.injectManyWithMeta(
+        alias,
+        parameter,
+        newContext,
+        injectableToBeInstantiated,
+      ),
 
     context: newContext,
 
     register: (...injectables) => {
-      di.register({ injectables, context: newContext });
+      di.register({
+        injectables,
+        context: newContext,
+        source: injectableToBeInstantiated,
+      });
     },
 
-    deregister: di.deregister,
+    deregister: (...injectables) => {
+      di.deregister({
+        injectables,
+        context: newContext,
+        source: injectableToBeInstantiated,
+      });
+    },
   };
 
   const instanceKey = injectableToBeInstantiated.lifecycle.getInstanceKey(
@@ -168,6 +185,7 @@ const withInstantiationDecoratorsFor = ({ injectMany, injectable }) => {
         instantiationDecoratorToken,
         undefined,
         context,
+        injectable,
       )
         .filter(isRelevantDecorator)
         .map(x => x.decorate);

packages/injectable/core/src/dependency-injection-container/injectMany.js

@@ -1,21 +1,38 @@
 import { isPromise } from '@ogre-tools/fp';
 
 export const nonDecoratedPrivateInjectManyFor =
-  ({ containerRootContextItem, getRelatedInjectables, getInject }) =>
+  ({
+    containerRootContextItem,
+    getRelatedInjectables,
+    getInject,
+    dependersMap,
+  }) =>
   ({ withMeta }) =>
   (
     injectionToken,
     instantiationParameter,
     oldContext = [containerRootContextItem],
+    source,
   ) => {
+    if (!dependersMap.has(injectionToken)) {
+      dependersMap.set(injectionToken, new Set());
+    }
+
+    dependersMap.get(injectionToken).add(source);
+
     const inject = getInject();
 
     const newContext = [...oldContext, { injectable: injectionToken }];
 
     const relatedInjectables = getRelatedInjectables(injectionToken);
 
     const injected = relatedInjectables.map(injectable => {
-      const instance = inject(injectable, instantiationParameter, newContext);
+      const instance = inject(
+        injectable,
+        instantiationParameter,
+        newContext,
+        injectionToken,
+      );
 
       if (!withMeta) {
         return instance;

packages/injectable/core/src/dependency-injection-container/register.js

@@ -3,12 +3,17 @@ import { getNamespacedIdFor } from './getNamespacedIdFor';
 
 export const registerFor =
   ({ registerSingle, injectMany }) =>
-  ({ injectables, context }) => {
+  ({ injectables, context, source }) => {
     injectables.forEach(injectable => {
       registerSingle(injectable, context);
     });
 
-    const callbacks = injectMany(registrationCallbackToken);
+    const callbacks = injectMany(
+      registrationCallbackToken,
+      undefined,
+      context,
+      source,
+    );
 
     injectables.forEach(injectable => {
       callbacks.forEach(callback => {

packages/injectable/core/src/dependency-injection-container/withInjectionDecorators.js

@@ -1,16 +1,30 @@
 import { isRelevantDecoratorFor } from './isRelevantDecoratorFor';
 import flow from './fastFlow';
 import { injectionDecoratorToken } from './createContainer';
+import { getCycleFor } from './getCycleFor';
 
 export const withInjectionDecoratorsFor =
-  ({ injectMany }) =>
+  ({ injectMany, dependersMap, getNamespacedId }) =>
   toBeDecorated =>
-  (alias, ...args) => {
-    if (alias.decorable === false) {
-      return toBeDecorated(alias, ...args);
+  (alias, parameter, oldContext, injectingInjectable) => {
+    if (!dependersMap.has(alias)) {
+      dependersMap.set(alias, new Set());
     }
 
-    const [, oldContext] = args;
+    dependersMap.get(alias).add(injectingInjectable);
+
+    const cycle = getCycleFor(dependersMap)(alias);
+
+    if (cycle)
+      throw new Error(
+        `Cycle of injectables encountered: "${cycle
+          .map(getNamespacedId)
+          .join('" -> "')}"`,
+      );
+
+    if (alias.decorable === false) {
+      return toBeDecorated(alias, parameter, oldContext, injectingInjectable);
+    }
 
     const newContext = [...oldContext, { injectable: alias }];
 
@@ -20,11 +34,12 @@ export const withInjectionDecoratorsFor =
       injectionDecoratorToken,
       undefined,
       newContext,
+      injectingInjectable,
     )
       .filter(isRelevantDecorator)
       .map(x => x.decorate);
 
     const decorated = flow(...decorators)(toBeDecorated);
 
-    return decorated(alias, ...args);
+    return decorated(alias, parameter, oldContext, injectingInjectable);
   };

packages/injectable/core/src/scenarios/cycle-detection.test.js

@@ -1,7 +1,7 @@
 import createContainer from '../dependency-injection-container/createContainer';
 import getInjectable from '../getInjectable/getInjectable';
 
-xdescribe('cycle-detection', () => {
+describe('cycle-detection', () => {
   describe('given no cycle and injected', () => {
     let di;
     let goodInjectable;
@@ -51,21 +51,21 @@ xdescribe('cycle-detection', () => {
       expect(actual).toBe('some-good-result');
     });
 
-    it('given only bad work is done, throws context of bad work', () => {
+    it('given only bad work is done, throws error about only the cycle', () => {
       expect(() => {
         di.inject(badInjectable);
       }).toThrow(
-        'Cycle of injectables encountered: "some-container" -> "some-bad-injectable" -> "some-worker-injectable" -> "some-cyclical-injectable-1" -> "some-cyclical-injectable-2" -> "some-cyclical-injectable-1"',
+        'Cycle of injectables encountered: "some-cyclical-injectable-1" -> "some-cyclical-injectable-2" -> "some-cyclical-injectable-1"',
       );
     });
 
-    fit('given good work is done, but then bad work is done, throws context of bad work', () => {
+    it('given good work is done, but then bad work is done, throws error about only the cycle', () => {
       di.inject(goodInjectable);
 
       expect(() => {
         di.inject(badInjectable);
       }).toThrow(
-        'Cycle of injectables encountered: "some-container" -> "some-bad-injectable" -> "some-worker-injectable" -> "some-cyclical-injectable-1" -> "some-cyclical-injectable-2" -> "some-cyclical-injectable-1"',
+        'Cycle of injectables encountered: "some-cyclical-injectable-1" -> "some-cyclical-injectable-2" -> "some-cyclical-injectable-1"',
       );
     });
   });