From 379c83933ed12a4ec712e7f3c9b0252e4a4601dd Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Thu, 27 Jul 2023 12:53:31 +0200 Subject: [PATCH] fix(module-sdk): Shared modules loading (#4611) * WIP * tests wording * Create nasty-files-type.md * cleanup --- .changeset/nasty-files-type.md | 5 ++++ integration-tests/plugins/medusa-config.js | 8 +----- .../src/loaders/__tests__/module-loader.ts | 16 +++++------ .../src/loaders/utils/load-internal.ts | 28 ++++++++----------- packages/product/src/loaders/connection.ts | 11 ++++++-- 5 files changed, 35 insertions(+), 33 deletions(-) create mode 100644 .changeset/nasty-files-type.md diff --git a/.changeset/nasty-files-type.md b/.changeset/nasty-files-type.md new file mode 100644 index 0000000000..b6b27d223d --- /dev/null +++ b/.changeset/nasty-files-type.md @@ -0,0 +1,5 @@ +--- +"@medusajs/modules-sdk": patch +--- + +fix(module-sdk): Shared modules loading should not share the entire core container but only the resources that are meant to be shared diff --git a/integration-tests/plugins/medusa-config.js b/integration-tests/plugins/medusa-config.js index 7cd5e423d1..59ec9bed54 100644 --- a/integration-tests/plugins/medusa-config.js +++ b/integration-tests/plugins/medusa-config.js @@ -47,14 +47,8 @@ module.exports = { }, productModuleService: { scope: "internal", - resources: "isolated", + resources: "shared", resolve: "@medusajs/product", - options: { - database: { - clientUrl: DB_URL, - debug: false, - }, - }, }, }, } diff --git a/packages/modules-sdk/src/loaders/__tests__/module-loader.ts b/packages/modules-sdk/src/loaders/__tests__/module-loader.ts index cc7f5e349b..8c8fa10282 100644 --- a/packages/modules-sdk/src/loaders/__tests__/module-loader.ts +++ b/packages/modules-sdk/src/loaders/__tests__/module-loader.ts @@ -23,7 +23,7 @@ describe("modules loader", () => { container = createMedusaContainer() }) - it("registers service as undefined in container when no resolution path is given", async () => { + it("should register the service as undefined in the container when no resolution path is given", async () => { const moduleResolutions: Record = { testService: { resolutionPath: false, @@ -52,7 +52,7 @@ describe("modules loader", () => { expect(testService).toBe(undefined) }) - it("registers service ", async () => { + it("should register the service ", async () => { const moduleResolutions: Record = { testService: { resolutionPath: "@modules/default", @@ -93,7 +93,7 @@ describe("modules loader", () => { expect(typeof testService).toEqual("object") }) - it("runs defined loaders and logs error", async () => { + it("should run the defined loaders and logs the errors if something fails", async () => { const moduleResolutions: Record = { testService: { resolutionPath: "@modules/brokenloader", @@ -121,7 +121,7 @@ describe("modules loader", () => { ) }) - it("logs error if no service is defined", async () => { + it("should log the errors if no service is defined", async () => { const moduleResolutions: Record = { testService: { resolutionPath: "@modules/no-service", @@ -149,7 +149,7 @@ describe("modules loader", () => { ) }) - it("throws error if no service is defined and module is required", async () => { + it("should throw an error if no service is defined and the module is required", async () => { expect.assertions(1) const moduleResolutions: Record = { testService: { @@ -181,7 +181,7 @@ describe("modules loader", () => { } }) - it("throws error if default package isn't found and module is required", async () => { + it("should throw an error if the default package isn't found and the module is required", async () => { expect.assertions(1) const moduleResolutions: Record = { testService: { @@ -213,7 +213,7 @@ describe("modules loader", () => { } }) - it("throws error if no scope is defined to the module", async () => { + it("should throw an error if no scope is defined on the module declaration", async () => { expect.assertions(1) const moduleResolutions: Record = { testService: { @@ -245,7 +245,7 @@ describe("modules loader", () => { } }) - it("throws error if resources is not set when scope is defined as internal", async () => { + it("should throw an error if the resources is not set when scope is defined as internal", async () => { expect.assertions(1) const moduleResolutions: Record = { testService: { diff --git a/packages/modules-sdk/src/loaders/utils/load-internal.ts b/packages/modules-sdk/src/loaders/utils/load-internal.ts index e2f78c122d..0511d0c105 100644 --- a/packages/modules-sdk/src/loaders/utils/load-internal.ts +++ b/packages/modules-sdk/src/loaders/utils/load-internal.ts @@ -69,24 +69,20 @@ export async function loadInternalModule( } } - const localContainer = - resources === MODULE_RESOURCE_TYPE.ISOLATED - ? createMedusaContainer() - : (container.createScope() as MedusaContainer) + const localContainer = createMedusaContainer() - if (resources === MODULE_RESOURCE_TYPE.ISOLATED) { - const moduleDependencies = resolution?.dependencies ?? [] + const dependencies = resolution?.dependencies ?? [] + if (resources === MODULE_RESOURCE_TYPE.SHARED) { + dependencies.push("manager", "configModule") + } - for (const dependency of moduleDependencies) { - localContainer.register( - dependency, - asFunction(() => { - return container.hasRegistration(dependency) - ? container.resolve(dependency) - : undefined - }) - ) - } + for (const dependency of dependencies) { + localContainer.register( + dependency, + asFunction(() => { + return container.resolve(dependency, { allowUnregistered: true }) + }) + ) } const moduleLoaders = loadedModule?.loaders ?? [] diff --git a/packages/product/src/loaders/connection.ts b/packages/product/src/loaders/connection.ts index 4d60943f89..d42d4f3237 100644 --- a/packages/product/src/loaders/connection.ts +++ b/packages/product/src/loaders/connection.ts @@ -12,7 +12,7 @@ import { EntitySchema } from "@mikro-orm/core" import * as ProductModels from "@models" import { createConnection } from "../utils" -import { ModulesSdkTypes } from "@medusajs/types" +import { ConfigModule, ModulesSdkTypes } from "@medusajs/types" export default async ( { @@ -28,7 +28,14 @@ export default async ( moduleDeclaration?.scope === MODULE_SCOPE.INTERNAL && moduleDeclaration.resources === MODULE_RESOURCE_TYPE.SHARED ) { - return + const { projectConfig } = container.resolve("configModule") as ConfigModule + options = { + database: { + clientUrl: projectConfig.database_url!, + driverOptions: projectConfig.database_extra!, + schema: projectConfig.database_schema!, + }, + } } const customManager = (