1

I am new to Swift/SwiftUI and am trying to build an app that works with the trello API.

There is a "TrelloApi" class that is available as an @EnvironmentObject in the entire app. The same class is also used to make API calls.

One board is viewed at a time. A board has many lists and each list has many cards.

Now I have an issue with my rendering where whenever I switch boards and any list in the new board has fewer cards in it than before, I get the following error in an onReceive handler where I need to do some checks to update the cards appearance:

Swift/ContiguousArrayBuffer.swift:575: Fatal error: Index out of range
2022-10-19 09:04:11.319982+0200 trello[97617:17713580] Swift/ContiguousArrayBuffer.swift:575: Fatal error: Index out of range

Models

struct BoardPrefs: Codable {
    var backgroundImage: String? = "";
}

struct BasicBoard: Identifiable, Codable {
    var id: String;
    var name: String;
    var prefs: BoardPrefs;
}

struct Board: Identifiable, Codable {
    var id: String;
    var name: String;
    var prefs: BoardPrefs;
    
    var lists: [List] = [];
    var cards: [Card] = [];
    var labels: [Label] = [];
    
}

struct List: Identifiable, Codable, Hashable {
    var id: String;
    var name: String;
    var cards: [Card] = [];
    
    private enum CodingKeys: String, CodingKey {
        case id
        case name
    }
}

struct Card: Identifiable, Codable, Hashable {
    var id: String;
    var idList: String = "";
    var labels: [Label] = [];
    var idLabels: [String] = [];
    var name: String;
    var desc: String = "";
    var due: String?;
    var dueComplete: Bool = false;
}

TrelloApi.swift (HTTP call removed for simplicity)

class TrelloApi: ObservableObject {
    let key: String;
    let token: String;
    
    @Published var board: Board;
    @Published var boards: [BasicBoard];
    
    init(key: String, token: String) {
        self.key = key
        self.token = token
        self.board = Board(id: "", name: "", prefs: BoardPrefs())
        self.boards = []
    }
    
    func getBoard(id: String, completion: @escaping (Board) -> Void = { board in }) {
        if id == "board-1" {
            self.board = Board(id: "board-1", name: "board-1", prefs: BoardPrefs(), lists: [
                List(id: "board-1-list-1", name: "board-1-list-1", cards: [
                    Card(id: "b1-l1-card1", name: "b1-l1-card1"),
                ]),
                List(id: "board-1-list-2", name: "board-1-list-2", cards: [
                    Card(id: "b1-l2-card1", name: "b1-l2-card1"),
                    Card(id: "b1-l2-card2", name: "b1-l2-card2"),
                ])
            ])
            
            completion(self.board)
        } else {
            self.board = Board(id: "board-2", name: "board-2", prefs: BoardPrefs(), lists: [
                List(id: "board-2-list-1", name: "board-2-list-1", cards: [
                ]),
                List(id: "board-2-list-2", name: "board-2-list-2", cards: [
                    Card(id: "b2-l2-card1", name: "b2-l2-card1"),
                ])
            ])
            
            completion(self.board)
        }
    }
}

ContentView.swift

struct ContentView: View {
    @EnvironmentObject var trelloApi: TrelloApi;
    
    var body: some View {
        HStack {
            VStack {
                Text("Switch Board")
                Button(action: {
                    trelloApi.getBoard(id: "board-1")
                }) {
                    Text("board 1")
                }
                Button(action: {
                    trelloApi.getBoard(id: "board-2")
                }) {
                    Text("board 2")
                }
            }
            VStack {
                ScrollView([.horizontal]) {
                    ScrollView([.vertical]) {
                        VStack(){
                            HStack(alignment: .top) {
                                ForEach($trelloApi.board.lists) { list in
                                    TrelloListView(list: list)
                                        .fixedSize(horizontal: false, vertical: true)
                                }
                            }
                            .padding()
                            .frame(maxHeight: .infinity, alignment: .top)
                        }
                    }
                }
            }
        }.onAppear {
            trelloApi.getBoard(id: "board-1")
        }
        .frame(minWidth: 900, minHeight: 600, alignment: .top)
    }
}

TrelloListView.swift

struct TrelloListView: View {
    @EnvironmentObject var trelloApi: TrelloApi;
    @Binding var list: List;
    
    var body: some View {
        VStack() {
            Text(self.list.name)
            Divider()
            SwiftUI.List(self.$list.cards, id: \.id) { card in
                CardView(card: card)
                
            }
            .listStyle(.plain)
            .frame(minHeight: 200)
        }
        .padding(4)
        .cornerRadius(8)
        .frame(minWidth: 200)
    }
}

CardView.swift

struct CardView: View {
    @EnvironmentObject var trelloApi: TrelloApi;
    
    @Binding var card: Card;
    
    var body: some View {
        VStack(alignment: .leading) {
            HStack {
                VStack(alignment: .leading, spacing: 0) {
                    Text(card.name)
                        .bold()
                        .font(.system(size: 14))
                        .multilineTextAlignment(.leading)
                        .lineLimit(1)
                        .foregroundColor(.white)
                    
                    Text(card.desc)
                        .lineLimit(1)
                        .foregroundColor(.secondary)
                }.padding()
                Spacer()
            }
        }
        .frame(alignment: .leading)
        .onReceive(Just(card)) { newCard in
            // CRASH LOCATION: "Index out of range" for self.card.labels
            if self.card.labels != newCard.labels {
                print("(check if card color should change based on labels)")
            }
        }
        .cornerRadius(4)
    }
}

I've highlighted the crash location with a comment. I don't pass any indexes in the ForEach or List and I am overwriting the entire trelloApi.board object, so I am not sure why I am getting this error.

I've tried using ForEach inside of the SwiftUI.List instead, but that also doesn't change anything.

The minimal reproducible code can also be found on my GitHub repo: https://github.com/Rukenshia/trello/tree/troubleshooting-board-switch-crash/trello

Jan
  • 1,268
  • 4
  • 12
  • 20
  • You should check if card.labels is empty or not. And then proceed from there. – Lawrence Gimenez Oct 19 '22 at 07:46
  • @LawrenceGimenez this doesn't seem to change anything - XCode highlights the exception on "card.labels", so if I add a check for `if card.labels.count > 0 { ... }`, it throws the same exception anyway. – Jan Oct 19 '22 at 07:53
  • 3
    Why are you even using onReceive here, isn’t it enough that `card` is declared as a Binding? – Joakim Danielson Oct 19 '22 at 07:57
  • @JoakimDanielson I removed most of the code for a simple reproducible example - I use this in the real code to update the cards background color depending on labels: https://github.com/Rukenshia/trello/blob/main/trello/views/CardView.swift#L258 – Jan Oct 19 '22 at 08:02
  • 1
    Yes I know it doesn't change anything, I am trying to step by step help you on debugging. Thanks for the response. I don't usually use onReceive but I use task. – Lawrence Gimenez Oct 19 '22 at 08:09
  • Appreciate your help! I used onReceive originally because the card could change in the background (for example when trello automations run), so I thought onReceive would be useful for when the card changes. Is this something I could also achieve with `.task`? I haven't heard of or used it before and just looked at the documentation but I'm not sure how I could use it to react to changes in the Binding. – Jan Oct 19 '22 at 10:34
  • what is a `Label` (in Card), is it your own struct or the SwiftUI `Label` view? If it is your own struct, then change the name, it may interfere with the SwiftUI `Label`. Similarly for `List`. P.S, you can remove all `;`, these are from a distant past. – workingdog support Ukraine Oct 20 '22 at 05:09
  • @workingdogsupportUkraine Label is my own struct, good hint I've updated the naming of all of them but sadly didn't change anything. Also thanks for the hint on semicolons, didn't know that! – Jan Oct 20 '22 at 07:52
  • 1
    That `.onReceive()` you use with a `Just` publisher created inline in the function call and which observes a binding is very suspicious. By definition, the view is already redrawn when the `@Binding` value changes. There is too much state in your code, try to refactor into smaller views and use `ObservableObjects` to model view state (such as `CardView`). – Louis Lac Oct 21 '22 at 13:01
  • 2
    Also, your code is scattered with anti-patterns such as `@State private var isHovering: Bool`. `@State` properties should always have an initial value and you should not be manually set in the view `init`. If you need to update the value as soon as the view appears, use the `.onAppear()` or `.task()` modifier. – Louis Lac Oct 21 '22 at 13:03
  • @LouisLac thanks! This cleared it up a lot. I've just started refactoring it and got things mostly re-implemented now. The anti-patterns you're speaking of, is there a comprehensive list somewhere so I could learn more about it? I think I also can't mark your comment as the answer - would you mind posting one so I can award you the bounty? – Jan Oct 21 '22 at 13:48
  • @Jan I wrote a detailed answer. Unfortunately I don't have any good post listing anti-patterns in mind but I recommend the Apple SwiftUI documentations and the free tutorials! – Louis Lac Oct 21 '22 at 15:05
  • @Jan I think I solved the crash issue. Take a look at my answer, I updated it with the solution. – Louis Lac Oct 22 '22 at 15:30

1 Answers1

2

The exact issue is hard to track down, but here are some observations and recommandations.

The .onReceive() modifier you are using looks suspicious because you initialize the publisher yourself inline in the function call. You generally use .onReceive() to react to events published from publishers set up by another piece of code.

Moreover, you are using this .onReceive() to react to changes in a @Binding property, which is redundant since by definition a @Binding already triggers view updates when its value changes.


EDIT

This seems to be the issue that causes the crash in your app. Changing the .onReceive() to .onChange() seems to solve the problem:

.onChange(of: card) { newCard in
  if self.card.labels != newCard.labels {
    print("(check if card color should change based on labels)")
  }
}

You also seem to duplicate some state:

.onReceive(Just(card)) { newCard in
  self.due = newCard.dueDate
}

Here, you duplicated the due date, there is one copy in self.due and another copy in self.card.dueDate. In SwiftUI there should only be one source of truth and for you it would be the card property. You duplicated the state in the init: self.due = card.wrappedValue.dueDate. Accessing the .wrappedValue of a @Binding/State is a code smell and the sign that you are doing something wrong.

Lastly, ou use an anti-pattern which can be dangerous:

struct CardView: View {
  @State private var isHovering: Bool
  
  func init(isHovering: String) {
    self._isHovering = State(initialValue: false)
  }

  var body: some View { 
    ...
  }
}

You should avoid initializing a @State property wrapper yourself in the view's init. A @State property must be initililized inline:

struct CardView: View {
  @State private var isHovering: Bool = false

  var body: some View { 
    ...
  }
}

If for some reason you have to customize the value of a @State property, you could use the .onAppear() or the newer .task() view modifier to change its value after the view creation:

struct CardView: View {
  @State private var isHovering: Bool = false

  var body: some View { 
    SomeView()
      .onAppear {
        isHovering = Bool.random()
      }
  }
}

As a general advice you should break up your views into smaller pieces. When a view depends on many @State properties and has lots of .onChange() or .onReceive() it is usually an indication that it is time to move the whole logic inside and ObservableObject or refactor into smaller components.

Louis Lac
  • 5,298
  • 1
  • 21
  • 36