Skip to content
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

Can't provide a class before its dependencies #13

Open
jeremiahkellick opened this issue Mar 8, 2020 · 2 comments
Open

Can't provide a class before its dependencies #13

jeremiahkellick opened this issue Mar 8, 2020 · 2 comments

Comments

@jeremiahkellick
Copy link

Take a look at this example code. With other IoC containers this would be perfectly valid, but in typed-inject this is an error.

class Foo {
  static inject = tokens('bar', 'baz');

  constructor(bar: string, baz: string) {}
}

const foo = rootInjector
  .provideValue('bar', 'bar value')
  .provideClass('foo', Foo)
  .provideValue('baz', 'baz value')
  .resolve('foo');

Currently this is considered an error at the provideClass call because the dependencies for that class are not satisfied. However, in my opinion, it should not check for the dependencies until you try to resolve it. Of course, the following should result in a compilation error:

class Foo {
  static inject = tokens('bar', 'baz');

  constructor(bar: string, baz: string) {}
}

const foo = rootInjector
  .provideValue('bar', 'bar value')
  .provideClass('foo', Foo)
  .resolve('foo');

However, I think the error here is with the resolve call, not the provideClass call. It's perfectly fine that you want to specify which class should be used when the 'foo' token is requested. However, it is not okay to try to resolve a 'foo' before all its dependencies are satisfied.

Take a look at this example of InversifyJS code. The Warrior is bound before its dependencies, Weapon and ThrowableWeapon. Now of course InversifyJS is not type safe. But after some experimentation, I'm quite confident that it would be possible to achieve this behavior while retaining 100% type safety and with the errors all being compilation errors, not runtime errors.

One major difference between Inversify and typed-inject is that typed-inject actually creates a new injector every time something is provided. I personally love this about typed-inject. However, there are some interesting implications for providing classes before their dependencies.

Take a look at this example:

const injector = rootInjector.provideClass('foo', Foo);

It will now be impossible for injector to ever resolve 'foo'. And if you were to later on provide the dependencies for 'foo', 'bar' and 'baz'. It would still be impossible for this injector instance to ever resolve 'foo'. Only the new child injector would be able to resolve 'foo'.

To me, this is not a bug, it's a feature. This is still a useful state for an injector to be in. For example, you could pass injector to some sub-module, and if the sub-module should want an instance of 'foo', it just needs to provide two strings. It does not need to know what specific implementation of 'foo' to use. This allows sub-modules of your app to provide configuration for certain services, without coupling those sub-modules to any specific implementation of those services.

Think about it as if the token has three states in the injector

  1. Unknown: The token has never been provided. The injector doesn't know it exists.
  2. Bound: The token has been bound to a certain class or factory. However, the injector is not capable of resolving this token because some of its dependencies have not been provided. The token will implicitly transition to the provided state in any child injectors where the dependencies are satisfied.
  3. Provided: All the dependencies of the token are satisfied, and the injector is capable of resolving it

To avoid changing the behavior of existing methods out from under people, new methods could be written that have this new behavior, bindFactory and bindClass. That way provide would still cause compilation errors if the dependencies are not satisfied, but if you want to specify which class should be used for a token before the dependencies are determined, you can use bindClass.

To further illustrate how these different states work take a look at this example.

const injector = rootInjector.bindClass('foo', Foo);

const childInjectorA = injector
  .provideValue('bar', 'bar value A');
  .provideValue('baz', 'baz value A');

const childInjectorB = injector
  .provideValue('bar', 'bar value B');
  .provideValue('baz', 'baz value B');

const fooA = childInjectorA.resolve('foo');

const fooB = childInjectorB.resolve('foo');

fooA and fooB are different instances of Foo. This is because while 'foo' was bound all the way up in injector it wasn't actually provided until the child injectors satisfied the dependencies.

I would be happy to attempt to write a PR for this myself, but first I wanted to get your feedback, Nico. Do you think this would be a good improvement to typed-inject?

@nicojs
Copy link
Owner

nicojs commented Mar 9, 2020

Hi @jeremiahkellick 👋 thanks for stopping by.

This issue largely describes the behavior of typed inject, this is all by design (as you also pointed out).

But after some experimentation, I'm quite confident that it would be possible to achieve this behavior while retaining 100% type safety

I'm less confident. Making it work at run-time is not a big problem, but compile time will be a hurdle. If it were possible, the error message would be very cryptic, as it is not yet possible to plugin to the compiler and provide different error messages. It also would make the code less elegant. I'm not saying no yet, I'll have to think about it more.

It would also have some interesting consequences. For example, what happens here?

class Foo {
  static inject = tokens('bar', 'baz');

  constructor(bar: string, baz: string) {}
}

const injector = rootInjector.bindClass('foo', Foo);

const childInjectorA = injector
  .provideValue('bar', 'bar value A');
  .provideValue('baz', 'baz value A')
  .provideValue('bar', 'Bar override. Which value is now used?');

const fooA = childInjectorA.resolve('foo');

A workaround with the current version of typed-inject:

function createFoo(bar: string, baz: string) {
  return new Foo(bar, baz);
}

const injector = rootInjector.provideValue('fooCreater', createFoo);

const childInjectorA = injector
  .provideValue('bar', 'bar value A')
  .provideValue('baz', 'baz value A');

function fooFactory(bar: string, baz: string, fooCreator: typeof createFoo){
  return createFoo(bar, baz);
}

fooFactory.inject = tokens('bar', 'baz', 'fooCreater')
const fooA = childInjectorA.injectFactory(fooFactory);

@jeremiahkellick
Copy link
Author

Yeah, the workaround isn't too bad. I'm still reasonably confident I could get this to work at compile time. I got a very basic experiment working locally. But there's no getting around the cryptic error message issue, although it remains to be seen exactly how cryptic it would be. The best we could do there is add documentation for error message translations like "if you see X it likely means Y"

I'm curious to see if anyone else is interested in this behavior. If it's just me, then I might as well use a workaround or different system. I say we let this issue sit for now and see if it gets any interest. If you're reading this would like to see this behavior, please add a +1 reaction to the OP.

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

No branches or pull requests

2 participants