hmlongco / Resolver

Swift Ultralight Dependency Injection / Service Locator framework
MIT License
2.14k stars 187 forks source link

Incorrect (reversed) container hierarchy #133

Closed ZsoltMolnarMBH closed 2 years ago

ZsoltMolnarMBH commented 2 years ago

Resolver version: current master Example:

let A = Resolver()
let B = Resolver(parent: A)

Expected: A.childContainers contains B

Experienced: B.childContainers contains A

Suspicious source code in Resolver.swift: https://github.com/hmlongco/Resolver/blob/0f2f48449e1b0a2a64d44586a7b84cca972c2368/Sources/Resolver/Resolver.swift#L73 The signature of the init function suggests self and parent should be swapped.

hmlongco commented 2 years ago

I tend to agree, but this initializer dates back to Resolver 1.0, and swapping the order now would break all of the existing code that uses the current behavior for mocking and testing.

Basically the "parent" was the next container in the chain to be searched if the registration wasn't found in the current container.

This was extended somewhat later on to allow for multiple child containers to be searched, and the same mechanism was used (perhaps inappropriately) to support the old-style initializer.

Hmmm... I wonder if an existing container (and all of its children) should be searched first before bumping back up the chain to the parent.

ZsoltMolnarMBH commented 2 years ago

Now I can see that the Container hierarchy has fundamentally changed in v1.4.3. I think the way to fix this, is to change this initializer signature, and raise the major version number. The version number would be especially a good indicator for projects using Resolver, that they should maybe rethink how they use the library. (Edit: Fix typo)

hmlongco commented 2 years ago

I changed the signature and updated the build number, but deprecated the old initializer so existing code still works until it's change. I did remove the formerly deprecated scopes.