Child State not properly updated in the "parent" reducer after using NavigationLink in a ForEach

[renaudjenny]

I've more or less followed this use case: https://github.com/pointfreeco/swift-composable-architecture/blob/main/Examples/CaseStudies/SwiftUICaseStudies/03-Navigation-Lists-LoadThenNavigate.swift despite I don't need to Load, so it should be simpler.

In a nutshell, I've a list of recipes RecipeListView, that I can navigate to, this will open a RecipeView which is the form view to edit the Recipe, very common case I guess.
I spend few hours trying multiple things to fulfil all the needs:

1. I want to have a list of Recipes and when I tap on it that will navigate to edit the recipe.
2. When I tap the + button, it will create a new recipe on the top of the list and immediately navigate to edit it.

Good thing is, 1. worked perfectly with a ForEachStore, but impossible for me to code the 2. feature. So I tried NavigationLink with selection: like it's used the case studies I linked above.

Unfortunately, something in my project is wrong. I can well edit the Recipe, every edit in RecipeReducer logged well, the view is well updated, etc. But the pullback in the "parent" reducer or something is incorrect for some reason, so in RecipeListReducer, the state is not modified...

I joined a video of the application, so it will be easier to understand what's wrong and what I'm expecting.
If I modify the title or meal count. It's not well updated (recipes looks to be unchanged). I'm expecting to see in the list Pâtes aux poires instead of Pâtes aux poireaux after modifications when I navigate back to the list.

The code is available here: https://github.com/renaudjenny/WeeklyRecipePlanning/tree/main/Shared/Recipe

There is the "domain" RecipeList* with the RecipeListView and RecipeListCore for the list.
And also the "domain" Recipe* with the RecipeView and RecipeCore.

I'm suspecting that my pullback is not properly made, but I don't know what to fix honestly.

// in RecipeListCore
struct RecipeListState: Equatable {
    var recipes: IdentifiedArrayOf<RecipeState>
    var selection: Identified<Recipe.ID, RecipeState>?
}

enum RecipeListAction: Equatable {
    case addNewRecipe
    case setNavigation(selection: Recipe.ID?)
    ...
    case recipe(RecipeAction)
}

struct RecipeListEnvironment { ... }

let recipeListReducer = Reducer<RecipeListState, RecipeListAction, RecipeListEnvironment>.combine(
    recipeReducer
        .pullback(
            state: \Identified.value,
            action: .self,
            environment: { $0 }
        )
        .optional()
        .pullback(
            state: \.selection,
            action: /RecipeListAction.recipe,
            environment: { RecipeEnvironment(uuid: $0.uuid) }
        ),
    Reducer { state, action, environment in
        switch action {
        case .addNewRecipe:
            let newRecipe = RecipeState(recipe: .new(id: environment.uuid()))
            state.recipes.insert(newRecipe, at: 0)
            return Effect(value: .setNavigation(selection: newRecipe.id))
        case let .setNavigation(selection: .some(id)):
            guard let recipe = state.recipes[id: id]
            else { return .none }
            state.selection = Identified(recipe, id: \.id)
            return .none
        case .setNavigation(selection: .none):
            state.selection = nil
            return .none
        ...
        case .recipe:
            // If I put a Breakpoint here, each modification on a recipe is well breaking here. However, when I `po state.recipes[...].mealCount` for instance, it's not well updated (it's still the same value, like non modified)
            return Effect(value: .save)
            ....
        }
    }
)

// In RecipeListView
...
struct RecipeListView: View {
    let store: Store<RecipeListState, RecipeListAction>

    var body: some View {
        WithViewStore(store) { viewStore in
            NavigationView {
                List {
                    ForEach(viewStore.recipes) { recipe in
                        NavigationLink(
                            destination: IfLetStore(
                                store.scope(
                                    state: \.selection?.value,
                                    action: RecipeListAction.recipe
                                ),
                                then: RecipeView.init(store:)
                            ),
                            tag: recipe.id,
                            selection: viewStore.binding(
                                get: \.selection?.id,
                                send: RecipeListAction.setNavigation(selection:)
                            )
                        ) {
                            RecipeRowView(recipe: recipe.recipe)
                        }
                    }
                    .onDelete { viewStore.send(.delete($0)) }
                }
                .navigationTitle("Recipes")
                .toolbar {
                    ToolbarItem(placement: addRecipeButtonPlacement) {
                        addNewRecipeButton
                    }
                }
            }
            .onAppear { viewStore.send(.load) }
        }
    }
}

Perhaps something is wrong in Recipe* domain but I doubt about that, as it was working with ForEachStore in some commits back.

// In RecipeCore
@dynamicMemberLookup
struct RecipeState: Equatable, Identifiable {
    var recipe: Recipe
    var ingredientList: IngredientListState {
        didSet { recipe.ingredients = IdentifiedArrayOf(ingredientList.ingredients.map(\.ingredient)) }
    }

    var id: Recipe.ID { recipe.id }

    init(recipe: Recipe) {
        self.recipe = recipe
        self.ingredientList = IngredientListState(
            ingredients: IdentifiedArrayOf(recipe.ingredients.map { IngredientState(ingredient: $0) })
        )
    }

    subscript<T>(dynamicMember keyPath: WritableKeyPath<Recipe, T>) -> T {
        get { recipe[keyPath: keyPath] }
        set { recipe[keyPath: keyPath] = newValue }
    }
}

enum RecipeAction: Equatable {
    case nameChanged(String)
    case mealCountChanged(Int)
    case ingredientList(IngredientListAction)
}

struct RecipeEnvironment {
    var uuid: () -> UUID
}

let recipeReducer = Reducer<RecipeState, RecipeAction, RecipeEnvironment>.combine(
    ingredientListReducer.pullback(
        state: \.ingredientList,
        action: /RecipeAction.ingredientList,
        environment: { IngredientListEnvironment(uuid: $0.uuid) }
    ),
    Reducer { state, action, environment in
        switch action {
        case let .nameChanged(name):
            state.name = name
            return .none
        case let .mealCountChanged(mealCount):
            state.mealCount = mealCount
            return .none

        case .ingredientList:
            return .none
        }
    }
)

// in RecipeView
struct RecipeView: View {
    let store: Store<RecipeState, RecipeAction>

    var body: some View {
        WithViewStore(store) { viewStore in
            Form {
                Section(header: Text("Title")) {
                    TextField("Name", text: viewStore.binding(get: { $0.name }, send: RecipeAction.nameChanged))
                        .font(.title)
                }

                Stepper("Meal count: \(viewStore.mealCount)", value: viewStore.binding(get: { $0.mealCount }, send: RecipeAction.mealCountChanged), in: 0...99)

                IngredientListView(store: store.scope(state: { $0.ingredientList }, action: RecipeAction.ingredientList))
            }
        }
    }
}

Sorry for this very long message, thanks a lot for the ones who read everything 😂. I hope someone could help me figure it out where the problem is. Cheers!

[renaudjenny]

I've probably found something relevant:

struct RecipeListState: Equatable {
    var recipes: IdentifiedArrayOf<RecipeState>
    var selection: Identified<Recipe.ID, RecipeState>? {
        didSet { print(selection?.value.mealCount) }
    }
}

This logs the correct mealCount when changed. So I obviously did:

struct RecipeListState: Equatable {
    var recipes: IdentifiedArrayOf<RecipeState>
    var selection: Identified<Recipe.ID, RecipeState>? {
        didSet { selection.map { recipes[id: $0.id] = $0.value } }
    }
}

But IMO that breaks the Unidirectional Flow (RecipeListState is modifying its own... state, if that makes sense) and this should be done in the Reducer if I'm not wrong. I also wrote this test:

    func testUpdateAndSaveRecipe() throws {
        let store = try XCTUnwrap(self.store)
        let saveSubject = try XCTUnwrap(self.saveSubject)

        let recipes = [Recipe].test
        let firstRecipe = try XCTUnwrap(recipes.first)

        store.assert(
            .send(.setNavigation(selection: firstRecipe.id)) {
                $0.selection = Identified(RecipeState(recipe: firstRecipe), id: firstRecipe.id)
            },
            .send(.recipe(RecipeAction.nameChanged("Modified by Test"))) { // State change does not match expectation: …
                $0.recipes[0].name = "Modified by Test"
            },
            .do { self.mainQueue.advance(by: .seconds(1)) },
            .receive(.save),
            .do { saveSubject.send(true) },
            .receive(.saved(.success(true)))
        )
    }

The test fails, and I can totally understand why. In the example here: https://github.com/pointfreeco/swift-composable-architecture/blob/main/Examples/CaseStudies/SwiftUICaseStudies/03-Navigation-Lists-LoadThenNavigate.swift this seems to work seamlessly, I guess I'm not looking in the right place...

I just updated the code with the failing test few minutes ago.

[renaudjenny]

I feel stupid about what I just wrote above...

I just need to update the recipes[id: selection.id] = selection.value in the reducer case .recipe: and that won't break the unidirectional flow.

Then this code works perfectly:

        ...
        case .recipe:
            state.selection.map {
                state.recipes[id: $0.id] = $0.value
            }
            return Effect(value: .save)
                .debounce(id: SaveDebounceId(), for: .seconds(1), scheduler: environment.mainQueue)
                .eraseToEffect()
        }

I didn't understand how it supposed to work for the case study: https://github.com/pointfreeco/swift-composable-architecture/blob/main/Examples/CaseStudies/SwiftUICaseStudies/03-Navigation-Lists-NavigateAndLoad.swift
I naively though it was done for us magically within the first pullback with state: \Identified.value

recipeReducer
        .pullback(
            state: \Identified.value,
            action: .self,
            environment: { $0 }
        )
        .optional()
        .pullback(
            state: \.selection,
            action: /RecipeListAction.recipe,
            environment: { RecipeEnvironment(uuid: $0.uuid) }
        ),

But it's actually explicitly done while you navigate back, and "parent" counter state is updated at this moment:

      case .setNavigation(selection: .none):
        if let selection = state.selection, let count = selection.value?.count {
          state.rows[id: selection.id]?.count = count // <-- I've missed this in the case study...
        }
        state.selection = nil
        return .cancel(id: CancelId())

Now everything makes sense. And I think I found the answer to my own question.

Also, I had to update my test to reflect what's going on. And now it makes even more sense of what happens actually:

    func testUpdateAndSaveRecipe() throws {
        let store = try XCTUnwrap(self.store)
        let saveSubject = try XCTUnwrap(self.saveSubject)

        let recipes = [Recipe].test
        let firstRecipe = try XCTUnwrap(recipes.first)

        store.assert(
            .send(.setNavigation(selection: firstRecipe.id)) {
                $0.selection = Identified(RecipeState(recipe: firstRecipe), id: firstRecipe.id)
            },
            .send(.recipe(RecipeAction.nameChanged("Modified by Test"))) {
                $0.selection?.value.name = "Modified by Test" // <- selection is updated
                $0.recipes[0].name = "Modified by Test" // <- you have to explicitly update the according recipe as well in the reducer, nothing is magically done for you like with the couple `ForEachStore & reducer.forEach`.
            },
            .do { self.mainQueue.advance(by: .seconds(1)) },
            .receive(.save),
            .do { saveSubject.send(true) },
            .receive(.saved(.success(true)))
        )
    }

[renaudjenny]

TL;DR: if you list something, like a list of recipes named recipes, and you can't use use the couple ForEachStore and recipeReducer.forEach(...) because you want to control the navigation with NavigationLink(destination:tag:selection:label:) what I named NavigationLink with selection:, you have to use the technique Identified selection:
State

struct RecipeListState: Equatable {
    var recipes: IdentifiedArrayOf<RecipeState>
    var selection: Identified<Recipe.ID, RecipeState>?
}

Action

enum RecipeListAction: Equatable {
    case setNavigation(selection: Recipe.ID?)
    case recipe(RecipeAction)
}

Reducer

let recipeListReducer = Reducer<RecipeListState, RecipeListAction, RecipeListEnvironment>.combine(
    recipeReducer
        .pullback(
            state: \Identified.value,
            action: .self,
            environment: { $0 }
        )
        .optional()
        .pullback(
            state: \.selection,
            action: /RecipeListAction.recipe,
            environment: { RecipeEnvironment(uuid: $0.uuid) }
        ),
    Reducer { state, action, environment in
        switch action {
        case let .setNavigation(selection: .some(id)):
            state.recipes[id: id].map {
                state.selection = Identified($0, id: \.id)
            }
            return .none
        case .setNavigation(selection: .none):
            state.selection = nil
            return .none

        case .recipe:
            state.selection.map {
                state.recipes[id: $0.id] = $0.value
            }
            return .none
        }
    }
)

It was actually the case right in the example case study here: https://github.com/pointfreeco/swift-composable-architecture/blob/main/Examples/CaseStudies/SwiftUICaseStudies/03-Navigation-Lists-NavigateAndLoad.swift#L65-L67
but for some reasons I completely passed by. So if someone else found themselves in this situation, I hope this discussion will help.