Make circular imports optional

packages/core/src/errors/index.ts

@@ -5,3 +5,4 @@ export { SchemaNotValidError } from './schema-not-valid';
 export { DependencyModuleUndefinedError } from './dependency-module-undefined';
 export { TypeDefNotFoundError } from './typedef-not-found';
 export { ProviderNotValidError } from './provider-not-valid';
+export { ModuleConfigRequiredError } from './module-config-required';

packages/core/src/errors/module-config-required.ts

@@ -0,0 +1,19 @@
+export class ModuleConfigRequiredError extends Error {
+  constructor(private _moduleName: string) {
+    super(`
+      GraphQL-Modules Error: Module needs a configuration object!
+      - #Module #${_moduleName} isn't imported by a configuration object.
+
+      Possible solutions:
+      - You should pass a valid configuration to import this module using forRoot.
+      - If you already pass a configuration object with forRoot in somewhere in the application.
+        You must import that module with forChild in other modules.
+    `);
+    Object.setPrototypeOf(this, ModuleConfigRequiredError.prototype);
+    Error.captureStackTrace(this, ModuleConfigRequiredError);
+  }
+  get moduleName() {
+    return this._moduleName;
+  }
+
+}

packages/core/src/graphql-module.ts

@@ -4,7 +4,7 @@ import { Provider, ModuleContext, Injector } from './di';
 import { DocumentNode, GraphQLSchema } from 'graphql';
 import { IResolversComposerMapping, composeResolvers } from './resolvers-composition';
 import { DepGraph } from 'dependency-graph';
-import { DependencyModuleNotFoundError, SchemaNotValidError, DependencyModuleUndefinedError, TypeDefNotFoundError } from './errors';
+import { DependencyModuleNotFoundError, SchemaNotValidError, DependencyModuleUndefinedError, TypeDefNotFoundError, ModuleConfigRequiredError } from './errors';
 import deepmerge = require('deepmerge');
 import { addInjectorToResolversContext, addInjectorToResolversCompositionContext, asArray } from './utils';
 
@@ -75,6 +75,9 @@ export interface GraphQLModuleOptions<Config, Request, Context> {
   schemaDirectives?: GraphQLModuleOption<ISchemaDirectives, Config, Request, Context>;
   logger?: GraphQLModuleOption<ILogger, Config, Request, Context>;
   extraSchemas?: GraphQLModuleOption<GraphQLSchema[], Config, Request, Context>;
+  mergeCircularImports?: boolean;
+  warnCircularImports?: boolean;
+  configRequired?: boolean;
 }
 
 /**
@@ -125,9 +128,20 @@ export class GraphQLModule<Config = any, Request = any, Context = any> {
    */
   constructor(
     private _options: GraphQLModuleOptions<Config, Request, Context> = {},
-    private _moduleConfig: Config = {} as Config,
+    private _moduleConfig?: Config,
   ) {
     _options.name = _options.name || Math.floor(Math.random() * Math.floor(Number.MAX_SAFE_INTEGER)).toString();
+    if (!('mergeCircularImports' in _options)) {
+      _options.mergeCircularImports = true;
+    }
+    if (!('warnCircularImports' in _options)) {
+      _options.warnCircularImports = true;
+    }
+    if (!('logger' in _options)) {
+      _options.logger = {
+        log() {},
+      };
+    }
   }
 
   /**
@@ -156,6 +170,9 @@ export class GraphQLModule<Config = any, Request = any, Context = any> {
    * the `typeDefs` and `resolvers` into `GraphQLSchema`
    */
   get schema() {
+    if (this._options.configRequired && !this._moduleConfig) {
+      throw new ModuleConfigRequiredError(this.name);
+    }
     if (typeof this._cache.schema === 'undefined') {
       this.buildSchemaAndInjector(this.modulesMap);
     }
@@ -166,13 +183,13 @@ export class GraphQLModule<Config = any, Request = any, Context = any> {
    * Gets the application dependency-injection injector
    */
   get injector(): Injector {
-
+    if (this._options.configRequired && !this._moduleConfig) {
+      throw new ModuleConfigRequiredError(this.name);
+    }
     if (typeof this._cache.injector === 'undefined') {
       this.buildSchemaAndInjector(this.modulesMap);
     }
-
     return this._cache.injector;
-
   }
 
   /**
@@ -567,6 +584,12 @@ export class GraphQLModule<Config = any, Request = any, Context = any> {
     } catch (e) {
       const { message } = e as Error;
       const currentPathStr = message.replace('Dependency Cycle Found: ', '');
+      if (!this._options.mergeCircularImports) {
+        throw e;
+      }
+      if (this._options.warnCircularImports) {
+        this.selfLogger.log(e.message);
+      }
       const currentPath = currentPathStr.split(' -> ');
       const moduleIndexMap = new Map<string, number>();
       let start = 0;
@@ -584,7 +607,7 @@ export class GraphQLModule<Config = any, Request = any, Context = any> {
         // if it is merged module, get one module, it will be enough to get merged one.
         return modulesMap.get(moduleName);
       });
-      const mergedModule = GraphQLModule.mergeModules<any, Request, any>(circularModules, modulesMap);
+      const mergedModule = GraphQLModule.mergeModules<any, Request, any>(circularModules, this._options.warnCircularImports, modulesMap);
       for (const moduleName of realPath) {
         modulesMap.set(moduleName, mergedModule);
         for (const subModuleName of moduleName.split('+')) {
@@ -606,7 +629,10 @@ export class GraphQLModule<Config = any, Request = any, Context = any> {
     }
   }
 
-  static mergeModules<Config = any, Request = any, Context = any>(modules: Array<GraphQLModule<any, Request, any>>, modulesMap?: ModulesMap<Request>): GraphQLModule<Config, Request, Context> {
+  static mergeModules<Config = any, Request = any, Context = any>(
+    modules: Array<GraphQLModule<any, Request, any>>,
+    warnCircularImports = false,
+    modulesMap?: ModulesMap<Request>): GraphQLModule<Config, Request, Context> {
     const nameSet = new Set();
     const typeDefsSet = new Set();
     const resolversSet = new Set<IResolvers<any, any>>();
@@ -683,6 +709,8 @@ export class GraphQLModule<Config = any, Request = any, Context = any> {
       schemaDirectives,
       extraSchemas,
       logger,
+      warnCircularImports,
+      mergeCircularImports: true,
     });
   }
 }

packages/core/tests/graphql-module.spec.ts

@@ -7,6 +7,7 @@ import {
   ModuleConfig,
   ModuleContext,
   OnRequest,
+  ModuleConfigRequiredError,
 } from '../src';
 import { execute, GraphQLSchema, printSchema, GraphQLField, GraphQLEnumValue, GraphQLString, defaultFieldResolver } from 'graphql';
 import { stripWhitespaces } from './utils';
@@ -25,6 +26,7 @@ describe('GraphQLModule', () => {
 
   const typesA = [`type A { f: String}`, `type Query { a: A }`];
   const moduleA = new GraphQLModule({
+    name: 'A',
     typeDefs: typesA,
     resolvers: ({ injector }) => ({
       Query: { a: () => ({}) },
@@ -41,6 +43,7 @@ describe('GraphQLModule', () => {
   };
   let resolverCompositionCalled = false;
   const moduleB = new GraphQLModule({
+    name: 'B',
     typeDefs: typesB,
     resolvers: resolversB,
     resolversComposition: {
@@ -57,12 +60,14 @@ describe('GraphQLModule', () => {
   const cContextBuilder = jest.fn(() => ({ user: { id: 1 } }));
   const typesC = [`type C { f: String}`, `type Query { c: C }`];
   const moduleC = new GraphQLModule({
+    name: 'C',
     typeDefs: typesC,
     contextBuilder: cContextBuilder,
   });
 
   // D
   const moduleD = new GraphQLModule({
+    name: 'D',
     typeDefs: typesC,
     contextBuilder: () => {
       throw new Error('oops');
@@ -71,13 +76,15 @@ describe('GraphQLModule', () => {
 
   // E
   const moduleE = new GraphQLModule({
+    name: 'E',
     typeDefs: typesC,
   });
 
   // F
   const typeDefsFnMock = jest.fn().mockReturnValue(typesC);
   const resolversFnMock = jest.fn().mockReturnValue({ C: {} });
   const moduleF = new GraphQLModule({
+    name: 'F',
     typeDefs: typeDefsFnMock,
     resolvers: resolversFnMock,
   });
@@ -89,7 +96,7 @@ describe('GraphQLModule', () => {
 
   // Queries
   const testQuery = gql`query { b { f }}`;
-  const app = new GraphQLModule({ imports: [moduleA, moduleB, moduleC] });
+  const app = new GraphQLModule({ imports: [moduleA, moduleB.forRoot({}), moduleC] });
 
   it('should return the correct GraphQLSchema', async () => {
     const schema = app.schema;
@@ -334,6 +341,17 @@ describe('GraphQLModule', () => {
       expect(injector.get(Provider1).test).toEqual(1);
       expect(injector.get(Provider2).test).toEqual(2);
     });
+    it('should not allow to use modules without configuration if required', async () => {
+      let error;
+      try {
+        const { injector } = new GraphQLModule({
+          configRequired: true,
+        });
+      } catch (e) {
+        error = e;
+      }
+      expect(error).toBeInstanceOf(ModuleConfigRequiredError);
+    });
     it('should encapsulate between providers from different non-dependent modules', async () => {
       class ProviderA {
         test = 0;
@@ -393,6 +411,24 @@ describe('GraphQLModule', () => {
       expect(result.data.test).toBeNull();
       expect(result.errors[0].message).toContain('ProviderB not provided in');
     });
+    it('should throw error if mergeCircularImports is disabled', async () => {
+      const moduleA = new GraphQLModule({
+        imports: () => [moduleA],
+      });
+      const moduleB = new GraphQLModule({
+        imports: () => [moduleB],
+      });
+      let errorMsg;
+      try {
+        const { schema, context } = new GraphQLModule({
+          imports: [moduleA, moduleB],
+          mergeCircularImports: false,
+        });
+      } catch (e) {
+        errorMsg = e.message;
+      }
+      expect(errorMsg).toContain('Dependency Cycle');
+    });
   });
   describe('CommuncationBridge', async () => {
     it('should set CommunicationBridge correctly', async () => {
@@ -471,6 +507,8 @@ describe('GraphQLModule', () => {
         contextBuilder: async () => {
           return {
             counter: 0,
+            foo: '',
+            bar: '',
           };
         },
         resolvers: {