Skip to content

Commit

Permalink
PR feedback 1
Browse files Browse the repository at this point in the history
  • Loading branch information
ncipollo committed Oct 16, 2024
1 parent 8fd7ed3 commit fc21f40
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 24 deletions.
27 changes: 13 additions & 14 deletions Sources/WhoopDIKit/Container/Container.swift
Original file line number Diff line number Diff line change
Expand Up @@ -60,28 +60,27 @@ public final class Container {
public func inject<T>(_ name: String? = nil,
params: Any? = nil,
_ localDefinition: (DependencyModule) -> Void) -> T {
guard !isLocalInjectActive else {
fatalError("Nesting WhoopDI.inject with local definitions is not currently supported")
}

isLocalInjectActive = true
defer {
isLocalInjectActive = false
localDependencyGraph.resetDependencyGraph()
}
// We need to maintain a reference to the local service dictionary because transient dependencies may also
// need to reference dependencies from it.
// ----
// This is a little dangerous since we are mutating a variable but it should be fine as long as you
// don't use `inject { }` within the scope of another `inject { }`.
return localDependencyGraph.aquireDependencyGraph { localServiceDict in
// Nested local injects are not currently supported. Fail fast here.
guard !isLocalInjectActive else {
fatalError("Nesting WhoopDI.inject with local definitions is not currently supported")
}

isLocalInjectActive = true
defer {
isLocalInjectActive = false
localDependencyGraph.resetDependencyGraph()
}

let localModule = DependencyModule()
localDefinition(localModule)
localModule.addToServiceDictionary(serviceDict: localServiceDict)

do {
return try get(name, params)
} catch {
print("Inject failed with stack trace:")
Thread.callStackSymbols.forEach { print($0) }
fatalError("WhoopDI inject failed with error: \(error)")
}
}
Expand Down
15 changes: 6 additions & 9 deletions Tests/WhoopDIKitTests/Container/ContainerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import Testing
@testable import WhoopDIKit

// This is unchecked Sendable so we can run our local inject concurrency test
@Suite(.serialized)
class ContainerTests: @unchecked Sendable {
private let container: Container

Expand Down Expand Up @@ -49,17 +48,15 @@ class ContainerTests: @unchecked Sendable {
// performing a local inject. 1000 times should do the trick.
container.registerModules(modules: [GoodTestModule()])

async let resultA = Task {
let _: Dependency = container.inject("C_Factory") { module in
Task.detached {
let _: Dependency = self.container.inject("C_Factory") { module in
module.factory(name: "C_Factory") { DependencyA() as Dependency }
}
}.result

async let resultB = Task {
let _: DependencyA = container.inject()
}.result
}

let _ = await [resultA, resultB]
Task.detached {
let _: DependencyA = self.container.inject()
}
}

@Test
Expand Down
1 change: 0 additions & 1 deletion Tests/WhoopDIKitTests/WhoopDITests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import Testing

@Suite(.serialized)
class WhoopDITests {

deinit {
WhoopDI.removeAllDependencies()
}
Expand Down

0 comments on commit fc21f40

Please sign in to comment.