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

[Bug]: HookScope invalidates view on each Environment change #29

Open
3 tasks done
josefdolezal opened this issue Jul 4, 2022 · 4 comments
Open
3 tasks done
Labels
bug Something isn't working

Comments

@josefdolezal
Copy link
Collaborator

Checklist

  • This is not a bug caused by platform.
  • Reviewed the README and documentation.
  • Checked existing issues & PRs to ensure not duplicated.

What happened?

Consider a simple SwiftUI app:

@main
struct _App: App {
    var body: some Scene {
        WindowGroup {
            __App__
        }
    }
}

where __App__ is a placeholder for either plain SwiftUI App or its Hooks alternative.

SwiftUI App
struct SwiftUIApp: View {
    var body: some View {
        let _ = print("SwiftUI - I changed")
        Text("")
    }
}
Hooks App
func HooksApp() -> some View {
    HookScope {
        let _ = print("HookScope - I changed")
        Text("")
    }
}

When you run both apps from Xcode 13.4.1 on iOS 15.5, you will see the following output in the console:

SwiftUIApp() HooksApp()
SwiftUI - I changed
 
 
HookScope - I changed
HookScope - I changed
HookScope - I changed

Considering both apps are stateless and do not invalidate its body, the console result should match.
When tracking down the issue using SwiftUI's _printChange(), it's clear that it's caused by library's HookScopeBody struct:

private struct HookScopeBody<Content: View>: View {
    @Environment(\.self) private var environment
    ...
    var body: some View {
        if #available(iOS 15.0, *) {
+       let _ = Self._printChanges()
        dispatcher.scoped(environment: environment, content)
    }
}

Changes printed by SwiftUI are:

HookScopeBody: @self, @identity, _dispatcher, _environment changed.
HookScopeBody: _environment changed.
HookScopeBody: _environment changed.

The problem is in observing Environment(\.self) which invalidates HookScopeBody's body for each change in environment, causing a view to re-render.

I haven't dig into finding a solution yet, but wanted to keep the issue on sight as it can potentially cause unexpected re-renders of the whole app.

Notes:

  • ⚠️ HookView is also affected (as it internally uses HookScope)
  • ⚠️ When working on a solution, we need to keep in mind that Context internally uses environments too.

Expected Behavior

HookScope body should only invalidate for key path specified in useEnvironment or types used in useContext.

Reproduction Steps

  1. Create a new project and replace @main struct with _App struct from above
  2. Replace the __App__ with SwiftUIApp() and run
  3. Replace SwiftUIApp() call with HooksApp()
  4. Compare console outputs

Swift Version

5.6+

Library Version

<= 0.0.8

Platform

No response

Scrrenshot/Video/Gif

No response

@josefdolezal josefdolezal added the bug Something isn't working label Jul 4, 2022
@ra1028
Copy link
Owner

ra1028 commented Jul 5, 2022

True, this was not well thought out when I implemented it, I'm sorry.
But if we stop using @Environment(\.self), useEnvironment will no longer be available as a parent HookScope needs to observe the whole EnvironmentValues to use it lazily when a parent view evaluates its body.
As for the Context, unfortunately @EnvironmentValue only supports key paths, and generic EnvironmentKey is not available, so it needs to get the entire EnvironmentValues and read/write them directly using subscript.
I don't have a better idea on this either.
Any insights for a possible solution?

@ra1028
Copy link
Owner

ra1028 commented Jul 5, 2022

Only for Context, possibly the EnvironmentValue can be replaced with a hack using EnvironmentObject and a dictionary.

@josefdolezal
Copy link
Collaborator Author

It's a tough one to be honest! I will try to explore possible solutions. One thing that might work is to utilise DynamicProperty and ObservableObject composition:

class ObservableEnvironment<E>: ObservableObject {
    @Environment e: E

    init(_ keyPath: KeyPath<EnvironmentValues, E>) {
        ...
    } 
}
class ObservableEnvironments: ObservableObject {
    var environments: [ObservableEnvironment<??>]
    var objectWillChangePublisher: PassthroughSubject<Void> // Or any custom publisher that merges all environments' publishers
}

...

class HookDispatch: ObservableObject {
   @ObservableObject environments: ObservableEnvironments // Will trigger HookScope re-render
}

if we were able to pass keyPath from a hook to a dispatcher, then dispatcher could observe ObservableEnvironment (because dispatcher is ObservableObject too!) only for keyPaths it received.


I just got back from vacation and haven't even opened Xcode yet, so it's possible it wouldn't work the way I think. One problem that's clear right from the snippet above is that the code will be messy, considering it would require dynamic typing and type erasure to make it all work.

On the bright side, if this trick worked, we could use the same approach to make @EnvironmentObject accessible in hooks - that would be 🤯.


I don't have the exact design in mind yet, but I hope you can get a grasp of what it would look like..

@josefdolezal
Copy link
Collaborator Author

Long time no see 🖖

Point-Free just recently introduced new dependency management system to their architecture and it seems like it could improve our Context API performance by a lot.

I explored our options a little, and came up with API build on top of Point-Free Dependencies:

let StringContext = Context<String>.self
let IntContext = Context<Int>.self

func MyView() -> some View {
    StringContext.Provider("Provided using context") { // Runs DependencyValues._current.withValues
        IntContext.Provider(1) { // Runs DependencyValues._current.withValues
            let string = useContext(StringContext) // Reads from DependencyValues._current
            let int = useContext(IntContext)

            Text("\(string) \(int)")
        }
    }
}

As folks from Point-Free described, setting dependencies (context values in our case) one-by-one comes with performance overhead. We could introduce an API to allow users to set all dependencies without nesting:

Provider { // Result builder scope
    StringContext.provides("Provided using context")
    IntContext.provides(1)
} content: {  // Runs DependencyValues._current.withValues only once
    let string = useContext(StringContext) // Reads from DependencyValues._current
    let int = useContext(IntContext)

    Text("\(string) \(int)")
}

I could open PR if you are interested.


This solution unfortunately doesn't solve the underlying issue with Environment being called multiple times, but helps reducing occurrence of it.

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

No branches or pull requests

2 participants