Turn Knit.Resolver into a class type #303
Open
+108
−469
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
skorulis-ap
force-pushed
the
skorulis/resolver-class
branch
16 times, most recently
from
49549d2 to
cf40012
Compare
skorulis-ap
commented
Aug 26, 2025
| @@ -58,18 +58,6 @@ public final class ScopedModuleAssembler<TargetResolver> { | |||
| behaviors: [Behavior] = [], | |||
| postAssemble: ((Container<TargetResolver>) -> Void)? = nil | |||
| ) throws { | |||
| // For provided modules, fail early if they are scoped incorrectly | |||
There was a problem hiding this comment.
This isn't needed since the TargetResolver check is already done at compile time.
skorulis-ap
force-pushed
the
skorulis/resolver-class
branch
2 times, most recently
from
90def33 to
6c38f05
Compare
bradfol
reviewed
Aug 28, 2025
| let resolver = r.resolve(Container<TargetResolver>.self)! as! TargetResolver | ||
| let resolver = r.resolve(Container<TargetResolver>.self)!.resolver |
There was a problem hiding this comment.
Ah I must have missed this one previously
bradfol
reviewed
Aug 28, 2025
bradfol
reviewed
Aug 28, 2025
bradfol
reviewed
Aug 28, 2025
bradfol
reviewed
Aug 28, 2025
bradfol
reviewed
Aug 28, 2025
| public func unsafeResolver(file: StaticString, function: StaticString, line: UInt) -> Swinject.Resolver { | ||
| _unwrappedSwinjectContainer(file: file, function: function, line: line) | ||
| } | ||
| public let resolver: TargetResolver |
There was a problem hiding this comment.
Does keeping this property access mean all the existing Container.register methods remain the same?
bradfol
reviewed
Aug 28, 2025
| @@ -40,9 +40,8 @@ public extension ModuleAssembly { | |||
| static var replaces: [any ModuleAssembly.Type] { [] } | |||
|
|
|||
| static func scoped(_ dependencies: [any ModuleAssembly.Type]) -> [any ModuleAssembly.Type] { | |||
There was a problem hiding this comment.
Can we remove static func scoped( from the public protocol now? Seems like it is no longer the extension point
There was a problem hiding this comment.
I've moved this into a private function. There's no need for it to be overridden anymore
skorulis-ap
force-pushed
the
skorulis/resolver-class
branch
2 times, most recently
from
08993fc to
e06231c
Compare
skorulis-ap
force-pushed
the
skorulis/resolver-class
branch
3 times, most recently
from
28f6855 to
4c867ff
Compare
skorulis-ap
force-pushed
the
skorulis/resolver-class
branch
from
4c867ff to
f645ad9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This changes how Resolvers are created so rather than every container conforming to every Resolver, there are concrete instances of Resolvers allowing correct inheritance.
For some reason this really pushed the Swift compiler causing multiple crashes. The top level Resolver must remain as a protocol otherwise type equality checks cause the compiler to crash.