Make Perception runtime checking opt-in - #398
Draft
johnnewman-square wants to merge 1 commit into
Draft
Conversation
Perception's debug-only runtime check reports Store state read from a view body that is not wrapped in WithPerceptionTracking. That modifier is required for observation to work below iOS 17, but unnecessary at iOS 17 and above, where native Observation tracks the read on its own. The check cannot tell those two situations apart, so on 17 and above it reports reads that are already working correctly. Leaving the check on by default made sense while clients were still on iOS 16, since a warning there is diagnosing something real. As clients bump to 17 the default has instead become a recurring cost: ios-register turns the warnings off in debug builds, Market Catalog would need to do the same, Market snapshot tests trip the check through modal content, and Xcode previews needed a suppression path of their own. Invert the default. Rename the flag to enablePerceptionChecking, defaulting to false, so that a client using WorkflowSwiftUI below iOS 17 asks for the check rather than everyone else asking to be rid of it. The availability gate on the suppression funnel goes away with it: an explicit opt-in states what the gate was previously inferring from the OS version. Drop the Xcode preview special case too, and with it the XcodePreviews type and its tests, which have no other consumer. Previews were detected and suppressed unconditionally because the check was on by default and a canvas runs no app startup code, leaving a preview no way to reach the configuration. With the check off by default, a preview gets the quiet behavior it wanted from the default itself, and a preview that does want checking can reach the configuration through Runtime.updateDefaultConfiguration, which writes a process-global rather than a task local and so survives past the call. Previews now follow the same configuration as everything else instead of overriding it. The render pass wrapping in Workflow+Preview.swift stays. It is not a preview exception but the plumbing that routes a workflow's reads of its own state through the configuration at all, since those reads never pass through a Store. Two consequences worth being explicit about. This is a source-breaking rename plus a behavior change for every client, so it wants a major version bump. And below iOS 17 it is a real loss of signal, because an untracked read there is a genuine defect -- the view will not update when the state it reads changes -- and such a client now gets no diagnostic at all until it opts in.
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
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.
Perception's debug-only runtime check reports
Storestate read from a view body that isn't wrapped inWithPerceptionTracking. That modifier is required for observation to work below iOS 17, but unnecessary at iOS 17 and above, where native Observation tracks the read on its own. The check can't tell those two situations apart, so on 17 and above it reports reads that are already working correctly.#393 made suppression available as an opt-out, which was the right call while clients were still on iOS 16 — a warning there is diagnosing something real. As clients bump to 17, the default has become a recurring cost instead.
Summary
Runtime.Configuration.suppressPerceptionCheckingWhenUsingObservationtoenablePerceptionChecking, defaulting tofalseXcodePreviewstype and its testsStoreTestscases to cover the new default and the opt-inThis is a breaking change — a source-breaking rename plus a behavior change for every client — so it wants a major version bump.
Why rename rather than flip the default
A bare default flip leaves the flag named
suppress…, so a client that wants the check writessuppressPerceptionCheckingWhenUsingObservation = false. A double negative at the one call site whose whole purpose is to say "yes, I want this" works against the change. The default flip is behaviorally breaking on its own, so the major bump is already owed; the rename rides along and leaves the API reading the way it behaves.Why the availability gate goes away
The old funnel only suppressed at iOS 17 and above, inferring from the OS version whether a warning was worth showing. An explicit opt-in states the same thing directly and more accurately — a client knows its own deployment target, and can decide for itself.
Why the preview special case goes away
#396 detected the preview process and suppressed unconditionally, on the reasoning that a preview can't opt in for itself. That reasoning was half right, and I'd like to correct it here since it's load-bearing for this change:
Runtime.withConfiguration(override:operation:)is a task local, so a binding placed around a#Previewbody-producing closure is indeed gone by the time SwiftUI evaluates that body. ButRuntime.updateDefaultConfigurationwrites a process-global instead, so it survives past the call and a preview can use it:So a preview was never actually locked out — it just had boilerplate to write. With the check off by default, it doesn't even need that: a preview gets quiet behavior from the default itself, and the one thing it can no longer do is have the check forced off against an explicit opt-in. Previews now follow the same configuration as everything else rather than overriding it, which is the outcome worth having.
XcodePreviewshad no other consumer, so it and its tests are deleted rather than left orphaned.What stays
The
withPerceptionCheckSuppressedwrapping ofPreviewView's representable callbacks, added in #396. That isn't a preview exception — it's the only thing routing a workflow's reads of its own state through the configuration, since those reads never pass through aStore. Remove it and a render pass would be checked regardless of what the configuration says.The cost, stated plainly
Below iOS 17 this is a real loss of signal. An untracked read there is a genuine defect rather than a false positive: without
WithPerceptionTracking, a view does not update when the state it reads changes. A client on iOS 16 that doesn't know to opt in now gets a silently non-updating view and no diagnostic pointing at why.Test plan
tuist test --path Samples UnitTests --test-targets WorkflowSwiftUI-Tests— 47/47, including both inverted cases:test_perceptionRuntimeWarningsAreDisabledByDefault(no override, asserts the check is quiet) andtest_perceptionRuntimeWarningsWhenCheckingIsEnabled(opts in, asserts the check fires)swift buildacross the whole package — the rename is a public property in the coreWorkflowmodule, and nothing else referenced itswiftformat --lintcleanTest count drops from 51 to 47 because the four
XcodePreviewsTestsare deleted with the type they covered.Checklist