chore(modules-sdk): load modules in sequence (#9327)

What:
- The time taken to load in sequence is the same as in parallel, and it doesn't create multiple db queries simultaneously when starting each module.
- Rework modules bootstrap (now all dependencies are available from the constructor and cross deps are allowed without any topological sort needed. It also allow improvements in the future)
  - First load all modules
  - then resolve and register instances


Co-authored-by: Adrien de Peretti <25098370+adrien2p@users.noreply.github.com>

packages/core/modules-sdk/src/medusa-app.ts

@@ -32,6 +32,7 @@ import { MODULE_PACKAGE_NAMES } from "./definitions"
 import {
   MedusaModule,
   MigrationOptions,
+  ModuleBootstrapOptions,
   RegisterModuleJoinerConfig,
 } from "./medusa-module"
 import { RemoteLink } from "./remote-link"
@@ -97,22 +98,29 @@ export async function loadModules(args: {
     sharedResourcesConfig,
     migrationOnly = false,
     loaderOnly = false,
-    workerMode = "server",
+    workerMode = "server" as ModuleBootstrapOptions["workerMode"],
   } = args
 
-  const allModules = {}
+  const allModules = {} as any
 
-  await Promise.all(
+  const modulesToLoad: {
-    Object.keys(modulesConfig).map(async (moduleName) => {
+    moduleKey: string
+    defaultPath: string
+    declaration: InternalModuleDeclaration | ExternalModuleDeclaration
+    sharedContainer: MedusaContainer
+    moduleDefinition: ModuleDefinition
+    moduleExports?: ModuleExports
+  }[] = []
+
+  for (const moduleName of Object.keys(modulesConfig)) {
     const mod = modulesConfig[moduleName]
     let path: string
     let moduleExports: ModuleExports | undefined = undefined
     let declaration: any = {}
     let definition: Partial<ModuleDefinition> | undefined = undefined
 
-      // Skip disabled modules
     if (mod === false) {
-        return
+      continue
     }
 
     if (isObject(mod)) {
@@ -129,10 +137,7 @@ export async function loadModules(args: {
     }
 
     declaration.scope ??= MODULE_SCOPE.INTERNAL
-      if (
+    if (declaration.scope === MODULE_SCOPE.INTERNAL && !declaration.resources) {
-        declaration.scope === MODULE_SCOPE.INTERNAL &&
-        !declaration.resources
-      ) {
       declaration.resources = MODULE_RESOURCE_TYPE.SHARED
     }
 
@@ -148,38 +153,49 @@ export async function loadModules(args: {
         sharedResourcesConfig?.database?.debug
     }
 
-      const loaded = (await MedusaModule.bootstrap({
+    modulesToLoad.push({
       moduleKey: moduleName,
       defaultPath: path,
       declaration,
       sharedContainer,
       moduleDefinition: definition as ModuleDefinition,
       moduleExports,
+    })
+  }
+
+  const loaded = (await MedusaModule.bootstrapAll(modulesToLoad, {
     migrationOnly,
     loaderOnly,
     workerMode,
-      })) as LoadedModule
+  })) as LoadedModule[]
 
   if (loaderOnly) {
-        return
+    return allModules
+  }
+
+  for (const { moduleKey } of modulesToLoad) {
+    const service = loaded.find((loadedModule) => loadedModule[moduleKey])?.[
+      moduleKey
+    ]
+    if (!service) {
+      throw new Error(`Module ${moduleKey} could not be loaded.`)
     }
 
-      const service = loaded[moduleName]
     sharedContainer.register({
       [service.__definition.key]: asValue(service),
     })
 
-      if (allModules[moduleName] && !Array.isArray(allModules[moduleName])) {
+    if (allModules[moduleKey] && !Array.isArray(allModules[moduleKey])) {
-        allModules[moduleName] = []
+      allModules[moduleKey] = []
     }
 
-      if (allModules[moduleName]) {
+    if (allModules[moduleKey]) {
-        ;(allModules[moduleName] as LoadedModule[]).push(loaded[moduleName])
+      ;(allModules[moduleKey] as LoadedModule[]).push(service)
     } else {
-        allModules[moduleName] = loaded[moduleName]
+      allModules[moduleKey] = service
     }
-    })
+  }
-  )
+
   return allModules
 }
 
@@ -374,7 +390,7 @@ async function MedusaApp_({
   const allModules = await loadModules({
     modulesConfig: modules,
     sharedContainer: sharedContainer_,
-    sharedResourcesConfig,
+    sharedResourcesConfig: { database: dbData },
     migrationOnly,
     loaderOnly,
     workerMode,

packages/core/modules-sdk/src/medusa-module.ts

@@ -273,6 +273,52 @@ class MedusaModule {
     MedusaModule.modules_.set(moduleKey, modules!)
   }
 
+  /**
+   * Load all modules and resolve them once they are loaded
+   * @param modulesOptions
+   * @param migrationOnly
+   * @param loaderOnly
+   * @param workerMode
+   */
+  public static async bootstrapAll(
+    modulesOptions: Omit<
+      ModuleBootstrapOptions,
+      "migrationOnly" | "loaderOnly" | "workerMode"
+    >[],
+    {
+      migrationOnly,
+      loaderOnly,
+      workerMode,
+    }: {
+      migrationOnly?: boolean
+      loaderOnly?: boolean
+      workerMode?: ModuleBootstrapOptions["workerMode"]
+    }
+  ): Promise<
+    {
+      [key: string]: any
+    }[]
+  > {
+    return await MedusaModule.bootstrap_(modulesOptions, {
+      migrationOnly,
+      loaderOnly,
+      workerMode,
+    })
+  }
+
+  /**
+   * Load a single module and resolve it once it is loaded
+   * @param moduleKey
+   * @param defaultPath
+   * @param declaration
+   * @param moduleExports
+   * @param sharedContainer
+   * @param moduleDefinition
+   * @param injectedDependencies
+   * @param migrationOnly
+   * @param loaderOnly
+   * @param workerMode
+   */
   public static async bootstrap<T>({
     moduleKey,
     defaultPath,
@@ -287,19 +333,82 @@ class MedusaModule {
   }: ModuleBootstrapOptions): Promise<{
     [key: string]: T
   }> {
-    const hashKey = simpleHash(
+    const [service] = await MedusaModule.bootstrap_(
-      stringifyCircular({ moduleKey, defaultPath, declaration })
+      [
+        {
+          moduleKey,
+          defaultPath,
+          declaration,
+          moduleExports,
+          sharedContainer,
+          moduleDefinition,
+          injectedDependencies,
+        },
+      ],
+      {
+        migrationOnly,
+        loaderOnly,
+        workerMode,
+      }
     )
 
-    if (!loaderOnly && MedusaModule.instances_.has(hashKey)) {
+    return service as {
-      return MedusaModule.instances_.get(hashKey)! as {
       [key: string]: T
     }
   }
 
-    if (!loaderOnly && MedusaModule.loading_.has(hashKey)) {
+  /**
-      return MedusaModule.loading_.get(hashKey)
+   * Load all modules and then resolve them once they are loaded
+   *
+   * @param modulesOptions
+   * @param migrationOnly
+   * @param loaderOnly
+   * @param workerMode
+   * @protected
+   */
+  protected static async bootstrap_<T>(
+    modulesOptions: Omit<
+      ModuleBootstrapOptions,
+      "migrationOnly" | "loaderOnly" | "workerMode"
+    >[],
+    {
+      migrationOnly,
+      loaderOnly,
+      workerMode,
+    }: {
+      migrationOnly?: boolean
+      loaderOnly?: boolean
+      workerMode?: "shared" | "worker" | "server"
     }
+  ): Promise<
+    {
+      [key: string]: T
+    }[]
+  > {
+    let loadedModules: {
+      hashKey: string
+      modDeclaration: InternalModuleDeclaration | ExternalModuleDeclaration
+      moduleResolutions: Record<string, ModuleResolution>
+      container: MedusaContainer
+      finishLoading: (arg: { [Key: string]: any }) => void
+    }[] = []
+
+    const services: { [Key: string]: any }[] = []
+
+    for (const moduleOptions of modulesOptions) {
+      const {
+        moduleKey,
+        defaultPath,
+        declaration,
+        moduleExports,
+        sharedContainer,
+        moduleDefinition,
+        injectedDependencies,
+      } = moduleOptions
+
+      const hashKey = simpleHash(
+        stringifyCircular({ moduleKey, defaultPath, declaration })
+      )
 
       let finishLoading: any
       let errorLoading: any
@@ -309,6 +418,16 @@ class MedusaModule {
         errorLoading = reject
       })
 
+      if (!loaderOnly && MedusaModule.instances_.has(hashKey)) {
+        services.push(MedusaModule.instances_.get(hashKey)!)
+        continue
+      }
+
+      if (!loaderOnly && MedusaModule.loading_.has(hashKey)) {
+        services.push(await MedusaModule.loading_.get(hashKey))
+        continue
+      }
+
       if (!loaderOnly) {
         MedusaModule.loading_.set(hashKey, loadingPromise)
       }
@@ -320,14 +439,17 @@ class MedusaModule {
       if (declaration?.scope !== MODULE_SCOPE.EXTERNAL) {
         modDeclaration = {
           scope: declaration?.scope || MODULE_SCOPE.INTERNAL,
-        resources: declaration?.resources || MODULE_RESOURCE_TYPE.ISOLATED,
+          resources:
+            (declaration as InternalModuleDeclaration)?.resources ||
+            MODULE_RESOURCE_TYPE.ISOLATED,
           resolve: defaultPath,
           options: declaration?.options ?? declaration,
-        dependencies: declaration?.dependencies ?? [],
+          dependencies:
+            (declaration as InternalModuleDeclaration)?.dependencies ?? [],
           alias: declaration?.alias,
           main: declaration?.main,
           worker_mode: workerMode,
-      }
+        } as any
       }
 
       // TODO: Only do that while legacy modules sharing the manager exists then remove the ternary in favor of createMedusaContainer({}, globalContainer)
@@ -371,13 +493,73 @@ class MedusaModule {
         throw err
       }
 
-    const services = {}
+      loadedModules.push({
+        hashKey,
+        modDeclaration,
+        moduleResolutions,
+        container,
+        finishLoading,
+      })
+    }
 
     if (loaderOnly) {
-      finishLoading(services)
+      loadedModules.forEach(({ finishLoading }) => finishLoading({}))
+      return [{}]
+    }
+
+    for (const {
+      hashKey,
+      modDeclaration,
+      moduleResolutions,
+      container,
+      finishLoading,
+    } of loadedModules) {
+      const service = await MedusaModule.resolveLoadedModule({
+        hashKey,
+        modDeclaration,
+        moduleResolutions,
+        container,
+      })
+
+      MedusaModule.instances_.set(hashKey, service)
+      finishLoading(service)
+      MedusaModule.loading_.delete(hashKey)
+      services.push(service)
+    }
+
     return services
   }
 
+  /**
+   * Resolve all the modules once they all have been loaded through the bootstrap
+   * and store their references in the instances_ map and return them
+   *
+   * @param hashKey
+   * @param modDeclaration
+   * @param moduleResolutions
+   * @param container
+   * @private
+   */
+  private static async resolveLoadedModule({
+    hashKey,
+    modDeclaration,
+    moduleResolutions,
+    container,
+  }: {
+    hashKey: string
+    modDeclaration: InternalModuleDeclaration | ExternalModuleDeclaration
+    moduleResolutions: Record<string, ModuleResolution>
+    container: MedusaContainer
+  }): Promise<{
+    [key: string]: any
+  }> {
+    const logger_ =
+      container.resolve(ContainerRegistrationKeys.LOGGER, {
+        allowUnregistered: true,
+      }) ?? logger
+
+    const services: { [key: string]: any } = {}
+
     for (const resolution of Object.values(
       moduleResolutions
     ) as ModuleResolution[]) {
@@ -390,6 +572,7 @@ class MedusaModule {
         let joinerConfig!: ModuleJoinerConfig
 
         try {
+          // TODO: rework that to store on a separate property
           joinerConfig = await services[keyName].__joinerConfig?.()
         } catch {
           // noop
@@ -424,10 +607,6 @@ class MedusaModule {
       })
     }
 
-    MedusaModule.instances_.set(hashKey, services)
-    finishLoading(services)
-    MedusaModule.loading_.delete(hashKey)
-
     return services
   }
 
@@ -447,7 +626,7 @@ class MedusaModule {
     }
 
     if (MedusaModule.loading_.has(hashKey)) {
-      return MedusaModule.loading_.get(hashKey)
+      return await MedusaModule.loading_.get(hashKey)
     }
 
     let finishLoading: any