Binding cache discussion

[notaphplover]

As @tonyhallett commented, after #1132 is merged, a binding may have a promise to a service cached instead of a service.

Binding.cache type could be fixed to T | Promise<T> | null and I think it would be a good fix, but maybe we could improve the behavior.

As inversify user, I could expect to get a service syncronously if the binding is under a singleton scope and the resolution promise is already fullfilled. Even if the resolution is async, once the promise is fullfilled there is no reason to avoid accessing the cached value in a syncronous way. But there's a problem: if the service activation is asyncronous, we are caching a Promise. Maybe we should update the binding once the promise is resolved, that way users could access services syncronously.

@tonyhallett what are your thoughts about this?

[tonyhallett]

Maybe we should update the binding once the promise is resolved, that way users could access services syncronously.

Seems sound. Leave it up to the dev to decide but how do you know it is safe to get without waiting on the promise ?

If you can think of a use case where it is guaranteed to be resolved for subsequent requests for the service and avoiding promises is essential then sure.

[notaphplover]

Seems sound. Leave it up to the dev to decide but how do you know it is safe to get without waiting on the promise ?

Sometimes there's no way to know it, sometimes the use case allows the user to know it:

Consider the bootstrap of an HTTP server. Consider MongoDb as the database and mongodb as the driver chosen. I probably want to inject a MongoClient on singleton scope.

Consider the bindings are established at the very beginning of the bootstrap process. At this point we could be binding the client in this way:

import { Container } from 'inversify';
import { MongoClient } from 'mongodb';
 
export const mongoClientSymbol = Symbol('MongoClient'); 
export const container = new Container();
container.bind(mongoClientSymbol).toDynamicValue(MongoClient.connect(mongoDbConnectionUri)).inSingletonScope();

On this scenario we could know for sure the connection is established before any repository uses this client (as an example, we could be waiting for it before requesting the HTTP server to serve our endpoints).

On this scenario, the repository could safely @inject the MongoClient and later be instatiated calling container.get because we know for sure the promise should be fullfilled before any repository instance is instantiated. With the current behavior there is no way to this because the cache would contain a fullfilled promise of MongoClient instead of a MongoClient

If you can think of a use case where it is guaranteed to be resolved for subsequent requests for the service and avoiding promises is essential then sure.

I wouldn't say it's essential, but the developer experience is way better since we allow the user to use a syncronous context if he decides it and it's possible.

What are your thoughts about this @inversify/maintainers ?

[tonyhallett]

@notaphplover I say go for it ! It is a great idea

[tonyhallett]

If you are changing binding.cache then implementationType can be changed too
implementationType: T | Newable<T> | null;

[leonardssh]

I think this issue can be closed. #1328

[parisholley]

i know this comment is way too late, though I stumbled across this while researching a change i'm going to propose. as the origin async author, the enforcement of throwing a LAZY error was very intentional. if you allow developers to choose when to pull a dependency based on implicit environment state, you are going to introduce regression risk if in the future the behavior of how that dependency is built changes (imagine implementing middleware, conditional onactivation hooks, etc). imagine having a huge chunk of your code path done in a synchronous way only to find out you will have to rip all out and make it async.

IMO, if you are writing code that expects the mongo connection to always be up and available, you should bootstrap mongo first and inject in the container and not rely on async at all.