Skip to content

Commit a8bd4ed

Browse files
Address PR review comments
- Extract activateCurrentSession() helper so start() and activateSession(for:) share one path - Add @mainactor doc comment to ServiceLocatorProvider explaining the isolation requirement - Add MARK comment to SessionProvider extension clarifying ServiceLocatorProvider conformance - Add comment in SceneDelegate explaining why @service can't be used for the dark mode subscription - Rename test variable login → loginSession for clarity Co-authored-by: Charles Wang <g-enius@users.noreply.github.com>
1 parent 1c40140 commit a8bd4ed

4 files changed

Lines changed: 23 additions & 13 deletions

File tree

Coordinator/Sources/Coordinator/AppCoordinator.swift

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,7 @@ public final class AppCoordinator: BaseCoordinator, SessionProvider {
5555
// MARK: - Start
5656

5757
override public func start() {
58-
session.activate()
59-
onSessionActivated?()
58+
activateCurrentSession()
6059
switch currentFlow {
6160
case .login:
6261
showLoginFlow(session: session)
@@ -67,13 +66,16 @@ public final class AppCoordinator: BaseCoordinator, SessionProvider {
6766

6867
// MARK: - Session Lifecycle
6968

69+
private func activateCurrentSession() {
70+
session.activate()
71+
onSessionActivated?()
72+
}
73+
7074
private func activateSession(for flow: AppFlow) -> Session {
7175
session.teardown()
72-
let newSession = sessionFactory.makeSession(for: flow)
73-
newSession.activate()
74-
session = newSession
75-
onSessionActivated?()
76-
return newSession
76+
session = sessionFactory.makeSession(for: flow)
77+
activateCurrentSession()
78+
return session
7779
}
7880

7981
// MARK: - Flow Management

Core/Sources/Core/ServiceLocator.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@ public class ServiceLocator {
6262
// MARK: - ServiceLocatorProvider
6363

6464
/// Any type that holds a ServiceLocator instance for instance-based DI resolution.
65+
///
66+
/// `@MainActor` is required because `ServiceLocator` itself is `@MainActor` —
67+
/// any property that returns a `ServiceLocator` must also be isolated to the main actor.
6568
@MainActor
6669
public protocol ServiceLocatorProvider {
6770
var serviceLocator: ServiceLocator { get }
@@ -76,6 +79,8 @@ public protocol SessionProvider: ServiceLocatorProvider {
7679
var session: Session { get }
7780
}
7881

82+
// MARK: - ServiceLocatorProvider
83+
7984
extension SessionProvider {
8085
public var serviceLocator: ServiceLocator { session.serviceLocator }
8186
}

FunApp/FunApp/SceneDelegate.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate {
6565

6666
private func subscribeToDarkMode() {
6767
guard let session = appCoordinator?.session else { return }
68+
// @Service can't be used here: SceneDelegate is not a ServiceLocatorProvider,
69+
// and the session (and its locator) changes on each transition — we must
70+
// re-resolve from the current session's locator on every activation.
6871
let toggles: FeatureToggleServiceProtocol = session.serviceLocator.resolve(for: .featureToggles)
6972
darkModeCancellable?.cancel()
7073
darkModeCancellable = toggles.appearanceModePublisher

Services/Tests/ServicesTests/SessionTests.swift

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,14 +85,14 @@ struct SessionTests {
8585
#expect(main.serviceLocator.isRegistered(for: .favorites))
8686

8787
main.teardown()
88-
let login = LoginSession()
89-
login.activate()
88+
let loginSession = LoginSession()
89+
loginSession.activate()
9090

9191
// Login session has only its own services — no stale favorites
92-
#expect(login.serviceLocator.isRegistered(for: .logger))
93-
#expect(login.serviceLocator.isRegistered(for: .network))
94-
#expect(login.serviceLocator.isRegistered(for: .featureToggles))
95-
#expect(!login.serviceLocator.isRegistered(for: .favorites))
92+
#expect(loginSession.serviceLocator.isRegistered(for: .logger))
93+
#expect(loginSession.serviceLocator.isRegistered(for: .network))
94+
#expect(loginSession.serviceLocator.isRegistered(for: .featureToggles))
95+
#expect(!loginSession.serviceLocator.isRegistered(for: .favorites))
9696
}
9797

9898
@Test("Favorites are fresh after session transition")

0 commit comments

Comments
 (0)