Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for “namespaced” (per-feature) configuration slices in ConfigKit, including DI tokens, a namespace registry for duplicate detection, and a consumer-facing injection decorator.
Changes:
- Exported new public API:
defineNamespace,NamespacedConfig,InjectConfig, andDuplicateNamespaceError. - Introduced namespace DI token generation (
getNamespaceToken) and a shared registry token (NAMESPACE_REGISTRY_TOKEN). - Updated
ConfigModule.register()/registerAsync()to provide + export the namespace registry for duplicate detection.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/index.ts | Exposes the new namespaced-config APIs from the package entrypoint. |
| src/define-namespace.ts | Implements defineNamespace() / NamespacedConfig.asProvider() and the duplicate-namespace error. |
| src/decorators/inject-config.decorator.ts | Adds @InjectConfig(namespace) as a thin wrapper over token-based injection. |
| src/constants.ts | Adds namespace token generator + registry token constant. |
| src/config.module.ts | Provides/exports the namespace registry so feature providers can detect duplicates. |
| * 3. ConfigModule (via forRoot / register) parses ALL namespaced schemas as | ||
| * part of the same validation pass, so misconfigured slices also prevent | ||
| * the app from booting. |
There was a problem hiding this comment.
The module-level doc says ConfigModule parses “ALL namespaced schemas as part of the same validation pass”, but the current implementation parses each namespace inside NamespacedConfig.asProvider() (via definition.parse(process.env)). Either update the documentation to match the per-provider parsing behavior, or move namespaced-slice validation into ConfigModule so all schemas are validated together as described.
| * 3. ConfigModule (via forRoot / register) parses ALL namespaced schemas as | |
| * part of the same validation pass, so misconfigured slices also prevent | |
| * the app from booting. | |
| * 3. Each `NamespacedConfig` validates its own schema when | |
| * `NamespacedConfig.asProvider()` is resolved, so a misconfigured slice | |
| * still prevents the app from booting during startup. |
| * export class DatabaseModule {} | ||
| * | ||
| * // database/database.service.ts | ||
| * constructor(@InjectConfig('database') private cfg: typeof dbConfig.schema._output) {} |
There was a problem hiding this comment.
The example type typeof dbConfig.schema._output is incorrect: NamespacedConfig doesn’t expose a schema property (it’s under dbConfig.definition.schema), and _output isn’t the recommended public type surface. The example should reference the schema via definition.schema and use z.output<...> (or an exported helper type) so consumers can copy/paste it without type errors.
| * constructor(@InjectConfig('database') private cfg: typeof dbConfig.schema._output) {} | |
| * constructor(@InjectConfig('database') private cfg: z.output<typeof dbConfig.definition.schema>) {} |
| * The namespace name must be unique across the entire application — attempting | ||
| * to call `asProvider()` on two configs with the same name will throw | ||
| * `DuplicateNamespaceError` at module init. |
There was a problem hiding this comment.
defineNamespace() docs (and DuplicateNamespaceError message) state namespaces must be unique across the entire application, but duplicate detection is implemented via a Set<string> provided by the nearest ConfigModule instance. If an app imports ConfigModule more than once (e.g., multiple register() calls), duplicates across those module graphs won’t be detected. Consider clarifying the scope in docs or making the registry truly app-wide.
| // A fresh Set is created per ConfigModule instance. | ||
| const namespaceRegistryProvider: Provider = { | ||
| provide: NAMESPACE_REGISTRY_TOKEN, | ||
| useValue: new Set<string>(), | ||
| }; | ||
|
|
||
| return { |
There was a problem hiding this comment.
The namespace registry is created as new Set<string>() per ConfigModule.register() / registerAsync() call. That means duplicate namespace detection only works within the scope of a single ConfigModule instance; importing ConfigModule multiple times will create multiple registries and allow duplicate namespaces across module graphs. If app-wide uniqueness is required, consider using a shared singleton registry (or enforce a single ConfigModule import) and document the expected usage.
Summary
Why
Checklist
npm run lintpassesnpm run typecheckpassesnpm testpassesnpm run buildpassesnpx changeset) if this affects consumersNotes