Fix Swift 6 actor isolation errors in RowndPlugin.swift#72
Open
leonelgalan wants to merge 1 commit into
Open
Conversation
Xcode 26 (Swift 6.2) enforces @mainactor isolation even in Swift 5 language mode. The configure method accesses @MainActor-isolated APIs (subscribe() and $current) from a nonisolated context, causing build failures. This wraps the state subscription in Task { @mainactor }, which is the canonical Swift concurrency migration pattern. Build errors without fix: - RowndPlugin.swift:46:68 — call to main actor-isolated instance method 'subscribe' in a synchronous nonisolated context - RowndPlugin.swift:48:58 — main actor-isolated property '$current' can not be referenced from a nonisolated context
Reviewer's guide (collapsed on small PRs)Reviewer's GuideWraps Rownd state subscription setup in a @mainactor Task to satisfy Swift 6 actor isolation rules while preserving the existing subscription behavior and avoiding duplicate initialization. Sequence diagram for RowndPlugin.configure state subscription with @mainactor TasksequenceDiagram
actor ReactNativeApp
participant RowndPlugin
participant MainActorTask
participant Rownd
participant RowndState
participant RowndPluginEventEmitter
ReactNativeApp->>RowndPlugin: configure()
alt state not yet initialized
RowndPlugin->>MainActorTask: create Task @MainActor [weak self]
activate MainActorTask
MainActorTask->>Rownd: getInstance()
Rownd-->>MainActorTask: RowndInstance
MainActorTask->>Rownd: state()
Rownd-->>MainActorTask: RowndStatePublisher
MainActorTask->>RowndState: subscribe(identityClosure)
RowndState-->>MainActorTask: initializedState
MainActorTask->>RowndPlugin: assign state = initializedState
MainActorTask->>RowndState: initializedState.$current.sink(newState)
loop on each newState
RowndState-->>MainActorTask: newState
MainActorTask->>RowndPluginEventEmitter: sendEvent(update_state, newState.toDictionary())
end
deactivate MainActorTask
else state already initialized or self deallocated
RowndPlugin-->>ReactNativeApp: return without changes
end
Class diagram for updated RowndPlugin state subscription structureclassDiagram
class RowndPlugin {
+state
+stateCancellable
+configure()
}
class Rownd {
+getInstance() Rownd
+state() RowndStatePublisher
}
class RowndStatePublisher {
+subscribe(identityClosure) RowndState
+$current
}
class RowndState {
+toDictionary() Dictionary
}
class RowndPluginEventEmitter {
+emitter RowndPluginEventEmitter
+sendEvent(withName, body) void
}
RowndPlugin --> Rownd : uses getInstance()
RowndPlugin --> RowndStatePublisher : uses state()
RowndStatePublisher --> RowndState : returns
RowndStatePublisher --> RowndState : $current.sink
RowndState --> RowndPluginEventEmitter : sendEvent(update_state, body)
RowndState --> RowndState : toDictionary() is called within sink callback
RowndPlugin ..> RowndState : holds state
RowndPlugin ..> RowndStatePublisher : holds stateCancellable
RowndPlugin ..> RowndPluginEventEmitter : emits update_state events
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- If
configure()is always expected to be called from the main actor, consider marking the method@MainActorinstead of spawning aTask { @MainActor … }, which would avoid the extra async hop and keep the call flow simpler and more predictable. - By moving the subscription setup into an async
Task,configure()now returns beforestateandstateCancellableare initialized; if callers rely on synchronous initialization, it may be worth documenting or revisiting that behavior (e.g., by makingconfigureasync or explicitly decoupling it into a two-phase setup).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- If `configure()` is always expected to be called from the main actor, consider marking the method `@MainActor` instead of spawning a `Task { @MainActor … }`, which would avoid the extra async hop and keep the call flow simpler and more predictable.
- By moving the subscription setup into an async `Task`, `configure()` now returns before `state` and `stateCancellable` are initialized; if callers rely on synchronous initialization, it may be worth documenting or revisiting that behavior (e.g., by making `configure` async or explicitly decoupling it into a two-phase setup).
## Individual Comments
### Comment 1
<location path="ios/RowndPlugin.swift" line_range="46" />
<code_context>
- withName: "update_state", body: try newState.toDictionary())
- } catch {
- print("Failed to encode Rownd state: \(String(describing: error))")
+ Task { @MainActor [weak self] in
+ guard let self = self, self.state == nil else { return }
+ let initializedState = Rownd.getInstance().state().subscribe { $0 }
</code_context>
<issue_to_address>
**issue (bug_risk):** The placement of `@MainActor` relative to the capture list is likely invalid Swift syntax.
In closure syntax, attributes must come before the capture list: `{ @MainActor [weak self] in ... }` is invalid. This will not compile as written. Consider either `Task { @MainActor in ... }` (no capture list) or `Task { [weak self] @MainActor in ... }`, which preserves both main-actor isolation and the weak capture.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| withName: "update_state", body: try newState.toDictionary()) | ||
| } catch { | ||
| print("Failed to encode Rownd state: \(String(describing: error))") | ||
| Task { @MainActor [weak self] in |
There was a problem hiding this comment.
issue (bug_risk): The placement of @MainActor relative to the capture list is likely invalid Swift syntax.
In closure syntax, attributes must come before the capture list: { @MainActor [weak self] in ... } is invalid. This will not compile as written. Consider either Task { @MainActor in ... } (no capture list) or Task { [weak self] @MainActor in ... }, which preserves both main-actor isolation and the weak capture.
Contributor
Author
|
I don't think we need to make the changes in the review:
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Xcode 26 (Swift 6.2) enforces @mainactor isolation even in Swift 5 language mode. The configure method accesses @MainActor-isolated APIs (subscribe() and $current) from a nonisolated context, causing build failures.
This wraps the state subscription in
Task { @MainActor }, which is the canonical Swift concurrency migration pattern.Build errors without fix:
Summary by Sourcery
Bug Fixes: