IdentifiedArray Equatable issues due to KeyPath

[sroche27r]

While using TCA we are running into issues where our tests are failing equality tests. The culprits are IdentifiedArray that use different initializers.

Here is some super simplified code that illustrates the problem:

class StateTests: XCTestCase {
    func testStateEquality() {
        struct SomeItem: Identifiable, Equatable {
            var id: String = "item-id"
        }
        struct State: Equatable {
            var items = IdentifiedArray<String, SomeItem>(id: \.id)
        }
        // this happens in the reducer
        var state = State()
        state.items.append(SomeItem())

        // this happens in our test
        let expectedState = State(items: .init(uniqueElements: [SomeItem()]))
        XCTAssertEqual(state, expectedState, "This is going to fail :(")
    }
}

This code looks sensible and I would assume that the test would pass. But the problem is if you use an initializer that has a KeyPath input parameter vs. one that uses generics to create the KeyPath, then the equality comparison will fail, even if they have all of the same elements. The reason is that the comparison of the KeyPaths fails.

Here is a quick unit test that shows the issue

class KeyPathTests: XCTestCase {
    func testKeyPathEquality() {
        struct Struct: Identifiable {
            let id: String = "id"
        }
        func problem<T: Identifiable>(keyPath: KeyPath<T, T.ID>) {
            let keyPath1: KeyPath<T, T.ID> = \.id
            XCTAssertEqual(keyPath1, keyPath, "This is going to fail :(")
        }
        problem(keyPath: \Struct.id)
    }
}

Unfortunately, there doesn't seem to be a proper fix for this.

I would propose one of the following solutions: option 1:
Remove the key path comparison in the Equatable extension. Wouldn't equality of the elements or dictionary be enough?

option 2: Deprecate all the inits that don't have a key path parameter. Force developers to always specify the keypath.

Hopefully I missed something and there is a proper fix for this.

[mbrandonw]

Wow, good catch! That seems really strange to us. I had to ask Joe Groff about it on twitter to see if that was expected.

We think it's probably ok to drop the id key path equality check in the Equatable conformance for now. Do you want to PR that or would you rather us take care of it? It'd also be nice to get a test case in place for this situation too.

[sroche27r]

Cool, I'll submit a PR