feat: add support for extending the framework with custom modules#832
feat: add support for extending the framework with custom modules#832marcinkrasowski merged 6 commits intomainfrom
Conversation
…mework Enable developers to define new base modules beyond the core modules using `createModule()` and `ApiConfig.customModules`. Includes `registerCustomModules()` helper, example documents module in mocked integration, and documentation guide. (cherry picked from commit accf9ab)
WalkthroughAdds a createModule() factory and a new documents base module (package + mocked integration), wires the documents module into the NestJS AppModule, adds generator templates for custom modules, and updates documentation and configs to support custom framework modules. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App Module
participant Factory as createModule()
participant DI as DI Container
participant Abstract as DocumentService<br/>(abstract)
participant Impl as MockedDocumentService
participant Ctrl as DocumentController
participant Client as HTTP Client
App->>Factory: createModule('documents') / register(config)
Factory-->>App: DynamicModule class
App->>DI: import Module
DI->>DI: Module.register(config) -> provide token config.service -> useClass config.serviceImpl
DI->>Impl: instantiate MockedDocumentService
DI->>Ctrl: instantiate DocumentController (inject DocumentService token)
Client->>Ctrl: GET /documents
Ctrl->>Abstract: call getDocumentList(query, authorization)
Abstract->>Impl: resolved to MockedDocumentService
Impl->>Impl: mapDocuments(query) -> produces Observable<Documents>
Impl-->>Ctrl: return Observable
Ctrl-->>Client: HTTP 200 + payload
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/integrations/mocked/src/modules/documents/documents.mapper.ts (1)
54-60: Consider throwingNotFoundExceptionfor proper HTTP 404 response.The generic
Errorthrown here will propagate as a 500 Internal Server Error to clients. Per the relevant code snippet,MockedDocumentService.getDocument()calls this directly without wrapping, so the raw error reaches the controller.Using NestJS's
NotFoundExceptionprovides a proper 404 response with a meaningful error message.♻️ Proposed fix
+import { NotFoundException } from '@nestjs/common'; import * as Model from './documents.model'; // ... MOCK_DOCUMENTS ... export const mapDocument = (id: string): Model.Document => { const document = MOCK_DOCUMENTS.find((doc) => doc.id === id); if (!document) { - throw new Error(`Document with id ${id} not found`); + throw new NotFoundException(`Document with id ${id} not found`); } return document; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked/src/modules/documents/documents.mapper.ts` around lines 54 - 60, The mapDocument function currently throws a generic Error when a document isn't found; change it to throw NestJS's NotFoundException so controllers return HTTP 404s: import NotFoundException from `@nestjs/common` and replace the throw in mapDocument (used by MockedDocumentService.getDocument) with throw new NotFoundException(`Document with id ${id} not found`) so the proper HTTP response and message are produced.packages/configs/integrations/src/models/custom-modules.ts (1)
1-3: Consider making the custom modules config more flexible.This file directly re-exports
CustomModulesfrom the mocked integration, which tightly couples the configs package to that specific integration. For a production setup, developers will need to replace this with their own custom modules config.Consider adding a comment clarifying this is a default/example configuration that should be customized:
import { CustomModules } from '@o2s/integrations.mocked/integration'; +// Default custom modules config from the mocked integration. +// Replace with your own custom modules when using a different integration. export const CustomModulesConfig = CustomModules;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/configs/integrations/src/models/custom-modules.ts` around lines 1 - 3, Replace the direct re-export of CustomModules from the mocked integration by making CustomModulesConfig explicitly a default/example placeholder and adding a clarifying comment; update the top of the file that currently imports CustomModules and exports CustomModulesConfig to instead document that CustomModulesConfig is an example default (pointing to the mocked integration symbol CustomModules) and should be replaced by consumers with their own modules — keep the exported name CustomModulesConfig so downstream code keeps compatibility but ensure the comment clearly states this file is a template for production overrides.packages/integrations/mocked/src/modules/documents/documents.service.mocked.ts (1)
15-21: Method signatures omit theauthorizationparameter from the abstract service.The abstract
DocumentServicedefines both methods with an optionalauthorization?: stringparameter (seedocuments.service.tslines 10-12), but this implementation omits them. While this works in TypeScript (optional params can be omitted in overrides), it would be clearer to include them for consistency, especially if a future non-mocked implementation needs authorization.♻️ Suggested fix to include authorization parameter
- getDocumentList(query: Model.GetDocumentListQuery): Observable<Model.Documents> { + getDocumentList(query: Model.GetDocumentListQuery, _authorization?: string): Observable<Model.Documents> { return of(mapDocuments(query)).pipe(responseDelay()); } - getDocument(params: Model.GetDocumentParams): Observable<Model.Document> { + getDocument(params: Model.GetDocumentParams, _authorization?: string): Observable<Model.Document> { return of(mapDocument(params.id)).pipe(responseDelay()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked/src/modules/documents/documents.service.mocked.ts` around lines 15 - 21, The mocked service methods getDocumentList and getDocument omit the optional authorization parameter present in the abstract DocumentService; update the signatures of getDocumentList(query: Model.GetDocumentListQuery, authorization?: string) and getDocument(params: Model.GetDocumentParams, authorization?: string) in documents.service.mocked.ts to include the optional authorization?: string parameter (you can ignore it inside the mocked implementations or pass it through if needed) so the mock matches the abstract service contract and remains consistent for future non-mocked implementations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/docs/docs/guides/integrations/extending-framework-modules.md`:
- Around line 23-31: Update the fenced code block that shows the package
directory listing to include a language specifier for plain text (e.g., use
```text or ```plaintext) so it renders correctly; locate the fenced block
containing the tree starting with
"packages/integrations/mocked/src/modules/documents/" in the README/guide (the
block that lists documents.model.ts, documents.service.ts, etc.) and change the
opening fence from ``` to ```text (or ```plaintext).
In `@packages/framework/src/utils/create-module.ts`:
- Around line 18-42: The `@Global`() decorator on the CustomModule class is dead
code for dynamic modules; remove the decorator from the createModule factory so
globality is controlled only via the DynamicModule return value. Edit the
createModule function and delete the `@Global`() line applied to class
CustomModule (leave `@Module` and the static register(config: CustomModuleConfig):
DynamicModule implementation unchanged) and ensure the returned object still
sets global: config.global !== false.
---
Nitpick comments:
In `@packages/configs/integrations/src/models/custom-modules.ts`:
- Around line 1-3: Replace the direct re-export of CustomModules from the mocked
integration by making CustomModulesConfig explicitly a default/example
placeholder and adding a clarifying comment; update the top of the file that
currently imports CustomModules and exports CustomModulesConfig to instead
document that CustomModulesConfig is an example default (pointing to the mocked
integration symbol CustomModules) and should be replaced by consumers with their
own modules — keep the exported name CustomModulesConfig so downstream code
keeps compatibility but ensure the comment clearly states this file is a
template for production overrides.
In `@packages/integrations/mocked/src/modules/documents/documents.mapper.ts`:
- Around line 54-60: The mapDocument function currently throws a generic Error
when a document isn't found; change it to throw NestJS's NotFoundException so
controllers return HTTP 404s: import NotFoundException from `@nestjs/common` and
replace the throw in mapDocument (used by MockedDocumentService.getDocument)
with throw new NotFoundException(`Document with id ${id} not found`) so the
proper HTTP response and message are produced.
In
`@packages/integrations/mocked/src/modules/documents/documents.service.mocked.ts`:
- Around line 15-21: The mocked service methods getDocumentList and getDocument
omit the optional authorization parameter present in the abstract
DocumentService; update the signatures of getDocumentList(query:
Model.GetDocumentListQuery, authorization?: string) and getDocument(params:
Model.GetDocumentParams, authorization?: string) in documents.service.mocked.ts
to include the optional authorization?: string parameter (you can ignore it
inside the mocked implementations or pass it through if needed) so the mock
matches the abstract service contract and remains consistent for future
non-mocked implementations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c1905d1e-57b1-4881-84b2-23ef883e9f8f
📒 Files selected for processing (22)
apps/api-harmonization/src/app.config.tsapps/api-harmonization/src/app.module.tsapps/docs/docs/guides/integrations/extending-framework-modules.mdapps/docs/docs/guides/integrations/overview.mdapps/docs/docs/integrations/commerce/medusa-js/how-to-setup.mdapps/docs/docs/integrations/commerce/medusa-js/usage.mdapps/docs/docs/main-components/harmonization-app/module-structure.mdpackages/configs/integrations/src/models/custom-modules.tspackages/configs/integrations/src/models/index.tspackages/framework/src/api-config.tspackages/framework/src/index.tspackages/framework/src/utils/create-module.tspackages/framework/src/utils/register-custom-modules.tspackages/integrations/mocked/package.jsonpackages/integrations/mocked/src/integration.tspackages/integrations/mocked/src/modules/documents/documents.controller.tspackages/integrations/mocked/src/modules/documents/documents.mapper.tspackages/integrations/mocked/src/modules/documents/documents.model.tspackages/integrations/mocked/src/modules/documents/documents.service.mocked.tspackages/integrations/mocked/src/modules/documents/documents.service.tspackages/integrations/mocked/src/modules/documents/index.tspackages/integrations/mocked/src/modules/index.ts
Coverage Report for packages/configs/vitest-config
File CoverageNo changed files found. |
|
@marcinkrasowski Should we also add a changelist file for these changes? |
added |
… split for custom modules
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/modules/documents/src/documents.model.ts (1)
20-30: Consider adding class-validator decorators for runtime validation.The request DTOs (
GetDocumentListQuery,GetDocumentParams) define the shape but lack runtime validation. Adding decorators like@IsOptional(),@IsInt(),@IsString()would enable NestJS'sValidationPipeto enforce constraints.♻️ Example with validation decorators
import { IsInt, IsOptional, IsString, IsIn } from 'class-validator'; import { Type } from 'class-transformer'; export class GetDocumentListQuery { `@IsOptional`() `@Type`(() => Number) `@IsInt`() offset?: number; `@IsOptional`() `@Type`(() => Number) `@IsInt`() limit?: number; `@IsOptional`() `@IsIn`(['CONTRACT', 'REPORT', 'POLICY', 'STATEMENT']) type?: DocumentType; // ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/modules/documents/src/documents.model.ts` around lines 20 - 30, Add runtime validation decorators to the DTOs so NestJS ValidationPipe can enforce constraints: on GetDocumentListQuery annotate offset and limit with `@IsOptional`(), `@Type`(() => Number) and `@IsInt`(); annotate search with `@IsOptional`() and `@IsString`(); annotate type and status with `@IsOptional`() and `@IsIn`([...]) using your DocumentType/DocumentStatus values; on GetDocumentParams annotate id with `@IsString`() or `@IsUUID`() and `@IsNotEmpty`(); also import the needed decorators from class-validator and Type from class-transformer and update exports accordingly.packages/integrations/mocked/src/modules/documents/documents.service.ts (1)
18-20: Consider wrapping synchronous errors in Observable error handling.Per the context from
documents.mapper.ts,mapDocument()throws a synchronousErrorwhen the document isn't found. Since this occurs beforeof()creates the Observable, the error propagates as a thrown exception rather than an Observable error stream.For consistency with RxJS patterns and to allow downstream error handling via
catchError(), consider usingdefer():♻️ Proposed refactor for consistent Observable error handling
-import { Observable, of } from 'rxjs'; +import { defer, Observable, of } from 'rxjs';getDocument(params: Model.GetDocumentParams): Observable<Model.Document> { - return of(mapDocument(params.id)).pipe(responseDelay()); + return defer(() => of(mapDocument(params.id))).pipe(responseDelay()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked/src/modules/documents/documents.service.ts` around lines 18 - 20, getDocument currently calls mapDocument(params.id) synchronously inside of(), so any synchronous Error thrown by mapDocument (from documents.mapper.ts) escapes as a thrown exception; wrap the call in an Observable factory (e.g., use RxJS defer or another lazy creator) so mapDocument is executed when subscribed and any thrown error is emitted as an Observable error stream, then apply responseDelay() as before; update the implementation in getDocument to call defer(() => of(mapDocument(params.id))).pipe(responseDelay()) (or equivalent) so downstream operators like catchError() can handle the error.packages/modules/documents/src/documents.controller.ts (1)
14-20: Consider adding validation for query parameters.The
GetDocumentListQuerycontainsoffsetandlimitparameters that arrive as strings from the query string. Without aValidationPipeorParseIntPipe, these may need explicit transformation to numbers.If the framework uses a global validation pipe, this is already handled. Otherwise, consider adding class-validator decorators to the model or using
@Query(new ValidationPipe({ transform: true })).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/modules/documents/src/documents.controller.ts` around lines 14 - 20, The getDocumentList controller accepts Model.GetDocumentListQuery where offset/limit arrive as strings; ensure they are validated/transformed to numbers by either adding class-validator/class-transformer decorators to the GetDocumentListQuery DTO (e.g., `@IsInt`(), `@Type`(() => Number) on offset/limit) or by applying a ValidationPipe with transform enabled to the `@Query` (e.g., `@Query`(new ValidationPipe({ transform: true })) query: Model.GetDocumentListQuery), or use ParseIntPipe on individual params; update the getDocumentList signature and/or DTO so documentService.getDocumentList always receives numeric offset/limit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@turbo/generators/templates/custom-module/service.hbs`:
- Around line 10-18: The template's type names for collection endpoints are
inconsistent: update the return types and related model references used in the
abstract methods get{{pascalCase name}}List and get{{pascalCase name}} to use
the pluralized model name (e.g., change Model.{{pascalCase name}}List to
Model.{{pascalCase name}}s) so generated modules match the example Documents
pattern; locate the declarations of get{{pascalCase name}}List, get{{pascalCase
name}}, and any other occurrences of Model.{{pascalCase name}}List in the
template and replace them with the plural form Model.{{pascalCase name}}s (and
adjust any corresponding type names like response types) to align with REST
collection naming.
---
Nitpick comments:
In `@packages/integrations/mocked/src/modules/documents/documents.service.ts`:
- Around line 18-20: getDocument currently calls mapDocument(params.id)
synchronously inside of(), so any synchronous Error thrown by mapDocument (from
documents.mapper.ts) escapes as a thrown exception; wrap the call in an
Observable factory (e.g., use RxJS defer or another lazy creator) so mapDocument
is executed when subscribed and any thrown error is emitted as an Observable
error stream, then apply responseDelay() as before; update the implementation in
getDocument to call defer(() =>
of(mapDocument(params.id))).pipe(responseDelay()) (or equivalent) so downstream
operators like catchError() can handle the error.
In `@packages/modules/documents/src/documents.controller.ts`:
- Around line 14-20: The getDocumentList controller accepts
Model.GetDocumentListQuery where offset/limit arrive as strings; ensure they are
validated/transformed to numbers by either adding
class-validator/class-transformer decorators to the GetDocumentListQuery DTO
(e.g., `@IsInt`(), `@Type`(() => Number) on offset/limit) or by applying a
ValidationPipe with transform enabled to the `@Query` (e.g., `@Query`(new
ValidationPipe({ transform: true })) query: Model.GetDocumentListQuery), or use
ParseIntPipe on individual params; update the getDocumentList signature and/or
DTO so documentService.getDocumentList always receives numeric offset/limit.
In `@packages/modules/documents/src/documents.model.ts`:
- Around line 20-30: Add runtime validation decorators to the DTOs so NestJS
ValidationPipe can enforce constraints: on GetDocumentListQuery annotate offset
and limit with `@IsOptional`(), `@Type`(() => Number) and `@IsInt`(); annotate search
with `@IsOptional`() and `@IsString`(); annotate type and status with `@IsOptional`()
and `@IsIn`([...]) using your DocumentType/DocumentStatus values; on
GetDocumentParams annotate id with `@IsString`() or `@IsUUID`() and `@IsNotEmpty`();
also import the needed decorators from class-validator and Type from
class-transformer and update exports accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 31c23dc8-b813-426a-850f-89b52ce51aac
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (40)
.changeset/social-mammals-beam.mdapps/api-harmonization/src/app.module.tsapps/docs/docs/guides/integrations/adding-new-integrations.mdapps/docs/docs/guides/integrations/extending-framework-modules.mdapps/docs/docs/guides/integrations/overview.mdapps/docs/docs/guides/using-generators.mdapps/docs/docs/main-components/harmonization-app/module-structure.mdpackages/configs/integrations/src/models/documents.tspackages/configs/integrations/src/models/index.tspackages/framework/src/index.tspackages/framework/src/utils/create-module.tspackages/integrations/mocked/package.jsonpackages/integrations/mocked/src/modules/documents/documents.mapper.tspackages/integrations/mocked/src/modules/documents/documents.service.tspackages/integrations/mocked/src/modules/documents/index.tspackages/modules/documents/.gitignorepackages/modules/documents/.prettierrc.mjspackages/modules/documents/eslint.config.mjspackages/modules/documents/lint-staged.config.mjspackages/modules/documents/package.jsonpackages/modules/documents/src/documents.controller.tspackages/modules/documents/src/documents.model.tspackages/modules/documents/src/documents.service.tspackages/modules/documents/src/index.tspackages/modules/documents/tsconfig.jsonpackages/modules/documents/turbo.jsonpackages/modules/documents/vitest.config.mjsturbo/generators/config.tsturbo/generators/templates/custom-module/controller.hbsturbo/generators/templates/custom-module/eslint.config.hbsturbo/generators/templates/custom-module/gitignore.hbsturbo/generators/templates/custom-module/index.hbsturbo/generators/templates/custom-module/lint-staged.config.hbsturbo/generators/templates/custom-module/model.hbsturbo/generators/templates/custom-module/package.hbsturbo/generators/templates/custom-module/prettierrc.hbsturbo/generators/templates/custom-module/service.hbsturbo/generators/templates/custom-module/tsconfig.hbsturbo/generators/templates/custom-module/turbo.hbsturbo/generators/templates/custom-module/vitestConfig.hbs
✅ Files skipped from review due to trivial changes (25)
- apps/docs/docs/guides/integrations/adding-new-integrations.md
- packages/modules/documents/lint-staged.config.mjs
- packages/modules/documents/vitest.config.mjs
- packages/configs/integrations/src/models/index.ts
- packages/modules/documents/tsconfig.json
- packages/modules/documents/.gitignore
- apps/docs/docs/guides/integrations/overview.md
- packages/modules/documents/src/index.ts
- turbo/generators/templates/custom-module/gitignore.hbs
- turbo/generators/templates/custom-module/turbo.hbs
- turbo/generators/templates/custom-module/vitestConfig.hbs
- turbo/generators/templates/custom-module/lint-staged.config.hbs
- packages/configs/integrations/src/models/documents.ts
- .changeset/social-mammals-beam.md
- apps/docs/docs/main-components/harmonization-app/module-structure.md
- apps/docs/docs/guides/using-generators.md
- packages/modules/documents/turbo.json
- turbo/generators/templates/custom-module/eslint.config.hbs
- packages/modules/documents/.prettierrc.mjs
- turbo/generators/templates/custom-module/tsconfig.hbs
- packages/modules/documents/package.json
- apps/docs/docs/guides/integrations/extending-framework-modules.md
- packages/framework/src/utils/create-module.ts
- turbo/generators/templates/custom-module/model.hbs
- turbo/generators/templates/custom-module/package.hbs
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/integrations/mocked/package.json
- packages/framework/src/index.ts
- packages/integrations/mocked/src/modules/documents/index.ts
- packages/integrations/mocked/src/modules/documents/documents.mapper.ts
- apps/api-harmonization/src/app.module.ts
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Summary
createModule()factory to@o2s/frameworkthat generates NestJS dynamic modules from config, enabling developers to define new base modules (e.g. documents, reports, warranties) beyond the core framework modulescustom-moduleTurbo generator that scaffolds a complete module package inpackages/modules/Motivation
The O2S framework provides a fixed set of core modules (invoices, tickets, orders, etc.). Developers can extend existing modules but cannot add entirely new domain-specific modules that follow the same abstract-service/swappable-integration pattern. This change closes that gap.
How it works
Custom modules follow the same architecture as core modules:
Module definition (
packages/modules/<name>/) — contains the abstract service, normalized data model, REST controller, and barrel export. This is the reusable contract, analogous to what@o2s/frameworkprovides for core modules.Integration implementation (
packages/integrations/<integration>/src/modules/<name>/) — contains the concrete service implementation and data mappers, analogous to how the mocked integration implements core modules.Registration — custom modules are registered directly in
app.module.tsusingcreateModule(), following the same pattern as the SurveyJS module:Block consumption — blocks import custom module services from
@o2s/configs.integrationsexactly like core services, using the sameuseExistingDI pattern.Changes
Framework (
packages/framework/)createModule()factory atsrc/utils/create-module.ts— generates a@Module({})class withregister()static method, mirroring core modules likeInvoiceModule@o2s/framework/modulesalongside existing core modulesExample module (
packages/modules/documents/)DocumentService,Documentmodel,DocumentController, and full package config (tsconfig, eslint, prettier, vitest, turbo)Mocked integration (
packages/integrations/mocked/)documents/module withMockedDocumentServiceimplementation and mock data mapper@o2s/modules.documents, exports concrete implementation@o2s/modules.documentsas dependency, updatedturbo.jsonbuild orderingConfig (
packages/configs/integrations/)documents.tsre-exportingService,MockedService,Controller,Modelfrom the mocked integration — follows the same pattern as core module configs likeinvoices.tsAPI Harmonization (
apps/api-harmonization/)DocumentsBaseModuleviacreateModule()inapp.module.tsGenerator (
turbo/generators/)custom-modulegenerator with 12 templates scaffolding a complete module package atpackages/modules/<name>/Documentation (
apps/docs/)guides/integrations/extending-framework-modules.md— complete walkthrough covering file structure, step-by-step manual setup, generator usage, multi-integrationsupport, block consumption, and frontend SDK extension
guides/integrations/overview.md,guides/integrations/adding-new-integrations.md,guides/using-generators.md,main-components/harmonization-app/module-structure.mdTest plan
npm run buildpasses (all 51 tasks)npm run lintpasses for framework, mocked integration, configsnpm run testpasses for framework (100% coverage) and mocked integrationnpm run generate→ selectcustom-module→ verify package scaffolded correctlynpm run dev→ verifyGET /documentsandGET /documents/:idendpoints respond with mock dataDocuments.Servicefrom@o2s/configs.integrations