pointfreeco / swift-composable-architecture

A library for building applications in a consistent and understandable way, with composition, testing, and ergonomics in mind.
https://www.pointfree.co/collections/composable-architecture
MIT License
12.35k stars 1.44k forks source link

Binding for scrollPosition not working when using ForEach #2838

Closed Rokas91 closed 8 months ago

Rokas91 commented 8 months ago

Description

Binding for scrollPosition is not working when using ForEach, but it works using ForEachStore.

Checklist

Expected behavior

scrollPosition should update binding attribute.

Actual behavior

Binding attribute is not updated.

Steps to reproduce

@Reducer
struct ItemFeature {

    @ObservableState
    struct State: Identifiable, Equatable {
        var id: UUID = .init()
    }

    enum Action: Equatable {}

    var body: some ReducerOf<Self> {
        EmptyReducer()
    }
}

@Reducer
struct ListFeature {

    @ObservableState
    struct State: Equatable {
        var items: IdentifiedArrayOf<ItemFeature.State> = .init(uniqueElements: [.init(), .init(), .init(), .init(), .init()])
        var scrolledID: ItemFeature.State.ID?
    }

    enum Action: BindableAction, Equatable {
        case binding(BindingAction<State>)
        case items(IdentifiedActionOf<ItemFeature>)
    }

    var body: some ReducerOf<Self> {
        BindingReducer()
        Reduce { state, action in
            switch action {
            case .binding(\.scrolledID):
                // Not called.
                return .none

            default:
                return .none
            }
        }
        .forEach(\.items, action: \.items, element: ItemFeature.init)
    }
}

struct ListView: View {

    @Bindable var store: StoreOf<ListFeature>

    var body: some View {
        ScrollView {
            LazyVStack {
                ForEach(store.scope(state: \.items, action: \.items)) { store in
                    Text(store.id.uuidString)
                        .frame(height: 500)
                }
            }
            .scrollTargetLayout()
        }
        .scrollPosition(id: $store.scrolledID)
    }
}

Replace ForEach with ForEachStore and binding for scroll position will work.

The Composable Architecture version information

1.8.2

Destination operating system

iOS 17.2

Xcode version information

15.2

Swift Compiler version information

No response

mbrandonw commented 8 months ago

Hi @Rokas91, this is mentioned in the migration guide for 1.7, but using ForEach with a $store binding uses the child stores for the identity of each row, not the identity of the state in the store. This means your scrollPosition is a completely different type than the underlying identity of the ForEach.

To fix you must use an explicit id key path:

ForEach(
  store.scope(state: \.items, action: \.items),
  id: \.state.id
) { store in
}

Since this is not an issue with the library I am going to convert it to a discussion.