pointfreeco / swift-identified-collections

A library of data structures for working with collections of identifiable elements in an ergonomic, performant way.
MIT License
539 stars 46 forks source link

IdentifiedArrayOf does not work correctly with @ObservableState #78

Closed alexataman closed 4 weeks ago

alexataman commented 1 month ago

Description

Hi there!

I’m encountering an issue when using IdentifiedArrayOf with @ObservableState. My project maintains an IdentifiedArrayOf to manage the state, and I have two main actions:

•   windowWasOpened(Window)
•   windowWasClosed(Window)

Here are the actions:

case .windowWasOpened(let window):
    state.windows.append(window)
    switch window {
    case .collection(let documentModel):
        state.states[id: documentModel.id.asString] = DocumentReducer.State(documentModel: documentModel)
    case .welcome:
        break
    }

case .windowWasClosed(let window):
    guard let index = state.windows.firstIndex(of: window) else {
        fatalError("Window \(window) does not exist in array")
    }
    state.windows.remove(at: index)
    switch window {
    case .collection(let documentModel):
        state.states.remove(id: documentModel.id.asString)
    case .welcome:
        break
    }

My State:

@ObservableState
struct State: Equatable {
    static func == (lhs: AppReducer.State, rhs: AppReducer.State) -> Bool {
        return lhs.windows == rhs.windows
    }

    var windows: [Window] = []
    var themeManager: ThemeManager
    var states: IdentifiedArrayOf<DocumentReducer.State> = []
}

DocumentReducer State:

@Reducer
struct DocumentReducer {

    @ObservableState
    struct State: Equatable, Identifiable {
        let documentModel: DocumentModel
        var id: String {
            documentModel.id.asString
        }
    }
   /...Some Code.../
}

Screenshot:

Знімок екрана 2024-10-16 о 16 16 20

Can you help identify the root cause of this issue?

Checklist

Expected behavior

When removing the state from the IdentifiedArrayOf, it should correctly clean up without causing crashes or referencing deleted state.

Actual behavior

When I add a new state to the array, everything works as expected. However, when I remove the state from the IdentifiedArrayOf, it causes a crash inside the PartialToState enum. It seems that @ObservableState and IdentifiedArrayOf do not work well together when performing deletions.

Steps to reproduce

  1. Add a new window with a unique DocumentModel to the IdentifiedArrayOf via the windowWasOpened action.
  2. Close that window by calling the windowWasClosed action and removing the corresponding state from IdentifiedArrayOf.
  3. The app crashes when trying to access the removed state.

Identified Collections version information

No response

Destination operating system

macOS 14.3.1 (23D60)

Xcode version information

Version 15.3

Swift Compiler version information

swift-driver version: 1.90.11.1 Apple Swift version 5.10 (swiftlang-5.10.0.13 clang-1500.3.9.4)
Target: arm64-apple-macosx14.0
stephencelis commented 1 month ago

@alexataman Can you attach a project that reproduces the problem? You've posted code snippets with sections omitted, and the fewer steps we have to take to reproduce the better chance we can look into the issue and address it.

alexataman commented 4 weeks ago

@stephencelis Sure! Here is the project that reproduces the issue.

The project was created on macOS 14.6.1 (23G93) using Xcode Version 16.0 (16A242d) and the main branch of TCA, commit fc5cbeec88114ff987f6c3cad3a7f3a3713fdb56.

Crash.zip

stephencelis commented 4 weeks ago

@alexataman You're using an unsafe !-unwrapping operation on the store.scope you pass to DocumentView. If you update to the following, safer scoping operation, it prevents the crash for me:

var body: some Scene {
  DocumentGroup(newDocument: CrashDocument()) { configuration in
    let documentModel = configuration.document.documentModel!
    if let docStore = store.scope(
      state: \.document[id: documentModel.id],
      action: \.document[id: documentModel.id]
    ) {
      DocumentView(
        store: docStore
      )
      .onDisappear {
        store.send(.windowWasClosed(documentModel))
      }
    } else {
      Text("Loading...")
        .onAppear {
          store.send(.windowWasOpened(documentModel))
        }
    }
  }
}

Because this isn't a bug with the library I'm going to convert this issue to a discussion.