Fix leaking graphs when classes are injected by lifecycle bound graphs

When a React construct component is injected by a lifecycle bound graph, the graph is retained in memory until the React construct is unmounted.

Injecting regular classes also contributed to the retention count which caused a bug as graphs are released on unmount.

packages/react-obsidian/src/graph/registry/GraphRegistry.ts

@@ -32,13 +32,13 @@ export class GraphRegistry {
 
   resolve<T extends Graph>(
     Graph: Constructable<T>,
-    source: 'lifecycleOwner' | 'serviceLocator' = 'lifecycleOwner',
+    source: 'lifecycleOwner' | 'classInjection' | 'serviceLocator' = 'lifecycleOwner',
     props: any = undefined,
   ): T {
     if ((this.isSingleton(Graph) || this.isBoundToReactLifecycle(Graph)) && this.has(Graph)) {
       return this.getFirst(Graph);
     }
-    if (this.isBoundToReactLifecycle(Graph) && source === 'serviceLocator') {
+    if (this.isBoundToReactLifecycle(Graph) && source !== 'lifecycleOwner') {
       throw new ObtainLifecycleBoundGraphException(Graph);
     }
     const graph = this.graphMiddlewares.resolve(Graph, props);

packages/react-obsidian/src/injectors/class/ClassInjector.ts

@@ -24,8 +24,12 @@ export default class ClassInjector {
   ): ProxyHandler<any> {
     return new class Handler implements ProxyHandler<any> {
       construct(target: any, args: any[], newTarget: Function): any {
-        const graph = graphRegistry.resolve(Graph, 'lifecycleOwner', args.length > 0 ? args[0] : undefined);
-        referenceCounter.retain(graph);
+        const isReactClassComponent = target.prototype?.isReactComponent;
+        const source = isReactClassComponent ? 'lifecycleOwner' : 'classInjection';
+        const graph = graphRegistry.resolve(Graph, source, args.length > 0 ? args[0] : undefined);
+        if (isReactClassComponent) {
+          referenceCounter.retain(graph);
+        }
         Reflect.defineMetadata(GRAPH_INSTANCE_NAME_KEY, graph.name, target);
         const argsToInject = this.injectConstructorArgs(args, graph, target);
         graph.onBind(target);

packages/react-obsidian/test/integration/lifecyleBoundGraphs.test.tsx

@@ -8,6 +8,7 @@ import {
   injectHook,
 } from '../../src';
 import { LifecycleBoundGraph } from '../fixtures/LifecycleBoundGraph';
+import { ObtainLifecycleBoundGraphException } from '../../src/graph/registry/ObtainLifecycleBoundGraphException';
 
 describe('React lifecycle bound graphs', () => {
   const Component = createFunctionalComponent();
@@ -30,18 +31,38 @@ describe('React lifecycle bound graphs', () => {
     expect(LifecycleBoundGraph.timesCreated).toBe(2);
   });
 
+  it('clears a bound graph after dependent components are unmounted when it was used for class injection', () => {
+    const Component2 = createFunctionalComponent({ instantiateInjectableClass: true});
+    const { unmount } = render(<Component2 />);
+    unmount();
+    render(<Component2 />);
+
+    expect(LifecycleBoundGraph.timesCreated).toBe(2);
+  });
+
   it('passes props to the component', () => {
     const { container } = render(<ClassComponent stringFromProps='Obsidian is cool' />);
     expect(container.textContent).toBe('A string passed via props: Obsidian is cool');
   });
 
   it('obtains a lifecycle bound graph only if it was already created', () => {
-    expect(() => Obsidian.obtain(LifecycleBoundGraph)).toThrowError(
+    expect(() => Obsidian.obtain(LifecycleBoundGraph)).toThrow(
       'Tried to obtain a @LifecycleBound graph LifecycleBoundGraph, but it was not created yet. '
       + '@LifecycleBound graphs can only be obtained after they were created by a React component or hook.',
     );
   });
 
+  it('throws when a lifecycle bound graph is used to inject a class before it was created', () => {
+    expect(() => {
+      @Injectable(LifecycleBoundGraph)
+      class Foo {
+        @Inject() private computedFromProps!: string;
+      }
+      // eslint-disable-next-line no-new
+      new Foo();
+    }).toThrow(ObtainLifecycleBoundGraphException);
+  });
+
   it(`resolves a dependency when @LifecycleBound graph is used as a service locator`, () => {
     render(<Component />);
 
@@ -64,8 +85,16 @@ describe('React lifecycle bound graphs', () => {
     expect(LifecycleBoundGraph.timesCreated).toBe(2);
   });
 
-  function createFunctionalComponent() {
-    const useHook = injectHook(() => {}, LifecycleBoundGraph);
+  type CreateOptions = {instantiateInjectableClass: boolean};
+  function createFunctionalComponent({instantiateInjectableClass}: CreateOptions = {
+    instantiateInjectableClass: false,
+  }) {
+    const useHook = injectHook(() => {
+      if (instantiateInjectableClass) {
+        // eslint-disable-next-line no-new
+        new Foo();
+      }
+    }, LifecycleBoundGraph);
 
     return injectComponent(() => {
       useHook();
@@ -81,4 +110,13 @@ describe('React lifecycle bound graphs', () => {
       return <>{this.computedFromProps}</>;
     }
   }
+
+  @Injectable(LifecycleBoundGraph)
+  class Foo {
+    @Inject() private computedFromProps!: string;
+
+    log() {
+      console.log(this.computedFromProps);
+    }
+  }
 });