refactor(core): Improve internal module architecture

Makes it work better with new plugin system

packages/core/e2e/plugin.e2e-spec.ts

@@ -52,12 +52,12 @@ describe('Plugins', () => {
         await server.destroy();
     });
 
-    it('calls onVendureBootstrap once only', () => {
-        expect(onBootstrapFn.mock.calls.length).toBe(1);
+    it('calls onVendureBootstrap', () => {
+        expect(onBootstrapFn).toHaveBeenCalled();
     });
 
-    it('calls onWorkerVendureBootstrap once only', () => {
-        expect(onWorkerBootstrapFn.mock.calls.length).toBe(1);
+    it('calls onWorkerVendureBootstrap', () => {
+        expect(onWorkerBootstrapFn).toHaveBeenCalled();
     });
 
     it('can modify the config in configure()', () => {
@@ -99,12 +99,12 @@ describe('Plugins', () => {
             await server.destroy();
         });
 
-        it('calls onVendureClose once only', () => {
-            expect(onCloseFn.mock.calls.length).toBe(1);
+        it('calls onVendureClose', () => {
+            expect(onCloseFn).toHaveBeenCalled();
         });
 
-        it('calls onWorkerVendureClose once only', () => {
-            expect(onWorkerCloseFn.mock.calls.length).toBe(1);
+        it('calls onWorkerVendureClose', () => {
+            expect(onWorkerCloseFn).toHaveBeenCalled();
         });
     });
 });

packages/core/src/api/api-internal-modules.ts

@@ -2,7 +2,7 @@ import { Module } from '@nestjs/common';
 
 import { ConfigModule } from '../config/config.module';
 import { DataImportModule } from '../data-import/data-import.module';
-import { PluginModule } from '../plugin/plugin.module';
+import { createDynamicGraphQlModulesForPlugins } from '../plugin/dynamic-plugin-api.module';
 import { ServiceModule } from '../service/service.module';
 
 import { IdCodecService } from './common/id-codec.service';
@@ -112,7 +112,12 @@ export class ApiSharedModule {}
  * The internal module containing the Admin GraphQL API resolvers
  */
 @Module({
-    imports: [ApiSharedModule, ServiceModule.forRoot(), DataImportModule, PluginModule.forAdmin()],
+    imports: [
+        ApiSharedModule,
+        ServiceModule.forRoot(),
+        DataImportModule,
+        ...createDynamicGraphQlModulesForPlugins('admin'),
+    ],
     providers: [...adminResolvers, ...entityResolvers, ...adminEntityResolvers],
     exports: [...adminResolvers],
 })
@@ -122,7 +127,7 @@ export class AdminApiModule {}
  * The internal module containing the Shop GraphQL API resolvers
  */
 @Module({
-    imports: [ApiSharedModule, ServiceModule.forRoot(), PluginModule.forShop()],
+    imports: [ApiSharedModule, ServiceModule.forRoot(), ...createDynamicGraphQlModulesForPlugins('shop')],
     providers: [...shopResolvers, ...entityResolvers],
     exports: [...shopResolvers],
 })

packages/core/src/app.module.ts

@@ -8,9 +8,10 @@ import { ConfigService } from './config/config.service';
 import { Logger } from './config/logger/vendure-logger';
 import { I18nModule } from './i18n/i18n.module';
 import { I18nService } from './i18n/i18n.service';
+import { PluginModule } from './plugin/plugin.module';
 
 @Module({
-    imports: [ConfigModule, I18nModule, ApiModule],
+    imports: [ConfigModule, I18nModule, ApiModule, PluginModule],
 })
 export class AppModule implements NestModule, OnApplicationShutdown {
     constructor(private configService: ConfigService, private i18nService: I18nService) {}

packages/core/src/data-import/data-import.module.ts

@@ -12,7 +12,7 @@ import { Populator } from './providers/populator/populator';
     // Important! PluginModule must be defined before ServiceModule
     // in order that overrides of Services (e.g. SearchService) are correctly
     // registered with the injector.
-    imports: [PluginModule.forRoot(), ServiceModule.forRoot(), ConfigModule],
+    imports: [PluginModule, ServiceModule.forRoot(), ConfigModule],
     exports: [ImportParser, Importer, Populator],
     providers: [ImportParser, Importer, Populator],
 })

packages/core/src/plugin/plugin.module.ts

@@ -6,7 +6,6 @@ import { ConfigModule } from '../config/config.module';
 import { ConfigService } from '../config/config.service';
 import { Logger } from '../config/logger/vendure-logger';
 
-import { createDynamicGraphQlModulesForPlugins } from './dynamic-plugin-api.module';
 import {
     getPluginModules,
     getWorkerControllers,
@@ -27,33 +26,10 @@ const PLUGIN_PROCESS_CONTEXT = 'PLUGIN_PROCESS_CONTEXT';
  * modules and in responsible for executing any lifecycle methods defined by the plugins.
  */
 @Module({
-    imports: [ConfigModule],
+    imports: [ConfigModule, ...getConfig().plugins],
+    providers: [{ provide: PLUGIN_PROCESS_CONTEXT, useValue: PluginProcessContext.Main }],
 })
 export class PluginModule implements OnModuleInit, OnModuleDestroy {
-    static forShop(): DynamicModule {
-        return {
-            module: PluginModule,
-            providers: [{ provide: PLUGIN_PROCESS_CONTEXT, useValue: PluginProcessContext.Main }],
-            imports: [...getConfig().plugins, ...createDynamicGraphQlModulesForPlugins('shop')],
-        };
-    }
-
-    static forAdmin(): DynamicModule {
-        return {
-            module: PluginModule,
-            providers: [{ provide: PLUGIN_PROCESS_CONTEXT, useValue: PluginProcessContext.Main }],
-            imports: [...getConfig().plugins, ...createDynamicGraphQlModulesForPlugins('admin')],
-        };
-    }
-
-    static forRoot(): DynamicModule {
-        return {
-            module: PluginModule,
-            providers: [{ provide: PLUGIN_PROCESS_CONTEXT, useValue: PluginProcessContext.Main }],
-            imports: [...getConfig().plugins],
-        };
-    }
-
     static forWorker(): DynamicModule {
         return {
             module: PluginModule,
@@ -61,12 +37,6 @@ export class PluginModule implements OnModuleInit, OnModuleDestroy {
             imports: [...pluginsWithWorkerControllers()],
         };
     }
-
-    private static mainBootstrapHasRun = false;
-    private static mainCloseHasRun = false;
-    private static workerBootstrapHasRun = false;
-    private static workerCloseHasRun = false;
-
     constructor(
         @Inject(PLUGIN_PROCESS_CONTEXT) private processContext: PluginProcessContext,
         private moduleRef: ModuleRef,
@@ -74,26 +44,22 @@ export class PluginModule implements OnModuleInit, OnModuleDestroy {
     ) {}
 
     async onModuleInit() {
-        if (!PluginModule.mainBootstrapHasRun && this.processContext === PluginProcessContext.Main) {
-            PluginModule.mainBootstrapHasRun = true;
+        if (this.processContext === PluginProcessContext.Main) {
             this.runPluginLifecycleMethods('onVendureBootstrap', instance => {
                 const pluginName = instance.constructor.name || '(anonymous plugin)';
                 Logger.verbose(`Bootstrapped plugin ${pluginName}`);
             });
         }
-        if (!PluginModule.workerBootstrapHasRun && this.processContext === PluginProcessContext.Worker) {
-            PluginModule.workerBootstrapHasRun = true;
+        if (this.processContext === PluginProcessContext.Worker) {
             this.runPluginLifecycleMethods('onVendureWorkerBootstrap');
         }
     }
 
     async onModuleDestroy() {
-        if (!PluginModule.mainCloseHasRun && this.processContext === PluginProcessContext.Main) {
-            PluginModule.mainCloseHasRun = true;
+        if (this.processContext === PluginProcessContext.Main) {
             await this.runPluginLifecycleMethods('onVendureClose');
         }
-        if (!PluginModule.workerCloseHasRun && this.processContext === PluginProcessContext.Worker) {
-            PluginModule.workerCloseHasRun = true;
+        if (this.processContext === PluginProcessContext.Worker) {
             this.runPluginLifecycleMethods('onVendureWorkerClose');
         }
     }

packages/core/src/service/service.module.ts

@@ -81,11 +81,9 @@ let defaultTypeOrmModule: DynamicModule;
 let workerTypeOrmModule: DynamicModule;
 
 /**
- * The ServiceModule is responsible for the service layer, i.e. accessing the database
- * and implementing the main business logic of the application.
- *
- * The exported providers are used in the ApiModule, which is responsible for parsing requests
- * into a format suitable for the service layer logic.
+ * The ServiceCoreModule is imported internally by the ServiceModule. It is arranged in this way so that
+ * there is only a single instance of this module being instantiated, and thus the lifecycle hooks will
+ * only run a single time.
  */
 @Module({
     imports: [ConfigModule, EventBusModule],
@@ -106,7 +104,7 @@ let workerTypeOrmModule: DynamicModule;
     ],
     exports: exportedProviders,
 })
-export class ServiceModule implements OnModuleInit {
+export class ServiceCoreModule implements OnModuleInit {
     constructor(
         private channelService: ChannelService,
         private roleService: RoleService,
@@ -132,7 +130,20 @@ export class ServiceModule implements OnModuleInit {
         await this.shippingMethodService.initShippingMethods();
         await this.paymentMethodService.initPaymentMethods();
     }
+}
 
+/**
+ * The ServiceModule is responsible for the service layer, i.e. accessing the database
+ * and implementing the main business logic of the application.
+ *
+ * The exported providers are used in the ApiModule, which is responsible for parsing requests
+ * into a format suitable for the service layer logic.
+ */
+@Module({
+    imports: [ServiceCoreModule],
+    exports: [ServiceCoreModule],
+})
+export class ServiceModule {
     static forRoot(): DynamicModule {
         if (!defaultTypeOrmModule) {
             defaultTypeOrmModule = TypeOrmModule.forRootAsync({