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

Singletons and OnActivation Not Working as Expected #1297

Closed
kferrone opened this issue Mar 8, 2021 · 4 comments
Closed

Singletons and OnActivation Not Working as Expected #1297

kferrone opened this issue Mar 8, 2021 · 4 comments

Comments

@kferrone
Copy link

kferrone commented Mar 8, 2021

To be clear, if I bind a constructor as a singleton, i.e. bind(THING).to(SomeClass).inSingletonScope();, this will work exactly as expected. The constructor is instantiated and cached in the container. This means the onActivation and constructor will only execute one time and get returns the single instance as it should.

My issue is I can only get this behavior to work when it is specifically a Class in singleton scope and nothing else. For the following toFactory, toConstantValue, toProvider, toFunction, toConstructor, basically everything else, the onActivation always runs any time you get the item.

Expected Behavior

In the Docs on Scope, it says toConstantValue is always inSingletonScope. It also says the factory and provider are singleton as well.

I expect the wrapped function around toFactory and toProvider to essentially run like a constructor would. So if the docs are correct about them being true singletons I would not expect the outer part to run every singe time I get the factory or provider.

for example I should not see the log output twice if a factory really is a singleton

container.bind("my-factory").toFactory((_ctx: interfaces.Context): interfaces.Factory<any> => {
  console.log("This should happen only once if singleton");
  return () => true;
}).onActivation((_ctx: interfaces.Context, myFactory: any) => {
      console.log("This should also happen only once if singleton");
      return myFactory;
});
const a = container.get("my-factory");
const b = container.get("my-factory");

I expect anything considered a singleton to run onActivation a single time, not on every single get.
Basically I expected the value returned by the factory to be stored in the cache, i.e. ()=>true

Current Behavior

Only a class bound as inSingletonScope will ever truly act like a singleton when it comes to onActivation. I have tried with everything else and every other option will run onActivation every single time I run a get or inject. This behavior is what I would call transient. The factory and provider are resolved on each and every request and onActivation is always run. With toConstantValue, toConstructor, and toFunction; the onActivation on every request makes the idea of activation kind of vague.

Possible Solution

Cache the result after onActivation just like a class would. Maybe introduce all three scopes as options on factory and provider injections.

for example this would make a lot of sense

container.bind("my-factory").toFactory((_ctx: interfaces.Context): interfaces.Factory<any> => {
  console.log("This should happen only once if singleton");
  return () => true;
}).inSingletonScope() //or inTransientScope
  .onActivation((_ctx: interfaces.Context, myFactory: any) => {
      console.log("This should also happen only once if singleton");
      return myFactory;
});

Steps to Reproduce (for bugs)

Try this binding

container.bind("message")
      .toConstantValue("Hello world")
      .onActivation((_ctx: interfaces.Context, message: any) => {
          console.log("I activated", message);
          return message;
        }
      );

Then get it a bunch of times and it always runs onActivation

const a = container.get("message");
const b = container.get("message");

Now if you do the same with a class, the onActivation only ever runs once.

@injectable()
class Foo {}
container.bind<Foo>("foo").to(Foo)
  .inSingletonScope()
  .onActivation((_ctx: interfaces.Context, foo: Foo) => {
      console.log("I activated", foo);
      return foo;
  });

Now try and get it twice and onActivation runs only once ever, as it should be

const a = container.get("foo");
const b = container.get("foo");

Context

I really wanted to be able to run code when something is accessed for the first time and only the first time. This way dependencies don't need to be available up until something is actually requested. For example, I wanted to activate a database connection the first time the client is accessed. The only way this is possible is if I wrap the underlying client in my own class and bind that as a singleton. Otherwise there is no other way in inversify to only run onActivation once.

When it is a class, I can activate in the constructor, postConstruct, or onActivation. They all work perfectly as expected.

Any deeper insight as to why only a class can be a true singleton?

@notaphplover
Copy link
Member

I'll have a look @kferrone :)

@notaphplover
Copy link
Member

@kferrone I've added a PR fixing this issue. @dcavanagh if you consider it a good fix I'll update the changelog and set the PR ready to be reviewed

@notaphplover
Copy link
Member

@kferrone the issue is fixed in the master branch! :)

@kferrone
Copy link
Author

You made me so very happy! We refactored our apps with this change and everything is so smooth and clear now. I have tested this heavily and I can verify it works well.

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