Breaking change from 5.0.5 to 5.1.1 when using factories and named bindings

[psoares-resilient]

Expected Behavior

Container retrieves the named resources properly

Current Behavior

Named binding using factories on 5.0.5 worked as expected. When updating to 5.1.1 only the first named binding resolved is retrieved no matter what named is used afterwards

Possible Solution

/shrug

Steps to Reproduce (for bugs)

The example given might not be architecturally perfect but gives a pretty good idea of what/how we do and what has gone wrong:

README tells you how to run and expectations. The code inside is commented to try and clear some aspects of it. https://github.com/7jpsan/inversify-factory-issue

Same on the browser... (Only difference on the links are the versions of inversify) https://stackblitz.com/edit/inversify-factory-break-505?file=index.ts https://stackblitz.com/edit/inversify-factory-break-511?file=index.ts

Context

We have a bit of a setup running in production. I have tried to simplify as much of it as I could to try and isolate the problem on the example while keeping the setup close to how we actually have it. It uses named bindings on "top level" to inject the right dependencies into the lower levels. For that we make use of factories and some name finding on the injection tree. We publish it as part of a set of libraries that expose a libBindings method that allows you to bind multiple instances of something in a fluid fashion, taking complexity of binding out of the client and into the lib itself e.g:

dsBindings(container)
      .default({
        region: 'DEFAULT',
        tableName: 'DEFAULT',
      })
      .named('XXX', {
        region: 'XXX',
        tableName: 'XXX',
      })
      .named('YYY', {
        region: 'YYY',
        tableName: 'YYY',
      });;

When a particular service is needed, the client can just inject @named('XXX') in the outer service and everything inside will be based on that.

Your Environment

• Version used: 5.0.5 and 5.1.1
• Environment name and version: node14 (npm7), node12(npm6), chrome 90
• Operating System and version: macOS desktop

[tonyhallett]

https://github.com/inversify/InversifyJS/commit/b315ae96f8e70a9b3f52bed9c7a21d9f4f4e4e05

@notaphplover

[tonyhallett]

@psoares-resilient git bisect with your test

[tonyhallett]

The reason why your Engine example does not work is that now the factory of engine options is now a singleton and as such it has already determined the target name for engine options for all subsequent invocations.

After the commit that introduced the change, the InTransientScope method is not available through the syntax. You would have to access the binding and set the scope on that. Not advisable.

_binding.scope = BindingScopeEnum.Transient;

Alternatively you could bind your factory of engine options through ToDynamicValue instead.

[psoares-resilient]

The reason why your Engine example does not work is that now the factory of engine options is now a singleton and as such it has already determined the target name for engine options for all subsequent invocations.

After the commit that introduced the change, the InTransientScope method is not available through the syntax. You would have to access the binding and set the scope on that. Not advisable.

_binding.scope = BindingScopeEnum.Transient;

Alternatively you could bind your factory of engine options through ToDynamicValue instead.

Thanks!

My concern was that the behaviour was for a breaking change being marked as a minor. Turns out it was a bug fix and the link to the update binding_to_syntax to singleton scope according to docs makes it clear now. We unintentionally relied on a bug and thus breaking for us.

I'll stay clear from the guts of it and update the factory to a dynamic value instead. Thanks again for looking into it. I'll close it now.