Fix better error messages for circular dependencies (microsoft#42) (m…

…icrosoft#90)

* Fix better error messages for circular dependencies (microsoft#42)

Ignore test cases folder.

* Rename imports folder to fixtures

* Code review fix typo in error message

* Move errorMatch helper to __tests__/utils scope.

* Fix tests after typo

src/__tests__/errors.test.ts

@@ -1,6 +1,7 @@
 import {instance as globalContainer} from "../dependency-container";
 import {inject, injectable} from "../decorators";
-
+import {A01} from "./fixtures/01-test-case-A01-injects-B01";
+import errorMatch from "./utils/error-match";
 afterEach(() => {
   globalContainer.reset();
 });
@@ -26,16 +27,12 @@ test("Error message composition", () => {
   expect(() => {
     globalContainer.resolve(A);
   }).toThrow(
-    new RegExp(
-      [
-        /Cannot inject the dependency "b" at position #1 of "A" constructor\. Reason:/,
-        /Cannot inject the dependency "c" at position #0 of "B" constructor\. Reason:/,
-        /Cannot inject the dependency "s" at position #0 of "C" constructor\. Reason:/,
-        /TypeInfo not known for "Object"/
-      ]
-        .map(x => x.source)
-        .join("\\s+")
-    )
+    errorMatch([
+      /Cannot inject the dependency "b" at position #1 of "A" constructor\. Reason:/,
+      /Cannot inject the dependency "c" at position #0 of "B" constructor\. Reason:/,
+      /Cannot inject the dependency "s" at position #0 of "C" constructor\. Reason:/,
+      /TypeInfo not known for "Object"/
+    ])
   );
 });
 
@@ -48,13 +45,21 @@ test("Param position", () => {
   expect(() => {
     globalContainer.resolve(A);
   }).toThrow(
-    new RegExp(
-      [
-        /Cannot inject the dependency "j" at position #0 of "A" constructor\. Reason:/,
-        /Attempted to resolve unregistered dependency token: "missing"/
-      ]
-        .map(x => x.source)
-        .join("\\s+")
-    )
+    errorMatch([
+      /Cannot inject the dependency "j" at position #0 of "A" constructor\. Reason:/,
+      /Attempted to resolve unregistered dependency token: "missing"/
+    ])
+  );
+});
+
+test("Detect circular dependency", () => {
+  expect(() => {
+    globalContainer.resolve(A01);
+  }).toThrow(
+    errorMatch([
+      /Cannot inject the dependency "b" at position #0 of "A01" constructor\. Reason:/,
+      /Cannot inject the dependency "a" at position #0 of "B01" constructor\. Reason:/,
+      /Attempted to construct an undefined constructor. Could mean a circular dependency problem./
+    ])
   );
 });

src/__tests__/fixtures/01-test-case-A01-injects-B01.ts

@@ -0,0 +1,7 @@
+import {B01} from "./01-test-case-B01-injects-A01";
+import {injectable} from "../../decorators";
+
+@injectable()
+export class A01 {
+  constructor(public b: B01) {}
+}

src/__tests__/fixtures/01-test-case-B01-injects-A01.ts

@@ -0,0 +1,7 @@
+import {A01} from "./01-test-case-A01-injects-B01";
+import {injectable} from "../../decorators";
+
+@injectable()
+export class B01 {
+  constructor(public a: A01) {}
+}

src/__tests__/utils/error-match.ts

@@ -0,0 +1,3 @@
+export default function errorMatch(lines: RegExp[]): RegExp {
+  return new RegExp(lines.map(x => x.source).join("\\s+"));
+}

src/dependency-container.ts

@@ -337,6 +337,12 @@ class InternalDependencyContainer implements DependencyContainer {
   }
 
   private construct<T>(ctor: constructor<T>, context: ResolutionContext): T {
+    if (typeof ctor === "undefined") {
+      throw new Error(
+        "Attempted to construct an undefined constructor. Could mean a circular dependency problem."
+      );
+    }
+
     if (ctor.length === 0) {
       return new ctor();
     }

test/jest.config.js

@@ -14,6 +14,7 @@ module.exports = {
       tsConfig: "src/__tests__/tsconfig.json"
     }
   },
+  testPathIgnorePatterns: ["/node_modules/", "/fixtures/", "/utils/"],
   moduleFileExtensions: ["ts", "tsx", "js"],
   setupFilesAfterEnv: ["<rootDir>/test/jest.setup.ts"],
   testEnvironment: "node",