Skip to content

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

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

mildronize
Copy link

@mildronize mildronize commented Mar 20, 2025

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:

export type CurriedFunctionWithDependencies = (name: string) => string;
export const curriedFunctionWithDependencies =
    (dep1: string): CurriedFunctionWithDependencies => (name: string) => `Hello ${name} with ${dep1}`;

// Step 1: Create DI Symbols
export const DI: InjectionTokens = {
    DEP1: Symbol('DEP1'),
    DEP2: Symbol('DEP2'),
    HIGHER_ORDER_FUNCTION_WITH_DEPENDENCIES: Symbol('HIGHER_ORDER_FUNCTION_WITH_DEPENDENCIES'),
};

// Step 2: Create Container
const container = createContainer();

// Step 3: Bind Dependencies
container.bind(DI.DEP1).toValue('dependency1');
container.bind(DI.DEP2).toValue(42);
container.bind('CURRIED_FUNCTION_WITH_DEPENDENCIES')
  .toCurry(curriedFunctionWithDependencies, [DI.DEP1]);

// Step 4: Retrieve Dependencies
const myService = container.get<MyServiceInterface>(DI.HIGHER_ORDER_FUNCTION_WITH_DEPENDENCIES);

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

// Step 1: Define Service Registry with Explicit Types
export const serviceRegistry = new ServiceRegistry()
    .define('DEP1').mapTo<string>()
    .define('DEP2').mapTo<number>()
    .define('CURRIED_FUNCTION_WITH_DEPENDENCIES').mapTo<CurriedFunctionWithDependencies>();

// Step 2: Create Container with Registry
const container = createContainer(serviceRegistry);
container.bind('DEP1').toValue('dependency1');
container.bind('DEP2').toValue(42);
container.bind('CURRIED_FUNCTION_WITH_DEPENDENCIES')
  .toCurry(curriedFunctionWithDependencies, ['DEP1']);

// Step 3: Retrieve Dependencies
const myService = container.get('CURRIED_FUNCTION_WITH_DEPENDENCIES');

Benefits of This Approach

  • No Need for Symbols: The ServiceRegistry automatically maps dependencies, eliminating unnecessary object creation.
  • Strong Type Safety: TypeScript enforces correct types at compile-time, reducing potential runtime errors.
  • Simplified API: The new design removes boilerplate code and improves readability.
  • Immediate Type Validation: If a dependency is bound incorrectly (e.g., passing a number instead of a string), TypeScript will throw an error during development.

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

image

@Evyweb
Copy link
Owner

Evyweb commented Mar 22, 2025

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.

@mildronize
Copy link
Author

mildronize commented Mar 22, 2025

@Evyweb Great to hear you’re already on it — really appreciate the update!

I’m curious about how you’re handling the creation of InjectionTokens that need to be defined before binding with dependency parameters. From my experience, using strings and symbols offers different trade-offs. Symbols tend to be more unique and less error-prone in larger codebases — though please correct me if I’m mistaken.

With the Registry pattern, I assume it’s currently optimized for string-based tokens, especially to support better auto-completion and type inference. However, if we plan to use symbols as injection tokens, it seems we’d need to define a separate mapping object to get things working — which might lead to some code duplication, similar to the current version of the library.

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!

@mildronize
Copy link
Author

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?

@Evyweb
Copy link
Owner

Evyweb commented Mar 23, 2025

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:

  • createContainer without deps types in params so you need to type during the get + use string as keys ✅
const container = createContainer();
container.bind('SIMPLE_FUNCTION').toFunction(sayHelloWorld);
const sayHello = container.get<SayHelloType>('SIMPLE_FUNCTION'); // => type: SayHelloType
  • createContainer with deps types so you don't need to type during the get (new way) ✅
const container = createContainer<{
      'SIMPLE_FUNCTION': SayHelloType
}>();
container.bind('SIMPLE_FUNCTION').toFunction(sayHelloWorld);
const sayHello = container.get('SIMPLE_FUNCTION'); // => type: SayHelloType
  • use symbols as keys (old way) ✅
  • use symbols as keys (new way) but as you mentioned it need more boilerplate, at the moment that's what I have:
// 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 ✅).

  • autocompletion on bind method if you passed the deps types when creating the container ✅

  • error on bind method if you passed the deps types when creating the container ✅

  • no autocompletion on bind method if you don't passed the deps types when creating the container ✅

  • autocompletion on get method if you passed the deps types when creating the container ✅

  • error on get method if you passed the deps types when creating the container ✅

  • no autocompletion on get method if you don't passed the deps types when creating the container ✅

My current constraints are that:

  • I don't want to break the existing behaviors as the library is already used in production. (no v2) ✅
  • I don't want to mix OOP/classes with functional approach as people asked for functional (even if I'm a OOP guy personally) ✅
  • If possible I don't want to introduce a new api with functions to call or chain, I would prefer to add types only ✅

I added a check mark on the points that are currently working on my local.
I think I will take more time to think about the approach I want to take for the lib. I will take time this week to read again your proposition and play with it.

@thada-ttss
Copy link

@Evyweb Thank you for the clarification — great improvement!

I have a few thoughts I'd love to explore further:

  • It seems we might not need a class here; I believe it could be refactored to a functional style. I'll need a bit more time to experiment with this.

  • I also think it's possible to keep the API consistent as you suggested. Using ServiceRegistry (as in my proposal) could remain an optional approach to declare types. However, ensuring type support without changing the API is quite challenging for me. Initially, I considered breaking the existing API, but if I have more time, I’d like to try modifying the types while preserving the current API structure.

  • As you can see in my unit tests and README.md, I’ve shifted from using symbol to string. While the internal logic can still accept symbol, type support for it hasn’t been fully implemented in my proposal yet. That said, the public API remains unchanged — the ServiceRegistry class is merely a helper to support typing in a more convenient way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants