Skip to content
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

Runtime warnings #49

Open
3 tasks done
arnauddorgans opened this issue Mar 6, 2024 · 11 comments
Open
3 tasks done

Runtime warnings #49

arnauddorgans opened this issue Mar 6, 2024 · 11 comments
Labels
bug Something isn't working due to a bug in the library.

Comments

@arnauddorgans
Copy link

arnauddorgans commented Mar 6, 2024

Description

This code snippet produces 3 runtime warnings on appear about Perceptible state was accessed but is not being tracked. Track changes to state by wrapping your view in a 'WithPerceptionTracking' view.

@Perceptible
class FeatureModel {
    var text = ""
}

struct ContentView: View {
    @Perception.Bindable var model: FeatureModel

    var body: some View {
        WithPerceptionTracking {
            TextField("", text: $model.text)
                .background(GeometryReader { proxy in
                    Color.clear
                        .randomModifier(of: proxy.safeAreaInsets)
                })
        }
    }
}

Checklist

  • I have determined that this bug is not reproducible using Swift's observation tools. If the bug is reproducible using the @Observable macro or another tool from the Observation framework, please file it directly with Apple.
  • If possible, I've reproduced the issue using the main branch of this package.
  • This issue hasn't been addressed in an existing GitHub issue or discussion.

Expected behavior

No runtime warning

Actual behavior

Runtime warning

Steps to reproduce

Full app code

@Perceptible
class FeatureModel {
    var text = ""
}

struct ContentView: View {
    @Perception.Bindable var model: FeatureModel

    var body: some View {
        WithPerceptionTracking {
            TextField("", text: $model.text)
                .background(GeometryReader { proxy in
                    Color.clear
                        .randomModifier(of: proxy.safeAreaInsets)
                })
        }
    }
}

extension View {
    @ViewBuilder
    func randomModifier<T>(of value: T) -> some View where T: Equatable {
        if #available(iOS 17.0, *) {
            self
        } else {
            fatalError() // We don't care here
        }
    }
}

@main
struct TCABugApp: App {
    var body: some Scene {
        WindowGroup {
            ContentView(model: .init())
        }
    }
}

Perception version information

1.1.2

Destination operating system

iOS 14.0

Xcode version information

Xcode 15.3

Swift Compiler version information

swift-driver version: 1.90.11.1 Apple Swift version 5.10 (swiftlang-5.10.0.13 clang-1500.3.9.4)
Target: arm64-apple-macosx14.0
@arnauddorgans arnauddorgans added the bug Something isn't working due to a bug in the library. label Mar 6, 2024
@stephencelis
Copy link
Member

@arnauddorgans I can't seem to reproduce this on 1.1.3 / main. Can you confirm that it's been fixed and close this out, or can you please submit a failing test case?

@stephencelis
Copy link
Member

Ah never mind, I couldn't reproduce it in our unit test suite but I was able to reproduce it in an app. Very strange! Will look into it!

@m430
Copy link

m430 commented May 29, 2024

Any update? Same problem.

@mbrandonw
Copy link
Member

Hi @m430, no update on our end. If someone has time to look into the issue we would happily accept a PR!

@arnauddorgans
Copy link
Author

arnauddorgans commented May 30, 2024

@m430 I used a workaround by using a SwiftUI State

@State private var searchText: String

var body: some View {
  WithPerceptionTracking {
    TextField("", text: $searchText)
      .bind($store.searchText, to: $searchText)    
  }
}

@ajkolean
Copy link

Just encountered this issue as well.

The interaction between GeometryReader and WithPerceptionTracking might lead to runtime warnings due to the way GeometryReader integrates into the SwiftUI view hierarchy. It seems that GeometryReader could be introducing a new view context that disrupts the state tracking context established by WithPerceptionTracking. This disruption might cause the Perception library to lose track of state changes, resulting in runtime warnings indicating that state is being accessed without proper tracking.

I examined the Thread.callStackSymbols and can confirm that accessing the GeometryProxy within the GeometryReader is triggering key path reads, causing the perceptionRegistrar.access method to be invoked.

This issue is likely due to the way SwiftUI and the Perception library manage state updates and how views re-render. The GeometryReader might be indirectly causing the model member getter to be called, even if it is not explicitly within the GeometryReader. This can happen due to SwiftUI's internal mechanisms for batching updates and managing state changes.

It seems like the safest course is to re-establish tracking whenever using GeometryReader.

GeometryReader { proxy in
  WithPerceptionTracking {
     //
  }
}

@mbrandonw
Copy link
Member

It seems like the safest course is to re-establish tracking whenever using GeometryReader.

This isn't related to this specific issue. What you are referring to is true of any escaping closure, and is mentioned in the docs:

You will also need to use WithPerceptionTracking in any escaping, trailing closures used in SwiftUI’s various navigation APIs, such as the sheet modifier:

@tyler927
Copy link

tyler927 commented Aug 2, 2024

Hey @mbrandonw , I recently ran into this "issue" myself while using GeometryReader. I was actually unaware that GeometryReader was an escaping closure. I totally see that it is now, but I think what tripped me up is it is not a modifier like the sheet is. Personally, I think it would be nice to explicitly call out GeometryReader in the docs as well.

@janczawadzki
Copy link

This is an interesting project but this caveat is bit concerning. Is there a chance this could be addressed in some future version with patching the view context hierarchy? Or is this hitting a more fundamental SwifUI design limitation/issue?

@stephencelis
Copy link
Member

@janczawadzki Can you explain why the caveat is concerning in particular? We don't think there's a fundamental issue with the library, and we think the library solves a real world problem for folks that want to adopt the Observation machinery of Swift in <iOS 17.

The issue raised here is one that I hope folks see as understandable:

  • WithPerceptionTracking is a necessary view in <iOS 17 for changes to be tracked and observed
  • This view is needed wherever there is a lazy/escaping context, so it can require some surgical attention and multiple instances in a single view conformance
  • When the library detects state is accessed in a view and it does not detect an enclosing WithPerceptionTracking, it does its best to warn the user
  • Tracking the above is surprisingly complicated, with lots of closures inside views that are not in the purview of view rendering, like action closures, but we err on the side of warning in such closures by default to grab your attention for testing
  • We're happy to improve things by suppressing warnings, but we'll need help from folks to continue to hone things

Note that the runtime warnings provided by the library are completely additive for debugging purposes, and don't exist in release, and the main issue is that it's difficult to suppress warnings for every situation: it requires demangling stack symbols and detecting very specific action closures and otherwise.

An alternative would have been for us to not emit warnings at all and just let folks know they need to test their views thoroughly, but we thought that's solving things in the wrong direction, and not providing the baseline of tools for folks to investigate potential problems and missing WithPerceptionTracking views.

@janczawadzki
Copy link

@janczawadzki Can you explain why the caveat is concerning in particular? We don't think there's a fundamental issue with the library, and we think the library solves a real world problem for folks that want to adopt the Observation machinery of Swift in <iOS 17.

It absolutely addresses a real problem, and my question wasn't at all around messages - those are necessary!

Challenge in adopting this in a non-trivial project is just knowing where modifications are needed, since this can't be identified (AFAIK?) via static analysis. How big of a change is this for me.

Hence my question - library would be super useful, but I don't understand the internals of the state stack well enough to know if this issue could be overcome or not. Best case scenario would obviously be "it just works", next best would be "here are the places you need to inject WithExceptionTracking." (... and I'm not sure how that would work exactly, to be clear!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working due to a bug in the library.
Projects
None yet
Development

No branches or pull requests

7 participants