inversify / InversifyJS

A powerful and lightweight inversion of control container for JavaScript & Node.js apps powered by TypeScript.
http://inversify.io/
MIT License
11.2k stars 714 forks source link

Bind/inject a promise, not the value the promise resolves to. #1482

Open WORMSS opened 1 year ago

WORMSS commented 1 year ago

I want to inject a promise, not the value the promise resolves to.

Expected Behavior

const keyN = Symbol();
const keyP = Symbol();
container.bind(keyN).toDynamicValue(() => 42).inSingletonScope();
container.bind(keyP).toDynamicValue(() => functionThatReturnsAPromise()).inSingletonScope();
const myNumber = container.get(keyN); // Works.
const myPromise = container.get(keyP); // Throws.
... // some more stuff
const value = await Promise.all(myPromise, ...otherPromises);

Current Behavior

_getButThrowIfAsync throws an error. You are attempting to construct '" + key + "' in a synchronous way\n but it has asynchronous dependencies.

But, I don't want the Value the Promise returns.. I want THE PROMISE itself. The promise IS the value.

Possible Solution

a container.get() that bypasses _getButThrowIfAsync and gets the exact thing I want.

Steps to Reproduce (for bugs)

I have optionally commented out the types and a bunch of other boilerplate, but you get the idea.

const { Container } = require("inversify");
/* type DIKey<T> = {}; ideally it should extend Symbol, but that had problems in current TS version */

class DIBinder {
  constructor(container) {
    this.container = container;
  }
  bindConstant/*<T>*/(key/*: DIKey<T>*/, value/*: T*/) {
    this.container.bind(key).toConstantValue(value);
  }
  bindDynamic/*<T>*/(key/*: DIKey<T>*/, func/*: () => T*/) {
    this.container.bind(key).toDynamicValue(() => func()).inSingletonScope();
  }
}
class DIResolver {
  constructor(container) {
    this.container = container;
  }
  get/*<T>*/(key/*: DIKey<T>*/)/*: T */ {
    // (some of our own logic)
    return this.container.get(key);
  }
}
const container = new Container();
const diBinder = new DIBinder(container);
const diResolver = new DIResolver(container);

const keyN = Symbol(); /* as DIKey<number>; */
const keyP = Symbol(); /* as DIKey<Promise<number>>; */
diBinder.bindDynamic(keyN, () => Math.random() * 42); // Key knows function MUST supply a number.
diBinder.bindDynamic(keyP, async () => Math.random() * 42); // Key knows function MUST supply a Promise<number>.
const myNumber = diResolver.get(keyN); // Key knows get() returns a number;
const myPromise = diResolver.get(keyP); // Key knows get() SHOULD returns return Promise of number;

Context

We are midway through the process of changing over everything to 'real' inversify with actual injection from constructors and stuff, but it's a big codebase and we are nowhere near finished, but we wanted to start getting things setup and ready, and doing our best to use it with vue3 composibles. but we found that this "throwing if it's a promise" is slowing us down in our interim because we need to keep switching between get and getAsync when it really isn't needed at this point. We kind of need a container.get_Even_If_Its_A_Promise_I_Dont_Care(key) function. So everything can go through a single call on the container.

Your Environment

Stack trace

Jameskmonger commented 1 year ago

As a workaround, could you construct an object which contains the promise? { myPromise: Promise }

I haven't used Inversify in a while so YMMV

WORMSS commented 1 year ago

I did think about that, but there would be about 6,000 changes across 2500 files. And we would have to delicately identify which ones are async and which ones are not to change to destructuring I've been trying to keep the same signature to what we already have (but with a different implementation under the hood) and slowly transfer over to the class based/factory based inversify and eventually our diResolver.get() will reduce down to almost nothing. At that point I can hopefully remove the need for DIType<T> and juse straight up use ServiceIdentifier<T>

Jameskmonger commented 1 year ago

Hey @WORMSS, just to clarify your issue, you are converting from some other DI system which has a get(key) and you want to switch the implementation to Inversify, without needing to use the existing getAsync check?

If so, I'd be interested in putting a solution together for this.

I feel like we could add a parameter to ContainerOptions called throwOnAsyncInSync?: boolean which defaults to true to preserve existing behaviour. Then we can skip the getButThrowIfAsync call if the options say to.

@PodaruDragos @dcavanagh @inversify/maintainers welcome any thoughts before I make the PR. ty.

WORMSS commented 1 year ago

That would be a great solution, since it's at the container level, that way we can even have both systems working independently. This will make our migration much simpler as we transition to the full power of inversify.

I just thought, this is doubly convenient because we can use our abstraction layer to bind to both containers simultaneously but our current (soon to be legacy) resolver can retrieve ONLY from the lacks { throwOnAsyncInSync: false } and then we can use the 2nd container for the "proper" implementation.

bradley-marker-ck commented 1 year ago

We were able to bind a Promise and retrieve it with get() on Inversify v5. We had to change this to migrate to Inversify v6. For now, we went with binding a wrapper around the Promise instead of the Promise but an option to just return the Promise would have simplified our Inversify v5 to v6 upgrade quite a bit.

Jameskmonger commented 9 months ago

Added to milestone 6.1.0, we will aim to introduce this functionality in the next minor release. cc @PodaruDragos

WORMSS commented 9 months ago

That will be great. We actually paused our migration completely and "ONLY" use direct container.get() and nothing is using class providers yet. So this is great news for us. We have some very funky late loading logic which having the two different container with different options will be super beneficial.

Thank you for keeping this on the back burner getting us to this point.