feat: Merge plugin modules (#10895)

Fixes: FRMW-2858

This PR merge the modules exported by the plugins with the modules defined within the user config. As a result, all modules get loaded without changing the internals of the loader.

However, you cannot disable the module of a plugin by re-adding it to the `modules` array. That is something we should handle separately. 

We've added the breaking change label because of the following fix:
We did broke the ability to completely disable modules in the past pr's, in this pr we re add the ability to disable a module and that this modules does not get loaded at all. ([here](6dd164f783))

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

.changeset/thin-games-worry.md

@@ -0,0 +1,7 @@
+---
+"@medusajs/medusa": patch
+"@medusajs/types": patch
+"@medusajs/utils": patch
+---
+
+Feat/merge plugin modules

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

@@ -106,7 +106,8 @@ export async function loadModules(args: {
     let declaration: any = {}
     let definition: Partial<ModuleDefinition> | undefined = undefined
 
-    if (mod === false) {
+    // TODO: We are keeping mod === false for backward compatibility for now
+    if (mod === false || (isObject(mod) && "disable" in mod && mod.disable)) {
       continue
     }
 

packages/core/types/src/common/config-module.ts

@@ -944,6 +944,13 @@ type ExternalModuleDeclarationOverride = ExternalModuleDeclaration & {
   disable?: boolean
 }
 
+/**
+ * Modules accepted by the defineConfig function
+ */
+export type InputConfigModules = Partial<
+  InternalModuleDeclarationOverride | ExternalModuleDeclarationOverride
+>[]
+
 /**
  * The configuration accepted by the "defineConfig" helper
  */
@@ -951,9 +958,7 @@ export type InputConfig = Partial<
   Omit<ConfigModule, "admin" | "modules"> & {
     admin: Partial<ConfigModule["admin"]>
     modules:
-      | Partial<
-          InternalModuleDeclarationOverride | ExternalModuleDeclarationOverride
-        >[]
+      | InputConfigModules
       /**
        * @deprecated use the array instead
        */
@@ -967,5 +972,5 @@ export type PluginDetails = {
   id: string
   options: Record<string, unknown>
   version: string
-  modules?: InputConfig["modules"]
+  modules?: InputConfigModules
 }

packages/core/utils/src/common/__tests__/define-config.spec.ts

@@ -800,7 +800,7 @@ describe("defineConfig", function () {
     `)
   })
 
-  it("should not include disabled modules", function () {
+  it("should include disabled modules", function () {
     expect(
       defineConfig({
         projectConfig: {
@@ -837,6 +837,9 @@ describe("defineConfig", function () {
           "cache": {
             "resolve": "@medusajs/medusa/cache-inmemory",
           },
+          "cart": {
+            "disable": true,
+          },
           "currency": {
             "resolve": "@medusajs/medusa/currency",
           },

packages/core/utils/src/common/__tests__/transform-modules.spec.ts

@@ -0,0 +1,75 @@
+import { MODULE_PACKAGE_NAMES, Modules } from "../../modules-sdk"
+import { transformModules } from "../define-config"
+
+describe("transformModules", () => {
+  test("convert array of modules to an object", () => {
+    const modules = transformModules([
+      {
+        resolve: require.resolve("../__fixtures__/define-config/github"),
+        options: {
+          apiKey: "test",
+        },
+      },
+    ])
+
+    expect(modules).toEqual({
+      GithubModuleService: {
+        options: {
+          apiKey: "test",
+        },
+        resolve: require.resolve("../__fixtures__/define-config/github"),
+      },
+    })
+  })
+
+  test("transform default module", () => {
+    const modules = transformModules([
+      {
+        resolve: MODULE_PACKAGE_NAMES[Modules.CACHE],
+      },
+    ])
+
+    expect(modules).toEqual({
+      cache: {
+        resolve: "@medusajs/medusa/cache-inmemory",
+      },
+    })
+  })
+
+  test("should manage loading priority of modules when its disabled at a later stage in the array", () => {
+    const modules = transformModules([
+      {
+        resolve: MODULE_PACKAGE_NAMES[Modules.CACHE],
+      },
+      {
+        resolve: MODULE_PACKAGE_NAMES[Modules.CACHE],
+        disable: true,
+      },
+    ])
+
+    expect(modules).toEqual({
+      cache: {
+        resolve: MODULE_PACKAGE_NAMES[Modules.CACHE],
+        disable: true,
+      },
+    })
+  })
+
+  test("should manage loading priority of modules when its not disabled at a later stage in the array", () => {
+    const modules = transformModules([
+      {
+        resolve: MODULE_PACKAGE_NAMES[Modules.CACHE],
+        disable: true,
+      },
+      {
+        resolve: MODULE_PACKAGE_NAMES[Modules.CACHE],
+      },
+    ])
+
+    expect(modules).toEqual({
+      cache: {
+        resolve: MODULE_PACKAGE_NAMES[Modules.CACHE],
+      },
+    })
+  })
+})

packages/core/utils/src/common/define-config.ts

@@ -1,6 +1,7 @@
 import {
   ConfigModule,
   InputConfig,
+  InputConfigModules,
   InternalModuleDeclaration,
 } from "@medusajs/types"
 import {
@@ -108,14 +109,79 @@ export function defineConfig(config: InputConfig = {}): ConfigModule {
 }
 
 /**
- * The user API allow to use array of modules configuration. This method manage the loading of the user modules
- * along side the default modules and re map them to an object.
+ * Transforms an array of modules into an object. The last module will
+ * take precedence in case of duplicate modules
+ */
+export function transformModules(
+  modules: InputConfigModules
+): Exclude<ConfigModule["modules"], undefined> {
+  const remappedModules = modules.reduce((acc, moduleConfig) => {
+    if (moduleConfig.scope === "external" && !moduleConfig.key) {
+      throw new Error(
+        "External modules configuration must have a 'key'. Please provide a key for the module."
+      )
+    }
+
+    if ("disable" in moduleConfig && "key" in moduleConfig) {
+      acc[moduleConfig.key!] = moduleConfig
+    }
+
+    // TODO: handle external modules later
+    let serviceName: string =
+      "key" in moduleConfig && moduleConfig.key ? moduleConfig.key : ""
+    delete moduleConfig.key
+
+    if (!serviceName && "resolve" in moduleConfig) {
+      if (
+        isString(moduleConfig.resolve!) &&
+        REVERSED_MODULE_PACKAGE_NAMES[moduleConfig.resolve!]
+      ) {
+        serviceName = REVERSED_MODULE_PACKAGE_NAMES[moduleConfig.resolve!]
+        acc[serviceName] = moduleConfig
+        return acc
+      }
+
+      let resolution = isString(moduleConfig.resolve!)
+        ? normalizeImportPathWithSource(moduleConfig.resolve as string)
+        : moduleConfig.resolve
+
+      const moduleExport = isString(resolution)
+        ? require(resolution)
+        : resolution
+
+      const defaultExport = resolveExports(moduleExport).default
+
+      const joinerConfig =
+        typeof defaultExport.service.prototype.__joinerConfig === "function"
+          ? defaultExport.service.prototype.__joinerConfig() ?? {}
+          : defaultExport.service.prototype.__joinerConfig ?? {}
+
+      serviceName = joinerConfig.serviceName
+
+      if (!serviceName) {
+        throw new Error(
+          `Module ${moduleConfig.resolve} doesn't have a serviceName. Please provide a 'key' for the module or check the service joiner config.`
+        )
+      }
+    }
+
+    acc[serviceName] = moduleConfig
+
+    return acc
+  }, {})
+
+  return remappedModules as Exclude<ConfigModule["modules"], undefined>
+}
+
+/**
+ * The user API allow to use array of modules configuration. This method manage the loading of the
+ * user modules along side the default modules and re map them to an object.
  *
  * @param configModules
  */
 function resolveModules(
   configModules: InputConfig["modules"]
-): ConfigModule["modules"] {
+): Exclude<ConfigModule["modules"], undefined> {
   /**
    * The default set of modules to always use. The end user can swap
    * the modules by providing an alternate implementation via their
@@ -225,67 +291,5 @@ function resolveModules(
     }
   }
 
-  const remappedModules = modules.reduce((acc, moduleConfig) => {
-    if (moduleConfig.scope === "external" && !moduleConfig.key) {
-      throw new Error(
-        "External modules configuration must have a 'key'. Please provide a key for the module."
-      )
-    }
-
-    if ("disable" in moduleConfig && "key" in moduleConfig) {
-      acc[moduleConfig.key!] = moduleConfig
-    }
-
-    // TODO: handle external modules later
-    let serviceName: string =
-      "key" in moduleConfig && moduleConfig.key ? moduleConfig.key : ""
-    delete moduleConfig.key
-
-    if (!serviceName && "resolve" in moduleConfig) {
-      if (
-        isString(moduleConfig.resolve!) &&
-        REVERSED_MODULE_PACKAGE_NAMES[moduleConfig.resolve!]
-      ) {
-        serviceName = REVERSED_MODULE_PACKAGE_NAMES[moduleConfig.resolve!]
-        acc[serviceName] = moduleConfig
-        return acc
-      }
-
-      let resolution = isString(moduleConfig.resolve!)
-        ? normalizeImportPathWithSource(moduleConfig.resolve as string)
-        : moduleConfig.resolve
-
-      const moduleExport = isString(resolution)
-        ? require(resolution)
-        : resolution
-
-      const defaultExport = resolveExports(moduleExport).default
-
-      const joinerConfig =
-        typeof defaultExport.service.prototype.__joinerConfig === "function"
-          ? defaultExport.service.prototype.__joinerConfig() ?? {}
-          : defaultExport.service.prototype.__joinerConfig ?? {}
-
-      serviceName = joinerConfig.serviceName
-
-      if (!serviceName) {
-        throw new Error(
-          `Module ${moduleConfig.resolve} doesn't have a serviceName. Please provide a 'key' for the module or check the service joiner config.`
-        )
-      }
-    }
-
-    acc[serviceName] = moduleConfig
-
-    return acc
-  }, {})
-
-  // Remove any modules set to false
-  Object.keys(remappedModules).forEach((key) => {
-    if (remappedModules[key].disable) {
-      delete remappedModules[key]
-    }
-  })
-
-  return remappedModules as ConfigModule["modules"]
+  return transformModules(modules)
 }

packages/core/utils/src/common/index.ts

@@ -80,3 +80,4 @@ export * from "./trim-zeros"
 export * from "./upper-case-first"
 export * from "./validate-handle"
 export * from "./wrap-handler"
+export * from "./merge-plugin-modules"

packages/core/utils/src/common/merge-plugin-modules.ts

@@ -0,0 +1,35 @@
+import type {
+  PluginDetails,
+  ConfigModule,
+  InputConfigModules,
+} from "@medusajs/types"
+import { transformModules } from "./define-config"
+
+/**
+ * Mutates the configModules object and merges the plugin modules with
+ * the modules defined inside the user-config file
+ */
+export function mergePluginModules(
+  configModule: ConfigModule,
+  plugins: PluginDetails[]
+) {
+  /**
+   * Create a flat array of all the modules exposed by the registered
+   * plugins
+   */
+  const pluginsModules = plugins.reduce((result, plugin) => {
+    if (plugin.modules) {
+      result = result.concat(plugin.modules)
+    }
+    return result
+  }, [] as InputConfigModules)
+
+  /**
+   * Merge plugin modules with the modules defined within the
+   * config file.
+   */
+  configModule.modules = {
+    ...transformModules(pluginsModules),
+    ...configModule.modules,
+  }
+}

packages/medusa/src/commands/db/generate.ts

@@ -2,6 +2,7 @@ import { join } from "path"
 import {
   ContainerRegistrationKeys,
   MedusaError,
+  mergePluginModules,
 } from "@medusajs/framework/utils"
 import { LinkLoader } from "@medusajs/framework/links"
 import { logger } from "@medusajs/framework/logger"
@@ -27,6 +28,8 @@ const main = async function ({ directory, modules }) {
     )
 
     const plugins = await getResolvedPlugins(directory, configModule, true)
+    mergePluginModules(configModule, plugins)
+
     const linksSourcePaths = plugins.map((plugin) =>
       join(plugin.resolve, "links")
     )

packages/medusa/src/commands/db/migrate.ts

@@ -1,5 +1,8 @@
 import { join } from "path"
-import { ContainerRegistrationKeys } from "@medusajs/framework/utils"
+import {
+  ContainerRegistrationKeys,
+  mergePluginModules,
+} from "@medusajs/framework/utils"
 import { LinkLoader } from "@medusajs/framework/links"
 import { logger } from "@medusajs/framework/logger"
 import { MedusaAppLoader } from "@medusajs/framework"
@@ -38,6 +41,8 @@ export async function migrate({
   )
 
   const plugins = await getResolvedPlugins(directory, configModule, true)
+  mergePluginModules(configModule, plugins)
+
   const linksSourcePaths = plugins.map((plugin) =>
     join(plugin.resolve, "links")
   )

packages/medusa/src/commands/db/rollback.ts

@@ -2,6 +2,7 @@ import { join } from "path"
 import {
   ContainerRegistrationKeys,
   MedusaError,
+  mergePluginModules,
 } from "@medusajs/framework/utils"
 import { LinkLoader } from "@medusajs/framework/links"
 import { logger } from "@medusajs/framework/logger"
@@ -27,6 +28,8 @@ const main = async function ({ directory, modules }) {
     )
 
     const plugins = await getResolvedPlugins(directory, configModule, true)
+    mergePluginModules(configModule, plugins)
+
     const linksSourcePaths = plugins.map((plugin) =>
       join(plugin.resolve, "links")
     )

packages/medusa/src/commands/db/sync-links.ts

@@ -2,7 +2,10 @@ import boxen from "boxen"
 import chalk from "chalk"
 import { join } from "path"
 import checkbox from "@inquirer/checkbox"
-import { ContainerRegistrationKeys } from "@medusajs/framework/utils"
+import {
+  ContainerRegistrationKeys,
+  mergePluginModules,
+} from "@medusajs/framework/utils"
 import { LinkMigrationsPlannerAction } from "@medusajs/framework/types"
 import { LinkLoader } from "@medusajs/framework/links"
 import { logger } from "@medusajs/framework/logger"
@@ -188,6 +191,8 @@ const main = async function ({ directory, executeSafe, executeAll }) {
     const medusaAppLoader = new MedusaAppLoader()
 
     const plugins = await getResolvedPlugins(directory, configModule, true)
+    mergePluginModules(configModule, plugins)
+
     const linksSourcePaths = plugins.map((plugin) =>
       join(plugin.resolve, "links")
     )

packages/medusa/src/loaders/__tests__/get-resolved-plugins.spec.ts

@@ -33,7 +33,7 @@ describe("getResolvedPlugins | relative paths", () => {
 
     expect(plugins).toEqual([
       {
-        resolve: path.join(fs.basePath, "./plugins/dummy/build"),
+        resolve: path.join(fs.basePath, "./plugins/dummy/.medusa/server/src"),
         name: "my-dummy-plugin",
         id: "my-dummy-plugin",
         options: { apiKey: "asecret" },
@@ -48,7 +48,10 @@ describe("getResolvedPlugins | relative paths", () => {
       name: "my-dummy-plugin",
       version: "1.0.0",
     })
-    await fs.create("plugins/dummy/build/modules/blog/index.js", ``)
+    await fs.create(
+      "plugins/dummy/.medusa/server/src/modules/blog/index.js",
+      ``
+    )
 
     const plugins = await getResolvedPlugins(
       fs.basePath,
@@ -67,7 +70,7 @@ describe("getResolvedPlugins | relative paths", () => {
 
     expect(plugins).toEqual([
       {
-        resolve: path.join(fs.basePath, "./plugins/dummy/build"),
+        resolve: path.join(fs.basePath, "./plugins/dummy/.medusa/server/src"),
         name: "my-dummy-plugin",
         id: "my-dummy-plugin",
         options: { apiKey: "asecret" },
@@ -77,7 +80,7 @@ describe("getResolvedPlugins | relative paths", () => {
             options: {
               apiKey: "asecret",
             },
-            resolve: "./plugins/dummy/build/modules/blog",
+            resolve: "./plugins/dummy/.medusa/server/src/modules/blog",
           },
         ],
       },
@@ -132,7 +135,10 @@ describe("getResolvedPlugins | package reference", () => {
 
     expect(plugins).toEqual([
       {
-        resolve: path.join(fs.basePath, "node_modules/@plugins/dummy/build"),
+        resolve: path.join(
+          fs.basePath,
+          "node_modules/@plugins/dummy/.medusa/server/src"
+        ),
         name: "my-dummy-plugin",
         id: "my-dummy-plugin",
         options: { apiKey: "asecret" },
@@ -149,7 +155,7 @@ describe("getResolvedPlugins | package reference", () => {
       version: "1.0.0",
     })
     await fs.create(
-      "node_modules/@plugins/dummy/build/modules/blog/index.js",
+      "node_modules/@plugins/dummy/.medusa/server/src/modules/blog/index.js",
       ``
     )
 
@@ -170,7 +176,10 @@ describe("getResolvedPlugins | package reference", () => {
 
     expect(plugins).toEqual([
       {
-        resolve: path.join(fs.basePath, "node_modules/@plugins/dummy/build"),
+        resolve: path.join(
+          fs.basePath,
+          "node_modules/@plugins/dummy/.medusa/server/src"
+        ),
         name: "my-dummy-plugin",
         id: "my-dummy-plugin",
         options: { apiKey: "asecret" },
@@ -180,7 +189,7 @@ describe("getResolvedPlugins | package reference", () => {
             options: {
               apiKey: "asecret",
             },
-            resolve: "@plugins/dummy/build/modules/blog",
+            resolve: "@plugins/dummy/.medusa/server/src/modules/blog",
           },
         ],
       },

packages/medusa/src/loaders/helpers/resolve-plugins.ts

@@ -4,6 +4,7 @@ import { isString, readDir } from "@medusajs/framework/utils"
 import { ConfigModule, PluginDetails } from "@medusajs/framework/types"
 
 const MEDUSA_APP_SOURCE_PATH = "src"
+const MEDUSA_PLUGIN_SOURCE_PATH = ".medusa/server/src"
 export const MEDUSA_PROJECT_NAME = "project-plugin"
 
 function createPluginId(name: string): string {
@@ -57,11 +58,8 @@ async function resolvePlugin(
   const resolvedPath = path.dirname(pkgJSON.path)
 
   const name = pkgJSON.contents.name || pluginPath
-  const srcDir = pkgJSON.contents.main
-    ? path.dirname(pkgJSON.contents.main)
-    : "build"
 
-  const resolve = path.join(resolvedPath, srcDir)
+  const resolve = path.join(resolvedPath, MEDUSA_PLUGIN_SOURCE_PATH)
   const modules = await readDir(path.join(resolve, "modules"), {
     ignoreMissing: true,
   })
@@ -75,7 +73,7 @@ async function resolvePlugin(
     version: pkgJSON.contents.version || "0.0.0",
     modules: modules.map((mod) => {
       return {
-        resolve: `${pluginPath}/${srcDir}/modules/${mod.name}`,
+        resolve: `${pluginPath}/${MEDUSA_PLUGIN_SOURCE_PATH}/modules/${mod.name}`,
         options: pluginOptions,
       }
     }),

packages/medusa/src/loaders/index.ts

@@ -7,6 +7,7 @@ import {
 import {
   ContainerRegistrationKeys,
   GraphQLSchema,
+  mergePluginModules,
   promiseAll,
 } from "@medusajs/framework/utils"
 import { asValue } from "awilix"
@@ -147,6 +148,8 @@ export default async ({
   )
 
   const plugins = await getResolvedPlugins(rootDirectory, configModule, true)
+  mergePluginModules(configModule, plugins)
+
   const linksSourcePaths = plugins.map((plugin) =>
     join(plugin.resolve, "links")
   )