Conversation
- Add optional parameter to ResourceConfig interface
- Integrate namespace auto-detection in defineResource function
- Update createEventsGetter to use resolved namespace/resourceName
- Update store key generation to use {namespace}/{resourceName} pattern
- Maintain backward compatibility with existing resource definitions
- Add comprehensive tests for namespace functionality
Closes #46
Task 3 of Sprint 2.5: Resource Definition Updates
- ✅ Resource config interface updated with namespace parameter
- ✅ Event generation uses resolved namespace pattern {namespace}.{resource}.{action}
- ✅ Store key generation uses pattern {namespace}/{resource}
- ✅ All 600 tests passing with namespace integration
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR implements namespace parameter support for the defineResource function as part of Task 3 Sprint 2.5. It adds optional namespace configuration with auto-detection capabilities and explicit override support while maintaining full backward compatibility.
- Adds optional
namespaceparameter toResourceConfiginterface with auto-detection fallback - Updates event generation to use resolved namespace in
{namespace}.{resourceName}.{action}pattern - Updates store key generation to use
{namespace}/{resourceName}pattern - Implements namespace resolution logic supporting explicit override, shorthand syntax, and auto-detection
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/kernel/src/resource/types.ts | Adds optional namespace parameter to ResourceConfig interface with comprehensive JSDoc documentation |
| packages/kernel/src/resource/grouped-api.ts | Updates createEventsGetter to use resolved namespace and resource name from config |
| packages/kernel/src/resource/define.ts | Implements namespace resolution logic and integrates it throughout the resource definition process |
| packages/kernel/src/resource/tests/grouped-api-factories.test.ts | Updates tests to match new createEventsGetter signature with namespace parameter |
| packages/kernel/src/resource/tests/define.test.ts | Adds comprehensive test coverage for namespace scenarios including auto-detection, explicit override, and shorthand syntax |
Comment on lines
+38
to
+45
| // If name also contains colon syntax, parse the resource name part | ||
| // This handles the edge case where both are provided | ||
| if (config.name.includes(':')) { | ||
| const [, resourceName] = config.name.split(':', 2); | ||
| if (resourceName) { | ||
| return { namespace: config.namespace, resourceName }; | ||
| } | ||
| } |
There was a problem hiding this comment.
The destructuring assignment const [, resourceName] skips the first element but doesn't validate it exists. Consider using const parts = config.name.split(':', 2); const resourceName = parts[1]; for clearer intent and easier debugging.
Comment on lines
+51
to
+54
| const [namespace, resourceName] = config.name.split(':', 2); | ||
| if (namespace && resourceName) { | ||
| return { namespace, resourceName }; | ||
| } |
There was a problem hiding this comment.
Consider extracting the namespace parsing logic into a separate helper function since it's used in two places (lines 40-44 and 50-53). This would reduce duplication and make the parsing logic more testable.
Addresses review feedback from PR #53: - Extract duplicate namespace parsing logic into parseNamespaceFromString() helper - Improve destructuring clarity by using explicit parts array - Reduce code duplication and improve testability - Better separation of concerns between parsing and resolution logic All 723 tests continue to pass, maintaining full backward compatibility.
pipewrk
added a commit
that referenced
this pull request
Nov 8, 2025
Addresses review feedback from PR #53: - Extract duplicate namespace parsing logic into parseNamespaceFromString() helper - Improve destructuring clarity by using explicit parts array - Reduce code duplication and improve testability - Better separation of concerns between parsing and resolution logic All 723 tests continue to pass, maintaining full backward compatibility.
pipewrk
added a commit
that referenced
this pull request
Nov 8, 2025
…ion-updates feat: add namespace parameter support to defineResource (Task 3 Sprint 2.5)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements Task 3 of Sprint 2.5: Resource Definition Updates - adding namespace parameter support to
defineResourcefunction with auto-detection and explicit override capabilities.Changes Made
✅ Resource Config Interface Updated
namespace?: stringparameter toResourceConfig<T, TQuery>interface✅ Event Generation Updated
createEventsGetterto use resolved namespace from parent controller{namespace}.{resourceName}.{action}✅ Store Key Generation Updated
{namespace}/{resourceName}(e.g.,my-plugin/job)✅ Integration & Namespace Resolution
resolveNamespaceAndName()helper function indefine.tsthat handles:{ namespace: 'my-plugin', name: 'job' }{ name: 'my-plugin:job' }{ name: 'job' }→ usesgetNamespace()detectionArchitecture Decisions
✅ Minimal API Surface Changes
createEventsGettersignature changed to accept resolved values from parentconfigparameterdefine.tshandles resolution, passes clean data down✅ Avoiding Dangerous Patterns
ResourceConfigstructure - namespace parameter was the right place for this dataTest Coverage
define.test.ts:namespace:namesyntaxcreateEventsGettersignatureAcceptance Criteria Met
✅ Resource config interface updated: Optional
namespaceparameter added with full TypeScript support✅ Event generation updated: Uses resolved namespace, follows
{namespace}.{resource}.{action}pattern✅ Store key generation updated: Uses
{namespace}/{resource}pattern with proper registration✅ Integration tests: All existing tests pass + new namespace-specific test coverage
Breaking Changes
None - This is fully backward compatible. Existing resource definitions continue to work exactly as before.
Next Steps
This completes Task 3 of Sprint 2.5. The namespace detection system (
getNamespace()) from Task 2 will be integrated separately to provide the auto-detection capabilities.Closes #46