fix: Resolve circular dependencies in the SDK package#3361
Conversation
3f5566d to
1bb9784
Compare
There was a problem hiding this comment.
Pull request overview
This PR resolves circular dependencies in the SDK by implementing Symbol-based dependency injection tokens for the Resends and Subscriber classes. This approach eliminates the need for delay() wrappers and provides better ESM compatibility for future migration.
Changes:
- Introduces Symbol-based injection tokens for
ResendsandSubscriberclasses - Updates dependency injection to use tokens instead of
delay()wrappers - Converts class imports to type-only imports where the class is now injected via token
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/sdk/src/tokens.ts | Defines Symbol-based injection tokens for Resends and Subscriber |
| packages/sdk/src/setupTsyringe.ts | Registers Resends and Subscriber classes with their respective tokens using ContainerScoped lifecycle |
| packages/sdk/src/subscribe/MessagePipelineFactory.ts | Replaces delay(() => Resends) with token-based injection and adds type-only import for Resends |
| packages/sdk/src/encryption/SubscriberKeyExchange.ts | Updates Subscriber injection to use token instead of direct class reference and converts to type-only import |
| packages/sdk/src/StreamrClient.ts | Updates resolution of Resends and Subscriber to use tokens instead of class references |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Resolve circular dependencies in the SDK's dependency injection (DI) container by using token-based injection instead of direct class references. This eliminates the need for
delay()wrappers forResendsandSubscriberclasses.Important
This change is necessary for future ESM compatibility. The current
delay(() => Class)approach relies on CommonJS module semantics where circular imports can be partially resolved at runtime. In ESM, module bindings are evaluated differently anddelay()won't be sufficient to break circular dependency cycles.Token-based injection provides a clean solution that works in both module systems.
Changes
tokens.tswith Symbol-based injection tokens forResendsandSubscribersetupTsyringe.tsto registerResendsandSubscriberclasses with the DI container using tokens (withContainerScopedlifecycle)@scoped(Lifecycle.ContainerScoped)decorators with@injectable()forResendsandSubscriberclasses (lifecycle is now specified during token registration)MessagePipelineFactoryto use token injection forResendsinstead ofdelay(() => Resends)SubscriberKeyExchangeto use token injection forSubscriber(previously injected directly withoutdelay())StreamrClientto resolveSubscriberandResendsusing tokens instead of class referencesLimitations and future improvements
ResendsandSubscriberare currently registered via tokens; other classes with circular dependency potential could be migrated to this pattern in the futuredelay()wrapper is still used forStreamRegistryandGroupKeyManagerinMessagePipelineFactory; these could be converted to token injection for consistency