Data in @EnvironmentObject and data in @Binding can be inconsistent

(My apologies that this is a SwiftUI question. I asked it on SO and Apple developer forum but didn't get a satisfying answer. I would appreciate it if anyone can share how you think about it.)

One important rule in SwiftUI is the single source of the truth. Given that, it's natural for one to assume that all the data accessed through the property wrappers provided by SwiftUI are consistent. But that's not true, as demonstrated in the example below.

The example app has two views: a list view and a detail view. Clicking an item in the list view goes to the detail view. The detail view contains a "Delete It" button. Clicking on the button crashes the detail view because the data in @EnvironmentObject and the data in @Binding are inconsistent.

import SwiftUI

struct Foo: Identifiable {
    var id: Int
    var value: Int
}

// Note that I use forced unwrapping in data model's APIs. The rationale: the caller of data model API should make sure it passes a valid id.
class DataModel: ObservableObject {
    @Published var foos: [Foo] = [Foo(id: 1, value: 1), Foo(id: 2, value: 2)]

    func get(_ id: Int) -> Foo {
        return foos.first(where: { $0.id == id })!
    }

    func remove(_ id: Int) {
        let index = foos.firstIndex(where: { $0.id == id })!
        foos.remove(at: index)
    }
}

struct ListView: View {
    @StateObject var dataModel = DataModel()

    var body: some View {
        NavigationView {
            List {
                ForEach($dataModel.foos) { $foo in
                    NavigationLink {
                        DetailView(fooID: $foo.id)
                    } label: {
                        Text("\(foo.value)")
                    }
                }
            }
        }
        .environmentObject(dataModel)
    }
}

struct DetailView: View {
    @EnvironmentObject var dataModel: DataModel
    // Note: I know I can pass the entire Foo's value to the detail view in this simple example. I pass Foo's id just to demonstrate the issue.
    @Binding var fooID: Int

    var body: some View {
        // The two print() calls are for debugging only.
        print(Self._printChanges())
        print(fooID)

        return VStack {
            Text("\(dataModel.get(fooID).value)")
            Button("Delete It") {
                dataModel.remove(fooID)
            }
        }
    }
}

struct ContentView: View {
    var body: some View {
        ListView()
    }
}

Below is my analysis of the root cause of the crash:

  • When the "Delete It" button gets clicked, it removes the item from data model.

  • Since the detail view accesses data model through @EnvironmentObject, the data model change triggers call of detail view's body.

  • It turns out that the fooID binding in the detail view doesn't get updated at the time, and hence DataModel.get(id:) call crashes the app.

I didn't expect the crash, because I thought the fooID binding value in the detail view would get updated before the detail view's body gets called. In my opinion the current behavior is a severe bug because it breaks the data consistency assumption. But when I google about this I find nothing.

I considered a few approaches to solve or work around the issue.

  1. Don't use @EnvironmentObject

This is the approach used in Apple's official tutorial, which passes binding all the way from list view to detail view and to edit view and uses no environment object. The approach is simple and elegant.

But unfortunately, in my opinion, this approach only works for simple data model. For example, if the item to be displayed in the detail view contains not only its own value but also the id of a related item. To show the basic information of that related item, we need to access data model. In general, I don't think it's always feasible to prepare all data needed by the detail view ahead in practical applications with complex data models..

  1. Validate param before calling data model API

In this approach, I accept the fact that binding value and the data model in environment object can be inconsistent so I validate binding value before passing it to data model API. However, if the binding value needs to be validated, there is no point in using it. Why not just use the regular property instead? So I end up with code like the following:

struct DetailView: View {
    @EnvironmentObject var dataModel: DataModel
    // No need to use binding.
    var fooID: Int

    var body: some View {
        // Note I modified DataModel.get() to return optional value.
        if let foo = dataModel.get(fooID) {
            VStack {
                Text("\(foo.value)")
                Button("Delete It") {
                    dataModel.remove(foo.id)
                }
            }
        }
    }
}

(In my App's code, I introduced view modifier and view wrapper to encapsulate the if statement and to also validate states in the view. But the basic idea is the above.)

The approach works well. My only concern is that it's very different from the approach used in Apple's demo apps (I'm aware there are many different ways to pass data between views though). Also, I'd hope to use Apple recommended approach (if it's possible) because it's simple and elegant. Since this is a common scenario, I can hardly believe it's just me running into it. I wonder how the others understand the issue and address it? Thanks.

It's not clear why you are passing binding instead of normal value.
Also it's not clear what do you want to show in detail view during the split second when the current item is deleted and the view is still visible (I think it would be reasonable to show the deleted item).

I'd do just this
import SwiftUI

struct Foo: Identifiable {
    var id: Int
    var value: Int
}

class DataModel: ObservableObject {
    static let singleton = DataModel()
    private init() {}
    
    @Published var foos = [Foo(id: 1, value: 1), Foo(id: 2, value: 2)]

    func remove(_ id: Int) {
        foos.removeAll { $0.id == id }
    }
}

struct ListView: View {
    @ObservedObject private var dataModel = DataModel.singleton

    var body: some View {
        NavigationView {
            List {
                ForEach(dataModel.foos) { foo in
                    NavigationLink {
                        DetailView(foo: foo)
                    } label: {
                        Text("\(foo.value)")
                    }
                }
            }
        }
    }
}

struct DetailView: View {
    let foo: Foo

    var body: some View {
        VStack {
            Text(String(foo.value))
            Button("Delete It") {
                // as the view doesn't depend on DataModel to render itself
                // it ok to not have DataModel as Observed value / parameter
                DataModel.singleton.remove(foo.id)
            }
        }
    }
}

struct ContentView: View {
    var body: some View {
        ListView()
    }
}
1 Like

It didn't occur to me to use binding at first. I was just looking for a robust way. My first thought was something like the code in my approach 2 (but I didn't do the if check at that time). After observing the crash, I tried a lot of other approaches, including using ObservableObject. During the course I found Apple's official tutorial (you can find its link in my original post above), and noticed it used binding to pass data among views. So I thought using binding might be able to resolve the data consistency issue. It turned out it crashed too.

It's not me who want to show it. It's SwiftUI. It's all due to this piece of the code in the detail view:

@EnvironmentObject var dataModel: DataModel

I added it because I want to make sure the detail view redraws itself when user changes the item (for example, in a edit view that is pushed into the navigation view stack after the detail view). The code causes the SwiftUI to call the detail view body whenever the data model changes. Ideally I would hope SwiftUI to call the list view's body first, which should remove the detail view (since the item is deleted) and hence the detail view's body doesn't get called (and hence no crash at all). But the actual result shows that's not the way how SwiftUI works (it calls the detail view's body first and hence the crash).

Thanks, but I believe the code is buggy (I will be more than happy to be proved wrong).

Let's think about this scenario:

The Foo struct contains an id of another struct Bar. And we want to display bar item name in foo item detail view. Also user can click the bar item name to go to bar item detail view, and then to its edit view. The view stack is like the following:

list view -> foo detail view -> bar detail view -> bar edit view

Now user modifies bar's name and presses BACK button to go back to foo detail view. Will bar item name in the foo item detail view gets updated? No, it won't! It's because:

  • First, since the detail view in your code contain just regular properties, it won't redraw itself when data model changes.
  • The list view will redraw itself. And it will regenerate the detail view during the course. However, as the detail view contains only foo property, which doesn't change (note that foo just contain bar's id not its name), so the default implementation of EqutableView in SwiftUI can't detect the change and wouldn't call the detail view's body to redraw it.

(Note: by "redraw itself" I mean calling the view's body.)

This potential bug is why I think it's important to add the following code in every view (unless the caller can pass all the data to the detail view. But as I explained in my original post, I don't think that's practical). But unfortunately, adding the following code causes the crash issue.

@EnvironmentObject var dataModel: DataModel

That's the reason I wonder how the others do it. Hope that explains it. Thanks.

PS: I also experimented on implementing view model (by using ObserverableObject) for each view. But I don't like that approach for various reasons, among which one minor reason is that it's verbose and doesn't feel like a SwiftUI way.

SwiftUI redraws the view and you supply the data. It is up to you what you supply during that split second when the model data is deleted. E.g. you supply red color. Or you supply the old data (in which case you need to have it). Totally up to you.

The code is not buggy, you've just changed the rules mid game, and my answer was against your original question.

In your new scenario Foo view will be redrawn properly: because it needs to show bar's name -> and it only has bar's id -> hence it needs to get bar from the model -> hence it needs to depend on the model. Alternatively you put Bar inside Foo unless of course you are mimicking reference semantic with that foo -> barId relationship.

Example implementation for your new scenario.
import SwiftUI

struct Foo: Identifiable {
    let id: Int
    let name: String
    let barId: Int
}

struct Bar: Identifiable {
    let id: Int
    var name: String
}

struct MyState {
    var foos: [Foo] = [Foo(id: 1, name: "Foo1", barId: 1), Foo(id: 2, name: "Foo2", barId: 2)]
    var bars: [Int: Bar] = [1 : Bar(id: 1, name: "Bar1"), 2 : Bar(id: 2, name: "Bar2")]
}

class Model: ObservableObject {
    static let singleton = Model()
    private init() {}
    
    @Published var state = MyState()

    func remove(_ id: Int) {
        state.foos.removeAll { $0.id == id }
    }
    
    func bar(for id: Int) -> Binding<Bar> {
        .init { [self] in
            state.bars[id]!
        } set: { [self] newValue in
            state.bars[id] = newValue
        }
    }
}

struct BarEditView: View {
    @Binding var bar: Bar
    
    var body: some View {
        TextField("edit", text: $bar.name)
    }
}

struct BarDetailView: View {
    @Binding var bar: Bar
    
    var body: some View {
        NavigationLink {
            BarEditView(bar: $bar)
        } label: {
            Text(bar.name)
        }
    }
}

struct FooDetailView: View {
    @ObservedObject private var model = Model.singleton
    let foo: Foo

    var body: some View {
        let bar = model.bar(for: foo.barId)
        
        VStack(spacing: 20) {
            Text(foo.name)
            Text(bar.wrappedValue.name)

            NavigationLink("Go to bar") {
                BarDetailView(bar: bar)
            }
            
            Button("Delete It") {
                model.remove(foo.id)
            }
        }
    }
}

struct FooView: View {
    let foo: Foo

    var body: some View {
        NavigationLink(foo.name) {
            FooDetailView(foo: foo)
        }
    }
}

struct ListView: View {
    @ObservedObject private var model = Model.singleton

    var body: some View {
        NavigationView {
            List {
                ForEach(model.state.foos) { foo in
                    FooView(foo: foo)
                }
            }
        }
    }
}

struct ContentView: View {
    var body: some View {
        ListView()
    }
}

Note, there are more than one way to skin that cat, e.g. instead of using bindings in BarEditView and BarDetailView you may use @ObservedObject in there.

Note2: error handling (including better optional unwrapping) is omitted for simplicity.

What I meant was that, while SwiftUI was advertised as a state driven architecture, it's not really that simple. There are unspoken implementation details that affect app behavior and may cause crash.

I appreciate your code and the discussion. I agree I changed the requirement. But it's just a simple change and you had to work out a completely different way to pass data between views. Do you see the issue? With all due respect to SwiftUI developers (it's a great UI framework, thank!), the ways to managing states in SwiftUI (I mean, passing data between views) are quite "fragile", in the sense that a simple change to data model may break the existing code and require a complete rework on how to manage states.

I'll demonstrate below how your new code will crash by just adding a simple new requirement to data model. Please read on.

Yes, that's the purpose of the setup. The issue is how to find a robust way to implement the dependence in SwiftUI.

If we put Bar inside Foo, then all the issues are gone. The approach used in Apple demo app will just work fine. The issue is it's not always feasible to implement data model that way. One example is a transfer between two bank accounts. It's not appropriate to put transfer in either account. You have to represent the relation between a transfer and its two accounts by using id.

Yes, the code works fine. But a simple change to the data model will crash it. For example, let's suppose, when we delete foo, we also need to delete bar it refers to. This is a quite reasonable requirement. A real world example: think foo as bank account and bar as transfers from/to this account.

Below is the diff. Clicking on the "Delete It" button in the detail view will crash it.

    func remove(_ id: Int) {
+       // First, remove the bar referenced by the the foo.
+       let foo = state.foos.first(where: { $0.id == id })!
+       state.bars[foo.barId] = nil
        state.foos.removeAll { $0.id == id }
    }

Hello,

Please do not ask SwiftUI questions on these forums. You seem to be aware that it is not the correct place for them. Currently, almost every other post I'm seeing is about Apple's proprietary frameworks. Please do not make the situation worse by setting a bad example.

It is unfortunate that you could not find a satisfactory answer elsewhere, but these forums are still not the correct place to ask.

Thank you.

The change can be simple, e.g. instead of Foo having barId you change it to have Bar itself (or vice versa). Or it was one to one relationship and you changed it to be one to many. Or you changed an array to a dictionary, etc. However simple those changes are you'll need to make the relevant changes to the rest of the code including SwiftUI related parts. Is that a problem? I do not think so.

With this change after the line marked *** you painted yourself into a situation where foo has some barId and there is no bar with such id. I would not recommend doing this as written. I'd do:

  • make Foo having an optional barId
struct Foo: Identifiable {
    let id: Int
    let name: String
    var barId: Int? // Now optional
}
  • Before you remove bar set foo's barId to nil to not have a situation where foo references some non existing bar:

      func remove(_ fooId: Int) {
          if let fooIndex = (state.foos.firstIndex { $0.id == fooId }) {
              if let barId = state.foos[fooIndex].barId {
                  state.foos[fooIndex].barId = nil // first this
                  state.bars[barId] = nil // then that
              }
              state.foos.remove(at: fooIndex)
          }
      }
    
  • make the bar binding returning method returning optional:

      func bar(for id: Int?) -> Binding<Bar>? {
          guard let id = id else { return nil }
          guard let bar = state.bars[id] else { return nil }
          return .init {
              bar
          } set: { [self] newValue in
              state.bars[id] = newValue
          }
      }
    
  • and handle missing bar in UI (e.g. this shows red color):

              if let bar = bar {
                  Text(bar.wrappedValue.name)
                  
                  NavigationLink("Go to bar") {
                      BarDetailView(bar: bar)
                  }
              } else {
                  Color.red.frame(width: 100, height: 50)
              }
    
Full version
import SwiftUI

struct Foo: Identifiable {
    let id: Int
    let name: String
    var barId: Int?
}

struct Bar: Identifiable {
    let id: Int
    var name: String
}

struct MyState {
    var foos: [Foo] = [Foo(id: 1, name: "Foo1", barId: 1), Foo(id: 2, name: "Foo2", barId: 2)]
    var bars: [Int: Bar] = [1 : Bar(id: 1, name: "Bar1"), 2 : Bar(id: 2, name: "Bar2")]
}

class Model: ObservableObject {
    static let singleton = Model()
    private init() {}
    
    @Published var state = MyState()

    func remove(_ fooId: Int) {
        if let fooIndex = (state.foos.firstIndex { $0.id == fooId }) {
            if let barId = state.foos[fooIndex].barId {
                state.foos[fooIndex].barId = nil // first this
                state.bars[barId] = nil // then that
            }
            state.foos.remove(at: fooIndex)
        }
    }
    
    func bar(for id: Int?) -> Binding<Bar>? {
        guard let id = id else { return nil }
        guard let bar = state.bars[id] else { return nil }
        return .init {
            bar
        } set: { [self] newValue in
            state.bars[id] = newValue
        }
    }
}

struct BarEditView: View {
    @Binding var bar: Bar
    
    var body: some View {
        TextField("edit", text: $bar.name)
    }
}

struct BarDetailView: View {
    @Binding var bar: Bar
    
    var body: some View {
        NavigationLink {
            BarEditView(bar: $bar)
        } label: {
            Text(bar.name)
        }
    }
}

struct FooDetailView: View {
    @ObservedObject private var model = Model.singleton
    let foo: Foo

    var body: some View {
        let bar = model.bar(for: foo.barId)
        
        VStack(spacing: 20) {
            Text(foo.name)
            
            if let bar = bar {
                Text(bar.wrappedValue.name)
                
                NavigationLink("Go to bar") {
                    BarDetailView(bar: bar)
                }
            } else {
                Color.red.frame(width: 100, height: 50)
            }
            
            Button("Delete It") {
                model.remove(foo.id)
            }
        }
    }
}

struct FooView: View {
    let foo: Foo

    var body: some View {
        NavigationLink(foo.name) {
            FooDetailView(foo: foo)
        }
    }
}

struct ListView: View {
    @ObservedObject private var model = Model.singleton

    var body: some View {
        NavigationView {
            List {
                ForEach(model.state.foos) { foo in
                    FooView(foo: foo)
                }
            }
        }
    }
}

struct ContentView: View {
    var body: some View {
        ListView()
    }
}
1 Like

It's true that the other part needs to change accordingly. But I don't expect a huge change (e.g. an architecture level change). For example, the way to pass data in your first code and that in your second code are very different.

I see your point. Though I don't think it matters in this case because the entire function is an "atomic" operation. I don't usually use optional type for this purpose, but thanks for the suggestion.

Did you notice you have come to the same conclusion as I? :grinning: Note the use of if statement. That's same as my approach 2. This is not a typical programming style you find in SwiftUI (at least not in Apple demos). But in the end of the day if one wants to write robust code, it has to be this way.

Since there are people unhappy about the SwiftUI topics, let's end the discussion (though I don't object if you have more comments). Thanks for the discussion. It's quite helpful and let me know how the others doing SwiftUI programming.

Just in case you're interested, my post in Apple developer forum is at:

https://developer.apple.com/forums/thread/699855

It's also worth pointing out that most of the discussion is about how to write SwiftUI program, instead of my original question (the inconsistency of the data in @EnvironmentObject and the data in @Binding). It seems we have to take it as the expected behavior unless Apple engineers give a definitive answer.