Flakey Behaviour — .refreshable on ScrollView

myurieff commented 1 year ago

Description

Attaching a .refreshable modifier on ScrollView and using the suggested ViewStore.send(_:while:) action in the body produces a flakey behaviour. Using the code example provided in the documentation of the function causes the following behaviour: 1 - User pulls to refresh: Loading stops immediately. No action received after side effect completes. 2 - User pulls to refresh: Loading waits for side effect to complete. Result action received after side effect completes. 3 - User pulls to refresh: Loading stops immediately. No action received after side effect completes. 4 - User pulls to refresh: Loading waits for side effect to complete. Result action received after side effect completes. and so on.

This behaviour is not present when using a List. The bug occurs on both latest released version 0.55.0 and on the main branch.

Vanilla SwiftUI seems to be working fine with both ScrollView and List.

Attached videos and test project:

https://github.com/pointfreeco/swift-composable-architecture/assets/25137824/779bd3e8-f631-40a3-a644-61a75c8520e9

https://github.com/pointfreeco/swift-composable-architecture/assets/25137824/fd2976f9-e391-4fc2-bf05-da10ddc111ca

TCARefreshable.zip

Checklist

[X] I have determined whether this bug is also reproducible in a vanilla SwiftUI project.
[X] If possible, I've reproduced the issue using the main branch of this package.
[X] This issue hasn't been addressed in an existing GitHub issue or discussion.

Expected behavior

1 out of 2 times, pull to refresh on ScrollView with TCA doesn't wait for loading to complete and the reducer doesn't receive action that is supposed to be fed back.

Actual behavior

Every time you do pull to refresh, the loading indicator persists while the side effect is ongoing, dismisses when it completes, and the reducer receives the result action from the side effect.

Steps to reproduce

Implement pull to refresh as explained in the ViewStore.send(_:while:) function.
Run the feature and pull to refresh multiple times consecutively.
Monitor feature behaviour.

The Composable Architecture version information

0.55.0 and main

Destination operating system

iOS 16.4

Xcode version information

14.3.1

Swift Compiler version information

swift-driver version: 1.75.2 Apple Swift version 5.8.1 (swiftlang-5.8.0.124.5 clang-1403.0.22.11.100)
Target: arm64-apple-macosx13.0

myurieff commented 1 year ago

Tested it with await viewStore.send(.pulledToRefresh).finish() with no success.

Edit 1 What seems to be happening is that with ScrollView the ViewStoreTask gets immediately cancelled when the bug occurs.

Edit 2 This disgusting workaround works:


extension ViewStore {
    @MainActor
    func send(_ action: ViewAction, suspendWhile predicate: @escaping (ViewState) -> Bool) async {
        self.send(action)
        await Task { @MainActor in
            _ = await self.publisher.values.first(where: { !predicate($0) })
        }.value
    }
}

mbrandonw commented 1 year ago

This seems to be a bug in SwiftUI. I can reproduce the same problem in vanilla SwiftUI if I use an observed object rather than @State:

class Model: ObservableObject {
  @Published var isLoading = false
  @Published var response: String?
  func load() async {
    self.isLoading = true
    defer { self.isLoading = false }
    try? await Task.sleep(for: .seconds(2))
    self.response = UUID().uuidString
  }
}

struct ContentViewVanilla: View {
  @ObservedObject var model = Model()

  var body: some View {
    TabView {
      NavigationView {
        ScrollView {
          Text(self.model.response ?? "Pull To Refresh…")
            .frame(maxWidth: .infinity, maxHeight: .infinity)
            .foregroundColor(self.model.response == nil ? .secondary : .primary)
        }
        .refreshable {
          await self.model.load()
        }
        .navigationTitle(Text("Vanilla Scroll View"))
      }
      .tabItem {
        Label("Scroll View", systemImage: "scroll")
      }

      NavigationView {
        List {
          Text(self.model.response ?? "Pull To Refresh…")
            .frame(maxWidth: .infinity, maxHeight: .infinity)
            .foregroundColor(self.model.response == nil ? .secondary : .primary)
        }
        .refreshable {
          await self.model.load()
        }
        .navigationTitle(Text("Vanilla List"))
      }
      .tabItem {
        Label("List", systemImage: "list.bullet.clipboard")
      }
    }
  }
}

It seems to have something to do with updating a published property while the pull-to-refresh work is being done. If you get rid of the isLoading state in the model it starts to work again. And who knows why this seems to only afflict scroll views and not lists.

If you get rid of the isLoading state in the TCA version and use await viewStore.send(…).finish() it will also start to work.

Since this doesn't seem to be a bug in the library I am going to convert it to a discussion.

pointfreeco / swift-composable-architecture