feat: allow to break cyclic dependency with optional()

restrict resolving `optional()` dependency in a constructor

packages/@wroud/di-tools-analyzer/__snapshots__/getDependenciesGraph.test.js.snap

@@ -1,6 +1,6 @@
 // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
 
-exports[`injectable > should make graph for async services implementations 1`] = `
+exports[`getDependenciesGraph > should make graph for async services implementations 1`] = `
 [
   {
     "id": "uuid-1",
@@ -71,7 +71,7 @@ exports[`injectable > should make graph for async services implementations 1`] =
 ]
 `;
 
-exports[`injectable > should make graph for async services implementations 2`] = `
+exports[`getDependenciesGraph > should make graph for async services implementations 2`] = `
 [
   {
     "name": "AppImpl (App) -> Logger",
@@ -160,7 +160,7 @@ exports[`injectable > should make graph for async services implementations 2`] =
 ]
 `;
 
-exports[`injectable > should make graph for sync services implementations 1`] = `
+exports[`getDependenciesGraph > should make graph for sync services implementations 1`] = `
 [
   {
     "id": "uuid-1",
@@ -231,7 +231,7 @@ exports[`injectable > should make graph for sync services implementations 1`] =
 ]
 `;
 
-exports[`injectable > should make graph for sync services implementations 2`] = `
+exports[`getDependenciesGraph > should make graph for sync services implementations 2`] = `
 [
   {
     "name": "App -> Logger",

packages/@wroud/di-tools-analyzer/src/getDependenciesGraph.test.ts

@@ -17,7 +17,7 @@ const mockImpl = vi.hoisted(() => {
 
 vi.mock(import("uuid"), () => mockImpl as any);
 
-describe.sequential("injectable", () => {
+describe("getDependenciesGraph", () => {
   beforeEach(() => {
     mockImpl.reset();
   });
@@ -26,35 +26,29 @@ describe.sequential("injectable", () => {
     expect(getDependenciesGraph).toBeDefined();
   });
 
-  it.sequential(
-    "should make graph for sync services implementations",
-    async () => {
-      const servicesCollection = mockSyncServicesCollection();
-      const graph = await getDependenciesGraph(servicesCollection);
-
-      expect(graph).toBeDefined();
-      expect(graph.nodes).toBeDefined();
-      expect(graph.links).toBeDefined();
-      expect(
-        graph.nodes.sort((a, b) => a.name.localeCompare(b.name)),
-      ).toMatchSnapshot();
-      expect(graph.links).toMatchSnapshot();
-    },
-  );
-
-  it.sequential(
-    "should make graph for async services implementations",
-    async () => {
-      const servicesCollection = mockAsyncServicesCollection();
-      const graph = await getDependenciesGraph(servicesCollection);
-
-      expect(graph).toBeDefined();
-      expect(graph.nodes).toBeDefined();
-      expect(graph.links).toBeDefined();
-      expect(
-        graph.nodes.sort((a, b) => a.name.localeCompare(b.name)),
-      ).toMatchSnapshot();
-      expect(graph.links).toMatchSnapshot();
-    },
-  );
+  it("should make graph for sync services implementations", async () => {
+    const servicesCollection = mockSyncServicesCollection();
+    const graph = await getDependenciesGraph(servicesCollection);
+
+    expect(graph).toBeDefined();
+    expect(graph.nodes).toBeDefined();
+    expect(graph.links).toBeDefined();
+    expect(
+      graph.nodes.sort((a, b) => a.name.localeCompare(b.name)),
+    ).toMatchSnapshot();
+    expect(graph.links).toMatchSnapshot();
+  });
+
+  it("should make graph for async services implementations", async () => {
+    const servicesCollection = mockAsyncServicesCollection();
+    const graph = await getDependenciesGraph(servicesCollection);
+
+    expect(graph).toBeDefined();
+    expect(graph.nodes).toBeDefined();
+    expect(graph.links).toBeDefined();
+    expect(
+      graph.nodes.sort((a, b) => a.name.localeCompare(b.name)),
+    ).toMatchSnapshot();
+    expect(graph.links).toMatchSnapshot();
+  });
 });

packages/@wroud/di-tools-analyzer/src/isOptionalDependency.test.ts

@@ -0,0 +1,18 @@
+import { describe, expect, it } from "vitest";
+import { isOptionalDependency } from "./isOptionalDependency.js";
+import { all, createService, optional, single } from "@wroud/di";
+
+describe("isOptionalDependency", () => {
+  it("should detect optional resolver", async () => {
+    const service = createService("Service");
+    expect(isOptionalDependency(optional(service))).toBe(true);
+    expect(isOptionalDependency(single(service))).toBe(false);
+    expect(isOptionalDependency(all(service))).toBe(false);
+  });
+  it("should detect optional resolver in combined resolver", async () => {
+    const service = createService("Service");
+
+    expect(isOptionalDependency(all(optional(service)))).toBe(true);
+    expect(isOptionalDependency(optional(all(service)))).toBe(true);
+  });
+});

packages/@wroud/di/src/debug.ts

@@ -8,6 +8,8 @@ export class Debug {
     serviceNotLoaded: (loader: Function) => "Service is not loaded yet",
     classNotDecorated: (name: string) =>
       `Class "${name}" not registered as service`,
+    optionalServiceAsDependency: (name: string, from: string) =>
+      `Optional service ${name} cannot be resolved during construction of service ${from}`,
   };
   public static warnings = {
     asyncValidation: (descriptor: IServiceDescriptor<unknown>) =>

packages/@wroud/di/src/debugDevelopment.ts

@@ -9,6 +9,8 @@ Debug.errors = {
     `Service implementation not loaded, loader: ${String(loader)}`,
   classNotDecorated: (name: string) =>
     `Class "${name}" not registered as service (please use @injectable or ServiceRegistry)`,
+  optionalServiceAsDependency: (name: string, from: string) =>
+    `Optional service ${name} cannot be resolved during construction of service ${from}. Please convert it to a regular dependency.`,
 };
 Debug.warnings = {
   asyncValidation: (descriptor: IServiceDescriptor<unknown>) =>

packages/@wroud/di/src/di/ServiceContainerBuilder.test.ts

@@ -1,9 +1,11 @@
 import { expect, it, describe } from "vitest";
 import { ServiceContainerBuilder } from "./ServiceContainerBuilder.js";
 import { ServiceCollection } from "./ServiceCollection.js";
-import { ServiceRegistry } from "./ServiceRegistry.js";
 import { lazy } from "../implementation-resolvers/lazy.js";
 import { single } from "../service-type-resolvers/single.js";
+import { createSyncMockedService } from "../tests/createSyncMockedService.js";
+import { createAsyncMockedService } from "../tests/createAsyncMockedService.js";
+import { createService } from "./createService.js";
 
 describe("ServiceContainerBuilder", () => {
   it("should be defined", () => {
@@ -24,30 +26,55 @@ describe("ServiceContainerBuilder", () => {
     );
   });
   it("should detect async cyclic dependencies", async () => {
-    class Test1 {}
-    class Test2 {}
-    ServiceRegistry.register(Test1, {
-      name: Test1.name,
-      dependencies: [single(Test2)],
-    });
-    ServiceRegistry.register(Test2, {
-      name: Test2.name,
-      dependencies: [single(Test1)],
-    });
+    const Test1Service = createService("Test1");
+    const Test2Service = createService("Test2");
+
+    const Test1 = createAsyncMockedService("Test1", () => [
+      single(Test2Service),
+    ]);
+    const Test2 = createAsyncMockedService("Test2", () => [
+      single(Test1Service),
+    ]);
+    const Test3 = createAsyncMockedService("Test3", () => []);
+    const Test4 = createAsyncMockedService("Test4", () => []);
 
     const builder = new ServiceContainerBuilder();
     builder
+      .addSingleton(Test3)
+      .addTransient(Test4)
       .addScoped(
-        Test1,
+        Test1Service,
         lazy(() => Promise.resolve(Test1)),
       )
       .addScoped(
-        Test2,
+        Test2Service,
         lazy(() => Promise.resolve(Test2)),
       );
 
     await expect(() => builder.validate()).rejects.toThrowError(
       "Cyclic dependency detected: Test1 -> Test2 -> Test1",
     );
+
+    expect(Test1.constructorMock).not.toHaveBeenCalled();
+    expect(Test2.constructorMock).not.toHaveBeenCalled();
+    expect(Test3.constructorMock).not.toHaveBeenCalled();
+    expect(Test4.constructorMock).not.toHaveBeenCalled();
+  });
+  it("validate should have no side effects", async () => {
+    const Test1 = createSyncMockedService("Test1", () => []);
+    const Test2 = createSyncMockedService("Test2", () => []);
+    const Test3 = createSyncMockedService("Test3", () => []);
+
+    const builder = new ServiceContainerBuilder();
+    builder.addSingleton(Test1).addScoped(Test2).addTransient(Test3);
+
+    await builder.validate();
+
+    for (const descriptor of builder) {
+      expect(descriptor.dry).not.toBeDefined();
+    }
+    expect(Test1.constructorMock).not.toHaveBeenCalled();
+    expect(Test2.constructorMock).not.toHaveBeenCalled();
+    expect(Test3.constructorMock).not.toHaveBeenCalled();
   });
 });

packages/@wroud/di/src/di/ServiceInstancesStore.test.ts

@@ -19,4 +19,20 @@ describe("ServiceInstancesStore", () => {
     instanceInfo.initialize(() => instance1);
     expect(store.addInstance(descriptor)).toBe(instanceInfo);
   });
+  it("should has", async () => {
+    const store = new ServiceInstancesStore();
+    const descriptor: IServiceDescriptor<{}> = {
+      service: {} as any,
+      resolver: {} as any,
+      lifetime: ServiceLifetime.Singleton,
+    };
+
+    store.addInstance(descriptor);
+    expect(store.hasInstanceOf(descriptor)).toBe(true);
+    expect(
+      store.hasInstanceOf({
+        ...descriptor,
+      }),
+    ).toBe(false);
+  });
 });

packages/@wroud/di/src/di/ServiceProvider.optional.development.test.ts

@@ -0,0 +1,32 @@
+///<reference types="@wroud/tests-runner/jest-extended.d.ts"/>
+import { describe, expect, it } from "vitest";
+import { ServiceContainerBuilder } from "./ServiceContainerBuilder.js";
+import { createService } from "./createService.js";
+import { createSyncMockedService } from "../tests/createSyncMockedService.js";
+import { optional } from "../service-type-resolvers/optional.js";
+import "../debugDevelopment.js";
+
+describe("ServiceProvider development", () => {
+  it("should not resolve from constructor", () => {
+    const AService = createService<InstanceType<typeof A>>("A");
+    const BService = createService<InstanceType<typeof B>>("B");
+
+    const A = createSyncMockedService("A", () => []);
+    const B = createSyncMockedService(
+      "B",
+      () => [optional(AService)],
+      (a) => {
+        a.resolve();
+      },
+    );
+
+    const serviceProvider = new ServiceContainerBuilder()
+      .addSingleton(AService, A)
+      .addSingleton(BService, B)
+      .build();
+
+    expect(() => serviceProvider.getService(BService)).toThrowError(
+      "Optional service A cannot be resolved during construction of service B. Please convert it to a regular dependency.",
+    );
+  });
+});

packages/@wroud/di/src/di/ServiceProvider.optional.test.ts

@@ -73,4 +73,59 @@ describe("ServiceProvider", () => {
     expect(c.deps).toHaveLength(1);
     expect(a.deps).toHaveLength(0);
   });
+  it("should break cycles", () => {
+    const AService = createService<InstanceType<typeof A>>("A");
+    const BService = createService<InstanceType<typeof B>>("B");
+    const CService = createService<InstanceType<typeof C>>("C");
+
+    const A = createSyncMockedService("A", () => [CService]);
+    const B = createSyncMockedService("B", () => [AService]);
+    const C = createSyncMockedService("C", () => [optional(BService)]);
+
+    const serviceProvider = new ServiceContainerBuilder()
+      .addSingleton(AService, A)
+      .addSingleton(BService, B)
+      .addSingleton(CService, C)
+      .build();
+
+    const c = serviceProvider.getService(CService);
+
+    expect(c.deps[0].resolve()).toEqual(serviceProvider.getService(BService));
+    expect(c.deps[0].resolve().deps[0]).toEqual(
+      serviceProvider.getService(AService),
+    );
+
+    const b = serviceProvider.getService(BService);
+    const a = serviceProvider.getService(AService);
+
+    expect(a.deps).toHaveLength(1);
+    expect(b.deps).toHaveLength(1);
+    expect(c.deps).toHaveLength(1);
+
+    expect(c.deps[0].resolve()).toBeInstanceOf(B);
+    expect(c.deps[0].resolve().deps[0]).toBeInstanceOf(A);
+    expect(c.deps[0].resolve().deps[0].deps[0]).toBeInstanceOf(C);
+  });
+  it("should not resolve from constructor", () => {
+    const AService = createService<InstanceType<typeof A>>("A");
+    const BService = createService<InstanceType<typeof B>>("B");
+
+    const A = createSyncMockedService("A", () => []);
+    const B = createSyncMockedService(
+      "B",
+      () => [optional(AService)],
+      (a) => {
+        a.resolve();
+      },
+    );
+
+    const serviceProvider = new ServiceContainerBuilder()
+      .addSingleton(AService, A)
+      .addSingleton(BService, B)
+      .build();
+
+    expect(() => serviceProvider.getService(BService)).toThrowError(
+      "Optional service A cannot be resolved during construction of service B",
+    );
+  });
 });

packages/@wroud/di/src/di/ServiceProvider.static.test.ts

@@ -3,6 +3,8 @@ import { describe, expect, it } from "vitest";
 import { ServiceProvider } from "./ServiceProvider.js";
 import { createService } from "./createService.js";
 import { single } from "../service-type-resolvers/single.js";
+import { createSyncMockedService } from "../tests/createSyncMockedService.js";
+import { ServiceContainerBuilder } from "./ServiceContainerBuilder.js";
 
 describe("ServiceProvider", () => {
   it("should should check for instance in static internalGetService", () => {
@@ -25,4 +27,17 @@ describe("ServiceProvider", () => {
       ServiceProvider.getDescriptors({} as any, createService("test")),
     ).toThrowError("provider must be an instance of ServiceProvider");
   });
+  it("should return descriptor for service", () => {
+    const A = createSyncMockedService("A");
+    const B = createSyncMockedService("B");
+    const S = createService("S");
+
+    const serviceProvider = new ServiceContainerBuilder()
+      .addSingleton(S, A)
+      .addSingleton(S, B)
+      .build();
+
+    expect(ServiceProvider.getDescriptor(serviceProvider, S)).toBeDefined();
+    expect(ServiceProvider.getDescriptors(serviceProvider, S)).toHaveLength(2);
+  });
 });

packages/@wroud/di/src/di/ServiceProvider.ts

@@ -125,6 +125,7 @@ export class ServiceProvider implements IServiceProvider {
   ): Generator<Promise<unknown>, T, unknown> {
     return yield* service.resolve(
       this.collection,
+      this.instancesStore,
       this.resolveServiceImplementation,
       requestedBy,
       mode,
@@ -136,7 +137,9 @@ export class ServiceProvider implements IServiceProvider {
     requestedBy: Set<IServiceDescriptor<any>>,
     mode: "sync" | "async",
   ): Generator<Promise<unknown>, T, unknown> {
-    validateRequestPath(requestedBy, descriptor);
+    if (!this.instancesStore.getInstanceInfo(descriptor)?.initialized) {
+      validateRequestPath(requestedBy, descriptor);
+    }
 
     if (descriptor.lifetime === ServiceLifetime.Singleton && this.parent) {
       return yield* (

packages/@wroud/di/src/helpers/getNameOfServiceType.test.ts

@@ -27,6 +27,10 @@ describe("getNameOfServiceType", () => {
     expect(getNameOfServiceType(null)).toBe("null");
   });
 
+  it("should return the correct name for a given undefined", () => {
+    expect(getNameOfServiceType(undefined)).toBe("undefined");
+  });
+
   it("should return the correct name for a given anonymous function from var", () => {
     const service = () => {};
     expect(getNameOfServiceType(service)).toBe("service");