Skip to content

Expose observe overloads for separate tracking and application of changes #286

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.

Sign up for GitHub

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Expose observe overloads for separate tracking and application of changes #286

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a8f3be2
Expose `observe` overloads for separate tracking and application of c…
maximkrouk May 11, 2025
5df4eb9
Add minor performance optimization for onChange(of:perform:task:)
maximkrouk May 20, 2025
c058a10
Fix strong token capture in `observe(_:onChange:task)`
maximkrouk May 20, 2025
a1573d6
Merge branch 'main' into expose-observe-function
stephencelis Sep 23, 2025
ef2926b
feat: Fixes for expose-observe-function
maximkrouk Sep 24, 2025
684393d
Merge remote-tracking branch 'github/expose-observe-function' into ex…
maximkrouk Sep 25, 2025
c37705e
Merge branch 'main' into expose-observe-function
stephencelis Sep 26, 2025
189e51a
fix: Isolation tests
maximkrouk Oct 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 57 additions & 22 deletions Sources/SwiftNavigation/Observe.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,7 @@ import ConcurrencyExtras
_observe(
apply,
task: { transaction, operation in
Task {
await operation()
}
call(operation)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The task here was breaking the isolation, maybe marking everything as @_inheritActorContext + @isolated(any) wasn't required, I'll check if it works without it later 🫠

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw Task.yield() is still not sufficient for awaiting for changes to be applied 😔

Copy link
Contributor Author

@maximkrouk maximkrouk Oct 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw Task.yield() is still not sufficient for awaiting for changes to be applied 😔
I also noticed that old observe(_:) should use a separate version of onChange/withRecursivePerceptionTracking, current one is calling this shared apply closure too often I'll investigate it a bit later 🫠

}
)
}
Expand All @@ -161,6 +159,7 @@ import ConcurrencyExtras
public func observe(
@_inheritActorContext
_ context: @escaping @isolated(any) @Sendable (_ transaction: UITransaction) -> Void,
@_inheritActorContext
onChange apply: @escaping @isolated(any) @Sendable (_ transaction: UITransaction) -> Void
) -> ObserveToken {
_observe(
Expand All @@ -183,10 +182,12 @@ import ConcurrencyExtras
/// - Returns: A token that keeps the subscription alive. Observation is cancelled when the token
/// is deallocated.
func _observe(
_ apply: @escaping @Sendable (_ transaction: UITransaction) -> Void,
task: @escaping @Sendable (
@_inheritActorContext
_ apply: @escaping @isolated(any) @Sendable (_ transaction: UITransaction) -> Void,
@_inheritActorContext
task: @escaping @isolated(any) @Sendable (
_ transaction: UITransaction,
_ operation: @escaping @Sendable () -> Void
_ operation: @escaping @isolated(any) @Sendable () -> Void
) -> Void = {
Task(operation: $1)
}
Expand All @@ -207,11 +208,14 @@ func _observe(
/// - Returns: A token that keeps the subscription alive. Observation is cancelled when the token
/// is deallocated.
func _observe(
_ context: @escaping @Sendable (_ transaction: UITransaction) -> Void,
onChange apply: @escaping @Sendable (_ transaction: UITransaction) -> Void,
task: @escaping @Sendable (
@_inheritActorContext
_ context: @escaping @isolated(any) @Sendable (_ transaction: UITransaction) -> Void,
@_inheritActorContext
onChange apply: @escaping @isolated(any) @Sendable (_ transaction: UITransaction) -> Void,
@_inheritActorContext
task: @escaping @isolated(any) @Sendable (
_ transaction: UITransaction,
_ operation: @escaping @Sendable () -> Void
_ operation: @escaping @isolated(any) @Sendable () -> Void
) -> Void = {
Task(operation: $1)
}
Expand All @@ -222,7 +226,7 @@ func _observe(
task: task
)

apply(.current)
callWithUITransaction(.current, apply)
return token
}

Expand All @@ -235,9 +239,12 @@ func _observe(
/// - Returns: A token that keeps the subscription alive. Observation is cancelled when the token
/// is deallocated.
func onChange(
of context: @escaping @Sendable (_ transaction: UITransaction) -> Void,
perform operation: @escaping @Sendable (_ transaction: UITransaction) -> Void,
task: @escaping @Sendable (
@_inheritActorContext
of context: @escaping @isolated(any) @Sendable (_ transaction: UITransaction) -> Void,
@_inheritActorContext
perform operation: @escaping @isolated(any) @Sendable (_ transaction: UITransaction) -> Void,
@_inheritActorContext
task: @escaping @isolated(any) @Sendable (
_ transaction: UITransaction,
_ operation: @escaping @Sendable () -> Void
) -> Void = {
Expand All @@ -248,15 +255,15 @@ func onChange(
SwiftNavigation.withRecursivePerceptionTracking(
of: { [weak token] transaction in
guard let token, !token.isCancelled else { return }
context(transaction)
callWithUITransaction(transaction, context)
},
perform: { [weak token] transaction in
guard
let token,
!token.isCancelled
else { return }

var perform: @Sendable () -> Void = { operation(transaction) }
var perform: @Sendable () -> Void = { callWithUITransaction(transaction, operation) }
for key in transaction.storage.keys {
guard let keyType = key.keyType as? any _UICustomTransactionKey.Type
else { continue }
Expand All @@ -277,18 +284,21 @@ func onChange(
}

private func withRecursivePerceptionTracking(
of context: @escaping @Sendable (_ transaction: UITransaction) -> Void,
perform operation: @escaping @Sendable (_ transaction: UITransaction) -> Void,
task: @escaping @Sendable (
@_inheritActorContext
of context: @escaping @isolated(any) @Sendable (_ transaction: UITransaction) -> Void,
@_inheritActorContext
perform operation: @escaping @isolated(any) @Sendable (_ transaction: UITransaction) -> Void,
@_inheritActorContext
task: @escaping @isolated(any) @Sendable (
_ transaction: UITransaction,
_ operation: @escaping @Sendable () -> Void
) -> Void
) {
withPerceptionTracking {
context(.current)
callWithUITransaction(.current, context)
} onChange: {
task(.current) {
operation(.current)
callWithUITransaction(.current, task) {
callWithUITransaction(.current, operation)

withRecursivePerceptionTracking(
of: context,
Expand All @@ -299,6 +309,31 @@ private func withRecursivePerceptionTracking(
}
}

@Sendable
private func call(_ f: @escaping @Sendable () -> Void) {
f()
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Workarounds to make @escaping @isolated(any) @Sendable () -> Void functions callable from sync contexts

TODO: remove @escaping


@Sendable
private func callWithUITransaction(
_ transaction: UITransaction,
_ f: @escaping @Sendable (_ transaction: UITransaction) -> Void
) {
f(transaction)
}

@Sendable
private func callWithUITransaction(
_ transaction: UITransaction,
_ f: @escaping @Sendable (
_ transaction: UITransaction,
_ operation: @escaping @isolated(any) @Sendable () -> Void
) -> Void,
_ operation: @escaping @isolated(any) @Sendable () -> Void
) {
f(transaction, operation)
}

/// A token for cancelling observation.
///
/// When this token is deallocated it cancels the observation it was associated with. Store this
Expand Down
Loading