-
Notifications
You must be signed in to change notification settings - Fork 1
Refactor Ioctopus for Enhanced Type Safety and Type Inference #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…cover test case for container
Hi @mildronize, First of all, thank you so much for your efforts — I really appreciate it! I've already started working on a backward-compatible version that supports an optional registry. Someone asked me about it a few days ago on a Discord channel, so I got a head start on it. I chose to go with a simple, class-free syntax for the registry without too many changes in the original code, and it’s already working. I just need to migrate a few of my existing projects before releasing it to be sure that it is fully backward-compatible. type Registry = {
'SIMPLE_FUNCTION': SayHelloType;
// Other deps types
};
const container = createContainer<Registry>();
// and still provide the original createContainer() for people already using it
// Then
container.bind('SIMPLE_FUNCTION').toFunction(sayHelloWorld); // string + symbol compatible
const sayHello = container.get('SIMPLE_FUNCTION'); // string + symbol compatible
expect(sayHello()).toBe('hello world'); // => sayHello is of type SayHelloType I expect to have it ready by today or tomorrow as everything is already done. Just need to play with on a real project before. |
@Evyweb Great to hear you’re already on it — really appreciate the update! I’m curious about how you’re handling the creation of With the Would you mind sharing how you’re managing symbol-based tokens in this new setup without duplicating code or sacrificing type safety? Thanks again for your hard work on this — I’m looking forward to seeing how it evolves! |
I think I understand your point better now. From what I gather, my proposal suggests using a registry class pattern where strings are used as references to symbol objects. In that sense, it’s similar to using strings as tokens. When you mentioned keeping the API consistent (i.e., not depending on the registry object as I proposed), it implies that if we use symbols without a registry, we lose type safety and the user would need to manually provide types. However, your approach shows that a type registry can provide type-safe mappings for string tokens and help reduce code duplication. Did I understand that correctly? |
Sorry for the late answer. I called it a registry but I was just talking about how to type the dependencies. I wasn't talking about the pattern.* In fact I'm trying to provide a simple way to have/keep:
const container = createContainer();
container.bind('SIMPLE_FUNCTION').toFunction(sayHelloWorld);
const sayHello = container.get<SayHelloType>('SIMPLE_FUNCTION'); // => type: SayHelloType
const container = createContainer<{
'SIMPLE_FUNCTION': SayHelloType
}>();
container.bind('SIMPLE_FUNCTION').toFunction(sayHelloWorld);
const sayHello = container.get('SIMPLE_FUNCTION'); // => type: SayHelloType
// di.ts
export const SIMPLE_FUNCTION = Symbol('SIMPLE_FUNCTION');
// then
import * as DI from "./DI";
const container = createContainer<{
[DI.SIMPLE_FUNCTION]: SayHelloType
}>();
container.bind(DI.SIMPLE_FUNCTION).toFunction(sayHelloWorld);
const sayHello = container.get(DI.SIMPLE_FUNCTION); // => type: SayHelloType => It is ok for my usage but I understand that people may want something simplier. For now I will keep it like this I think (this version ✅).
My current constraints are that:
I added a check mark on the points that are currently working on my local. |
@Evyweb Thank you for the clarification — great improvement! I have a few thoughts I'd love to explore further:
|
Hi, Evyweb team,
First of all, I truly appreciate your work on Ioctopus, especially its approach to dependency injection without relying on the Reflection API. However, I noticed some opportunities to enhance type safety and reduce code duplication.
Problem Statement
Currently, when using
@evyweb/ioctopus
, we define dependencies as follows:The challenge here is that the current API lacks type safety, especially when binding dependencies. We rely on Symbols, but there is no strict type enforcement on what each dependency requires. This can lead to runtime errors that TypeScript cannot catch.
Proposed Solution
I have refactored the entire project to introduce type-safe dependency management with type inference. Instead of manually creating Symbols, we define dependencies using a ServiceRegistry that automatically maps each dependency to a specific type.
New Approach
Benefits of This Approach
Final Thoughts
I understand that API design is a matter of preference, and you may or may not accept this PR. However, regardless of the outcome, I still plan to maintain and publish a type-safe version of this approach as an alternative package.
Thank you for your hard work on Ioctopus—I genuinely appreciate your contributions to dependency injection in TypeScript.
My Test Result
I've also attached the test result with 100% coverage test