Enhancment: Overrides in scoped containers

[zveljkovic]

Hi and thank you for wonderful library. I have a Logger class which logs startup of app and some other things. Inside scoped container (per request) I generate TraceId and currently I need to have two loggers. One that accepts TraceId and one that does not, otherwise all the same.

I tried registering Logger in main container with injected traceId (value "app") but then when tried to overwrite the Logger in scoped container it still returns the main instance and not the one with proper traceId.

Can we have some enhancement, that when we instantiate from a scoped container that we first search it and then main container? Or that dispose cache removes only one registration? And one suggestion for addition is that we can have .optional() when registering object which in my case will put the traceId as undefined when resolved.

EDIT: I have figured out a solution. Use one Logger class with traceId injected in main container and in scope container register it under different name.

[jeffijoe]

So in the docs it says

Be careful! If a scope's parent has already resolved a scoped value, that value will be returned.

let counter = 1
container.register({
  counterValue: asFunction(() => counter++).scoped()
})
const scope1 = container.createScope()
const scope2 = container.createScope()

container.cradle.counterValue === 1

// These are resolved to the parent's cached value.
scope1.cradle.counterValue === 1
scope1.cradle.counterValue === 1
scope2.cradle.counterValue === 1
scope2.cradle.counterValue === 1

To be honest I don't even know why I made that decision. I'll investigate, thanks!

EDIT: I think I did that as a precaution against resolving the same thing over and over in deeply nested scopes, but this was before I even released Awilix v2 that I made that decision and after having used it for a long time I think it's safe to say that that isn't going to be a problem.

And one suggestion for addition is that we can have .optional()

This is already supported, but only in CLASSIC mode which uses default parameter values to signal optional values.

class Logger {
  constructor(traceId = null) {}
}

Alternatively you can register({ traceId: asValue(null) }) in the root container and override it in the scope.

[jeffijoe]

Just peeked at the tests, and the current behavior does seem like it is/was desired.

    it('uses parents cache when scoped', function() {
      const container = createContainer()
      let scopedCounter = 1
      container.register({
        scoped: asFunction(() => scopedCounter++).scoped()
      })

      const scope1 = container.createScope()
      expect(scope1.cradle.scoped).toBe(1)
      expect(scope1.cradle.scoped).toBe(1)

      const scope2 = scope1.createScope()
      expect(scope2.cradle.scoped).toBe(1)
      expect(scope2.cradle.scoped).toBe(1)

      expect(container.cradle.scoped).toBe(2)
      expect(container.cradle.scoped).toBe(2)
      expect(scope2.cradle.scoped).toBe(1)
    })

If I change that behavior it's a breaking change and I'd have to push a new major version. I would like to avoid that for now, because I'm toying with the idea of customizing lifetimes further, such as adding an option to control this specific behavior, which would also be a breaking API change.

I'm going to keep this issue open though, thank you for bringing it up!

[jeffijoe]

I'm just realizing that by making this change, resolving a singleton using a scoped container would not reuse the singleton — that would be bad.

[zveljkovic]

Just wanted to say thank you for your hard work on this library. Really appreciated.

[jeffijoe]

@zveljkovic thank you for that, you just made my day! 😄

[jeffijoe]

I just realized what you need to do. Register your top level logger as scoped as well! That should give you exactly the behavior you want! The top level logger will be used when no other logger has been resolved in a child scope. 😄

Scratch that, the top level one would be used anyway.

However, I think if you re-register your logger in the scoped container under the same name, then it should work the way you want

[jeffijoe]

I think I was wrong about my comment regarding singletons being broken by making this change. I will keep this in mind for sure. It does make more sense that each scope maintains its own cache rather than look up the tree. It would be more deterministic and I can also get rid of an internal family tree set, making it way simpler.

Once again, this means incrementing major version as it is a breaking change.

[henrikra]

@jeffijoe Incrementing major version is not a problem when you have proper release notes ;)

[jeffijoe]

@henrikra Wasn't sure how much interest there was in this, so thanks for commenting! I'll try to get this done when I have some time 😀

[henrikra]

Btw just reading your article on Medium. It is fu***ing awesome 🎉

[jeffijoe]

@henrikra thank you sir! 🙏