nicojs / typed-inject

Type safe dependency injection for TypeScript
Apache License 2.0
439 stars 23 forks source link

enhance needed: optional #2

Closed kosl90 closed 5 years ago

kosl90 commented 5 years ago

@nicojs you did a great job 👍!

I try to replace inversify with typed-inject, but it's failed. Because the optional injection is missed.

I think we could provide a API like provideOptional(name:string) which could still take advantage of TContext, or maybe we could just pass undefined into provideValue, provideClass, provideFactory. And it might be good for semantics that a function named optional for token.

nicojs commented 5 years ago

Thanks for your kind words 😅

I try to replace inversify with typed-inject, but it's failed. Because the optional injection is missed.

Would it be possible to request an optional like this:

class Foo {
  static inject = tokens('bar');
  constructor(public bar?: string /* Optional! */) { } 
}

// If 'bar' isn't known at injection time:
rootInjector
  .provideValue('bar', undefined)
  .injectClass(Foo)

// If 'bar' is known at injection time:
rootInjector
  .provideValue('bar', 'baz')
  .injectClass(Foo)

Something like provideOptional would not make sense in my opinion. At provide time, you should know whether or not you can provide the value, right? It is at declaration time that you don't know it.

We might be able to make an exception for values that are assignable to undefined. Meaning that missing tokens for undefined values would be allowed. Is this what you are aiming at?

kosl90 commented 5 years ago

I agree that provideOptional is not a good API, but I don't figure out a better solution so far 😅. It's the reason for

or maybe we could just pass undefined into provideValue, provideClass, provideFactory.

Optional injection looks like a troublemaker in this beautiful solution 😅, maybe we should think about it twice.

BTW, according to my test, passing undefined works for provideValue, but it should not work for provideClass, maybe provideFactory neither (I do not test provideFactory).

nicojs commented 5 years ago

BTW, according to my test, passing undefined works for provideValue, but it should not work for provideClass, maybe provideFactory neither (I do not test provideFactory).

For provideFactory it would be: .provideFactory('bar', () => undefined). This doesn't make sense for provideClass, agreed.

So the current workaround is to provide undefined explicitly. I'm willing to look into making undefined injection implicit, but it could also be viewed as a feature: by providing undefined explicitly you can never forget to provide it.

@kosl90

How do you feel about that?

kosl90 commented 5 years ago

Oops, it's embarrassing. I was supposed to express that 'passing undefined to provideClass should make sense' 😂.

The workaround about provideValue and provideFactory, argeed.

nicojs commented 5 years ago

Ok, if we document this workaround in the readme, do you agree that we can close this issue? Or do you want to explore the implicit way as well?

kosl90 commented 5 years ago

sure, it's ok to close this issue. we could add a comment when we get something new.