From 3f8b9e27495cc86450200f9f87d3c5405c6abade Mon Sep 17 00:00:00 2001
From: Emilio Astarita <emilio.astarita@gmail.com>
Date: Wed, 5 Feb 2020 00:14:59 -0300
Subject: [PATCH] Fix better error messages for circular dependencies (#42)
 (#90)

* Fix better error messages for circular dependencies (#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                  | 43 +++++++++++--------
 .../fixtures/01-test-case-A01-injects-B01.ts  |  7 +++
 .../fixtures/01-test-case-B01-injects-A01.ts  |  7 +++
 src/__tests__/utils/error-match.ts            |  3 ++
 src/dependency-container.ts                   |  6 +++
 test/jest.config.js                           |  1 +
 6 files changed, 48 insertions(+), 19 deletions(-)
 create mode 100644 src/__tests__/fixtures/01-test-case-A01-injects-B01.ts
 create mode 100644 src/__tests__/fixtures/01-test-case-B01-injects-A01.ts
 create mode 100644 src/__tests__/utils/error-match.ts

diff --git a/src/__tests__/errors.test.ts b/src/__tests__/errors.test.ts
index 661f1b0..1e12b10 100644
--- a/src/__tests__/errors.test.ts
+++ b/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./
+    ])
   );
 });
diff --git a/src/__tests__/fixtures/01-test-case-A01-injects-B01.ts b/src/__tests__/fixtures/01-test-case-A01-injects-B01.ts
new file mode 100644
index 0000000..8ac4709
--- /dev/null
+++ b/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) {}
+}
diff --git a/src/__tests__/fixtures/01-test-case-B01-injects-A01.ts b/src/__tests__/fixtures/01-test-case-B01-injects-A01.ts
new file mode 100644
index 0000000..12fce2b
--- /dev/null
+++ b/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) {}
+}
diff --git a/src/__tests__/utils/error-match.ts b/src/__tests__/utils/error-match.ts
new file mode 100644
index 0000000..d90e6e1
--- /dev/null
+++ b/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+"));
+}
diff --git a/src/dependency-container.ts b/src/dependency-container.ts
index 23872b1..899f8e5 100644
--- a/src/dependency-container.ts
+++ b/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();
     }
diff --git a/test/jest.config.js b/test/jest.config.js
index 2c24fda..68cc0cd 100644
--- a/test/jest.config.js
+++ b/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",