Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to me as the review manager. When contacting the review manager directly, please put "SE-0506" at the start of the subject line.
What goes into a review?
The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:
What is your evaluation of the proposal?
Is the problem being addressed significant enough to warrant a change to Swift?
Does this proposal fit well with the feel and direction of Swift?
If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
More information about the Swift evolution process is available at:
Should there be an overload of withContinuousObservationTracking that doesn’t take any options to mirror the existing version of withObservationTracking ? And if so, would that version be equivalent to passing an empty set or the set of all available options?
"continuous" observation looks like what I feel observation should've been all along.
Is there any point in the other API, or in keeping the existing API un-deprecated, given that we'd expect most if not all consumers to adopt continuous observation?
withContinuousObservationTracking offers two cancellation methods — the explicit cancel on an event, synchronous with its delivery, or the dropping/consumption of the Token, asynchronous with respect to event delivery.
Are both mechanisms required?
How do they interact?
What guarantee does dropping/consuming the token offer with respect to future invocations of the apply block? Presumably there's always a case where the block runs at least once more after the token drops.
I have no intuition about what guarantees @_inheritActorContext @isolated(any) @Sendable @escaping might or might not provide. It seems clear it's intended to allow a non-Sendable object in the calling context to be captured and referenced by the block, but does this actually work in general? I can convince myself that it'll work if the calling context is @MainActor, but
will it work if the calling context is nonisolated? My mental model of how this all works is that if there isn't an actor, there isn't a way to call back in the same isolation, so I'm skeptical?
If the calling context is actor-instance-isolated, will the isolated(any) closure cause a strong retain on the actor instance for as long as the observation is in place? Is that desirable?
And, although I had a partial answer in the other thread, I think I need to ask about concurrent and coincident updates to the observed properties:
if another thread modifies an observed property during the invocation of the apply block, is there a guarantee (in the absence of cancellation) that that new value will be observed by either the current or a future invocation of the apply block?
if the apply block itself modifies an observed property during its invocation, is there a guarantee (in the absence of cancellation) that the new value will be observed by either the current or a future invocation of the apply block?
I consider myself pretty ill-equipped to comment on the fundamental problem and solution. I just don’t have a good handle on either. However, I still have managed to form two thoughts.
The first is that it is extremely common to see people struggle with the withObservationTracking function. I am very excited to see it get attention.
The second comment is about the signature of the withContinuousObservationTracking replacement itself. I’m going to reproduce it here just for reference.
I think it is very notable that apply is synchronous.
A synchronous @ioslated(any) function is an exotic type. Because at callsites, an await is still required. However, I could see it being necessary for implementation reasons. And no doubt there is implementation complexity here.
As written now, we are going to see a lot of unstructured concurrency uses within these closures. If this function could be made asynchronous, I think it would be a considerable win for ergonomics and correctness. And if not, I think the reasons why should be documented in the proposal (which I tried to find, but could not. Very sorry if it I missed it!)
For users to call? no, the manual cancellation is paramount in the effect - calling that will enforce the tracking to be cancelled.
If a token is manually cancelled that sets the tracking into a cancelled state, if the token is dropped somehow then it is terminated
Yes, the one exception to the dropping is that you get the first call to do the "initial setup" phase of the observation.
This is specifically the same interface as Observations - the rules are that it if the closure is called on a specific isolation it promises to have future invocations to be called on that same isolation and furthermore if the closure is isolated to a specific actor then there is no actual sending across that boundary. However if the closure is not being setup in an isolated context then the items within the closure are enforced to be Sendable by the compiler.
This is still an outstanding general issue with observation, however I have a prototype of potentially handling that.
This cannot be permitted; if so SwiftUI would actually hard-assert about that (making observation incompatible with SwiftUI - which is distinctly a no-go behavior).
This is VERY intentional, the apply cannot be asynchronous at all - the fundamentals of Observation require that the application of the context must be synchronous to ensure numerous factors around the behavior of that closure. I'm not sure I buy the "folks will use Task" argument because firstly the tracking in that wont actually work (modulo perhaps the new initially synchronous initializer for Task). The asynchronous variant only has impact if we were to really explore observable actor types, which is still something to explore in the future. Doing so would require a fair amount of design and rethink around the functions exposed to observe that and would likely need overloads for other reasons.
I think I understand what you mean and is why I tried to preface this by saying “I don’t know what I’m doing”. It is clear that I did not. Thanks for addressing it!
Hopefully I didn't come across too bristly about it - Observation has some pretty unique approaches that can feel quasi "magic".
Under the hood it uses thread local storage to track things that could potentially be disjointed or cyclic graph structures but it also leans on that synchronous apply so that the isolation and suspension behave as expected around how the transaction boundaries of changes work for the Observations API, SwiftUI's usage of Observation, and now the new continuous form of tracking.
None of these prevent a future in which we open up observation of asynchronous mechanisms but that should be paired with any investigation around actors (which are currently restricted from being @Observable due to a blocking feature around KeyPath).
Nope not at all. It was my mistake. I attempted to skim the proposal to find mention of exactly how and why this function was constructed as it was. I’m not the least-bit surprised this was both intentional and required.
Love this proposal - feels like a natural evolution of observation tracking.
For the proposed implementation of withContinuousObservationTracking, it isn’t clear to me why the onChange param is dropped in favor of the Event being passed directly into the apply closure. The one code example in the proposal for withContinuousObservationTracking doesn’t consume the event:
Inthe proposed update to withObservationTracking, the separations of concerns is clear to me - the apply block is executed immediately and is used to determine which properties to install tracking on. The onChange block is executed the next time that the value of those properties is mutated (subject to the new options param).
It would feel intuitive for the new withContinuousObservationTracking to follow that same pattern, i.e.
init(view: MyView, model: MyObservable) {
synchronization = withContinuousObservationTracking(options: [.willSet]) { [view, model] in
view.label.text = model.someStringValue
} onChange: { event in
// executes on `willSet` each time the value of `model.someStringValue` changes until
// the observation is canceled due to deallocation or calling `.cancel()` on the token
}
}
Is there interest in aligning the usage of the withObservationTracking and withContinuousObservationTracking in such a way? If not, I think it would be valuable to include a deeper explanation of the apply block of the latter in the proposal, namely in how the initial work (i.e. view.label.text = model.someStringValue) is performed distinct of the consumption of the event param.
I believe any solution to the "concurrent modification" problem will also naturally solve this "coincident modification" problem.
Whilst it's possible that ObservationRegistrar.willSet/didSet could use the TaskLocal to distinguish the two, I don't see the necessity.
(the only way I can see to solve the concurrent modification problem is for ObservationRegistrar.access to install tracking for the property. It isn't currently documented, but this would mean that access must be called before the property implementation reads any values; this is currently the case for code generated by the macro, but might plausibly not be for manual implementations, so it should be documented. If access installs tracking atomically, then both problems fall out:
if the value is modified before access installs tracking, then this iteration of the tracking loop will see the modification.
if the value is modified after access installs tracking, then we will exit the apply block with the next iteration of the tracking loop already scheduled, and the next iteration of the tracking loop will see this modification.
whether the modification is "concurrent" or "coincident" is irrelevant, only that it is atomic with respect to access installing tracking.
Yes, you can create an infinite loop this way, but that's never been Observation's goal to prevent.)
To note here: the behavior with regards to the exact moment of when an observation is installed is an implementation detail and kind-of tangential to this proposal. That being said there are fixes contained in the current implementation with regards to the staleness of events caused by invalidations during concurrent mutations - it improves the "best effort" for any race conditions that may be extant in consumers of the APIs without violating the requirements of current users of the APIs.
The concept of task local willSet and didSet events are quite out of scope of this proposal as well and would need considerable engineering effort to investigate what that would mean and how to even approach it. Fundamentally it almost seems like a different beast but that needs investigation.
Sure, the implementation detail is irrelevant to this proposal, I brought it up only to refute your previous claim that coincident modification can never be safe. Any solution to concurrent modification will necessarily fix coincident modification, and any statement that "coincident modification is always disallowed" is a statement of policy, rather than safety, and an unnecessary one as far as I can see.
This isn't a "oh it's a kind of an edge case and doesn't really matter much" bug — maybe I could buy that observation was only for nonisolated/global-actor-isolated classes & the language simply lacked a way to express that constraint, until Foundation's ProgressReporter proposal was accepted. Now it's explicitly clear that observation is supposed to work for concurrent modification to Sendable classes, and it absolutely does not.
Ultimately I think you're either fixing the problem or you're not. And if you're not fixing the problem, and indeed don't seem certain how to fix the problem, I'm quite unconvinced that we should be building more APIs with the same problem.
ObservationTracking.Event/matches(_:) will have subtle and unintuitive behavior when a client is accessing an observable object via computed properties (more complete example from the pitch thread); the proposed API results in changes in behavior on the client end if an observed object’s property changes from stored to computed (which is API- and ABI-compatible today, and to my understanding is intended to be opaque to clients)
ObservationTracking.Event/matches(_:) operating at KeyPath granularity limits its ability to disambiguate between changes: an observation tracking closure which references multiple Observable objects of the same type cannot determine which of the observed objects changed, only which property did
This function seems to best support simple object graphs (or more precisely, those where each type has one instance, or few enough that disambiguations is unnecessary)
Not novel to this proposal: new APIs with @_inheritActorContext closures are thorny as a developer, because this underscored attribute is largely hidden to developers but the behavior difference is essential to understanding the puzzle of attributes that affect the isolation with which a closure is run
Is the problem being addressed significant enough to warrant a change to Swift?
Yes. “How do I observe didSet?” has been a frequently asked developer question since the introduction of Observable, and offering differently shaped tools to address that question expands the “free” value of @Observable.
How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
Engagement in the pitch thread; a quick re-read now.