-
-
Notifications
You must be signed in to change notification settings - Fork 152
fix(realtime): make Push associated to MainActor #721
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -32,13 +32,15 @@ | |||||||||
var pushes: [String: PushV2] = [:] | ||||||||||
} | ||||||||||
|
||||||||||
private let mutableState = LockIsolated(MutableState()) | ||||||||||
@MainActor | ||||||||||
private var mutableState = MutableState() | ||||||||||
Check warning on line 36 in Sources/Realtime/RealtimeChannelV2.swift
|
||||||||||
|
||||||||||
let topic: String | ||||||||||
let config: RealtimeChannelConfig | ||||||||||
let logger: (any SupabaseLogger)? | ||||||||||
let socket: RealtimeClientV2 | ||||||||||
var joinRef: String? { mutableState.joinRef } | ||||||||||
|
||||||||||
@MainActor var joinRef: String? { mutableState.joinRef } | ||||||||||
|
||||||||||
let callbackManager = CallbackManager() | ||||||||||
private let statusSubject = AsyncValueSubject<RealtimeChannelStatus>(.unsubscribed) | ||||||||||
|
@@ -81,6 +83,7 @@ | |||||||||
} | ||||||||||
|
||||||||||
/// Subscribes to the channel | ||||||||||
@MainActor | ||||||||||
public func subscribe() async { | ||||||||||
if socket.status != .connected { | ||||||||||
if socket.options.connectOnSubscribe != true { | ||||||||||
|
@@ -109,7 +112,7 @@ | |||||||||
) | ||||||||||
|
||||||||||
let joinRef = socket.makeRef() | ||||||||||
mutableState.withValue { $0.joinRef = joinRef } | ||||||||||
mutableState.joinRef = joinRef | ||||||||||
|
||||||||||
logger?.debug("Subscribing to channel with body: \(joinConfig)") | ||||||||||
|
||||||||||
|
@@ -497,8 +500,8 @@ | |||||||||
filter: filter | ||||||||||
) | ||||||||||
|
||||||||||
mutableState.withValue { | ||||||||||
$0.clientChanges.append(config) | ||||||||||
Task { @MainActor in | ||||||||||
mutableState.clientChanges.append(config) | ||||||||||
} | ||||||||||
Comment on lines
+503
to
505
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since the enclosing context is already on the MainActor, consider appending directly to mutableState.clientChanges instead of dispatching via a Task, to simplify execution and ensure immediate consistency.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||
|
||||||||||
let id = callbackManager.addPostgresCallback(filter: config, callback: callback) | ||||||||||
|
@@ -538,32 +541,28 @@ | |||||||||
self.onSystem { _ in callback() } | ||||||||||
} | ||||||||||
|
||||||||||
@MainActor | ||||||||||
@discardableResult | ||||||||||
func push(_ event: String, ref: String? = nil, payload: JSONObject = [:]) async -> PushStatus { | ||||||||||
let push = mutableState.withValue { | ||||||||||
let message = RealtimeMessageV2( | ||||||||||
joinRef: $0.joinRef, | ||||||||||
ref: ref ?? socket.makeRef(), | ||||||||||
topic: self.topic, | ||||||||||
event: event, | ||||||||||
payload: payload | ||||||||||
) | ||||||||||
|
||||||||||
let push = PushV2(channel: self, message: message) | ||||||||||
if let ref = message.ref { | ||||||||||
$0.pushes[ref] = push | ||||||||||
} | ||||||||||
let message = RealtimeMessageV2( | ||||||||||
joinRef: joinRef, | ||||||||||
ref: ref ?? socket.makeRef(), | ||||||||||
topic: self.topic, | ||||||||||
event: event, | ||||||||||
payload: payload | ||||||||||
) | ||||||||||
|
||||||||||
return push | ||||||||||
let push = PushV2(channel: self, message: message) | ||||||||||
if let ref = message.ref { | ||||||||||
mutableState.pushes[ref] = push | ||||||||||
} | ||||||||||
|
||||||||||
return await push.send() | ||||||||||
} | ||||||||||
|
||||||||||
private func didReceiveReply(ref: String, status: String) async { | ||||||||||
let push = mutableState.withValue { | ||||||||||
$0.pushes.removeValue(forKey: ref) | ||||||||||
} | ||||||||||
await push?.didReceive(status: PushStatus(rawValue: status) ?? .ok) | ||||||||||
@MainActor | ||||||||||
private func didReceiveReply(ref: String, status: String) { | ||||||||||
let push = mutableState.pushes.removeValue(forKey: ref) | ||||||||||
push?.didReceive(status: PushStatus(rawValue: status) ?? .ok) | ||||||||||
} | ||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't a breaking change since the method is
async
, the call site already have to call this method with anawait
independent of the running actor.