Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem with find and view with generics #219

Closed
Tyler-Keith-Thompson opened this issue Dec 27, 2022 · 9 comments
Closed

Problem with find and view with generics #219

Tyler-Keith-Thompson opened this issue Dec 27, 2022 · 9 comments
Labels
bug Something isn't working

Comments

@Tyler-Keith-Thompson
Copy link
Contributor

Before I get to the problem let me explain my motivation. The library I'm writing handles navigation, so the tests for it involve creating an absurd number of views that are unique to a given test. The problem is that find (and, for that matter, some other inspection methods) no longer work due to the regex check on type names.

Here is a test that reproduces the problem:

func testViewContainerWithNestedGenericParameter() throws {
    struct ViewWrapper<V: View>: View {
        @ViewBuilder var view: () -> V
        init(@ViewBuilder view: @escaping () -> V) {
            self.view = view
        }
        var body: some View {
            view()
        }
    }
    let sut = try ViewWrapper {
        ViewWrapper {
            Text("findme")
        }
    }.inspect()
    XCTAssertNoThrow(try sut.find(ViewWrapper<ViewWrapper<Text>>.self))
    XCTAssertNoThrow(try sut.find(ViewWrapper<Text>.self))
}

Here is the commit that caused the problem: 22a48ce

I've verified in a fork that reverting this commit gets me back to the old, functioning behavior.

I have another problem with find and generics that likely has a similar cause, but I'm having trouble creating a minimally reproducible example.

@nalexn
Copy link
Owner

nalexn commented Dec 27, 2022

Hey, yeah, I'll have a look. Before I get a hotfix version out I would like to resolve the other issue you mentioned with regards to the find functionality, so please elaborate on the details in a separate ticket or here

@Tyler-Keith-Thompson
Copy link
Contributor Author

Please forgive this wildly complex test. You'll notice the ViewWrapper type is now outside the test, but the views it wraps are inside it, this reasonably reproduces what I'm seeing. Wish I could simplify further, but it's been a miracle to get it this far.

func testViewContainerWithNestedGenericParameter() throws {
    struct CustomView: View {
        var body: some View { Text("\(String(describing: Self.self))") }
    }
    struct CustomView2: View {
        var body: some View { Text("\(String(describing: Self.self))") }
    }
    let view = ViewWrapper(current: CustomView()) {
        ViewWrapper(current: CustomView2())
    }
    let initialExp = expectation(description: "Inspected view before state change")
    view.inspection.inspect { sut in
        XCTAssertNoThrow(try sut.find(ViewWrapper<CustomView, ViewWrapper<CustomView2, Never>>.self))
        XCTAssertNoThrow(try sut.find(ViewWrapper<CustomView2, Never>.self))
        XCTAssertNoThrow(try sut.find(CustomView.self)) // This fails, but shouldn't
        initialExp.fulfill()
    }
    let finalExp = expectation(description: "Inspected view after state change")
    view.inspection.inspect(onReceive: Self.proceedSubject) { sut in
        XCTAssertNoThrow(try sut.find(CustomView2.self)) // This fails, but shouldn't
        finalExp.fulfill()
    }

    ViewHosting.host(view: view)

    wait(for: [initialExp], timeout: 0.3)

    Self.proceedSubject.send()

    wait(for: [finalExp], timeout: 0.3)
}

static let proceedSubject = PassthroughSubject<Void, Never>()
struct ViewWrapper<Current: View, Next: View>: View {
    @State var isShowingNext = false
    @State private var current: Current
    @State private var next: Next? = nil

    let inspection = Inspection<Self>()

    init(current: Current) where Next == Never {
        _current = State(wrappedValue: current)
    }
    init(current: Current, next: @escaping () -> Next) {
        _current = State(wrappedValue: current)
        _next = State(wrappedValue: next())
    }

    var body: some View {
        Group {
            if !isShowingNext {
                current
            } else if let next {
                next
            }
        }
        .onReceive(CustomViewTests.proceedSubject) {
            isShowingNext = true
        }
        .onReceive(inspection.notice) { inspection.visit(self, $0) }
    }
}

@nalexn
Copy link
Owner

nalexn commented Dec 30, 2022

Hey @Tyler-Keith-Thompson , could you verify locally the following fix.

Without reverting 22a48ce, replace in Inspector.swift

if let range = str.range(of: ".(unknown context at ")

with

while let range = str.range(of: ".(unknown context at ")

This fixes the provided same tests for me, though I want to make sure it works for your original setup.

@Tyler-Keith-Thompson
Copy link
Contributor Author

Tyler-Keith-Thompson commented Dec 30, 2022

Okay, your fix works but there's still a goofy thing with find I think. I believe my more complex test proves it...

func testViewContainerWithNestedGenericParameter() throws {
    struct CustomView: View {
        var body: some View { Text("\(String(describing: Self.self))") }
    }
    struct CustomView2: View {
        var body: some View { Text("\(String(describing: Self.self))") }
    }
    let view = ViewWrapper(current: CustomView()) {
        ViewWrapper(current: CustomView2())
    }
    let initialExp = expectation(description: "Inspected view before state change")
    view.inspection.inspect { sut in
        XCTAssertNoThrow(try sut.find(ViewWrapper<CustomView, ViewWrapper<CustomView2, Never>>.self))
        XCTAssertThrowsError(try sut.find(ViewWrapper<CustomView2, Never>.self)) // This should throw, because the view isn't there...the if/else condition should stop it from being there.
        // print try sut.find(ViewWrapper<CustomView2, Never>.self).pathToRoot and you get view(ViewWrapper<EmptyView>.self). Which is why it's able to find the view that isn't there, I think.
        initialExp.fulfill()
    }

    ViewHosting.host(view: view)

    wait(for: [initialExp], timeout: 0.3)
}

static let proceedSubject = PassthroughSubject<Void, Never>()
struct ViewWrapper<Current: View, Next: View>: View {
    @State var isShowingNext = false
    @State private var current: Current
    @State private var next: Next? = nil

    let inspection = Inspection<Self>()

    init(current: Current) where Next == Never {
        _current = State(wrappedValue: current)
    }
    init(current: Current, next: @escaping () -> Next) {
        _current = State(wrappedValue: current)
        _next = State(wrappedValue: next())
    }

    var body: some View {
        Group {
            if !isShowingNext {
                current
            } else if let next {
                next
            }
        }
        .onReceive(CustomViewTests.proceedSubject) {
            isShowingNext = true
        }
        .onReceive(inspection.notice) { inspection.visit(self, $0) }
    }
}

@Tyler-Keith-Thompson
Copy link
Contributor Author

This also means that find will find the wrong view because it isn't respecting the generic specialization. This can at least be temporarily worked around by using skipFound

@nalexn
Copy link
Owner

nalexn commented Jan 6, 2023

@Tyler-Keith-Thompson it seems that ignoring the generic parameters in the custom view is the old behavior, so changing this will break tests for others.

The motivation for doing this was to allow locating and unwrapping a custom view with generic parameters by providing any type as the generic parameter instead of a real one. So essentially, sut.find(ViewWrapper<EmptyView, Never>.self) finds the same view as sut.find(ViewWrapper<CustomView2, Never>.self).

For simplified examples like yours, it may look like a strange oversight but in more practical use cases resolved view type of the generic parameter might be enormously long, and forcing to provide the exact type would make the tests more fragile when view structure changes.

In the case when you have multiple ViewWrappers in the same hierarchy, a more robust approach would be to search for CustomView or CustomView2 and then calling .parent() for shifting the pointer to the correct ViewWrapper, if you need to.

@nalexn nalexn added bug Something isn't working pending release A fixed issue that'll be released in an upcoming update labels Jan 6, 2023
@Tyler-Keith-Thompson
Copy link
Contributor Author

In that case, is there a way to cast a parent into a specific type? Cause I can't call parent.actualView

@nalexn
Copy link
Owner

nalexn commented Jan 6, 2023

Sure! Parent is a type erased view, so you'd need to unwrap it with .view:

.parent().view(ViewWrapper<EmptyView, Never>.self).actualView

The generic parameters can be any.

@nalexn
Copy link
Owner

nalexn commented Jan 14, 2023

Fixed released with v0.9.4

@nalexn nalexn closed this as completed Jan 14, 2023
@nalexn nalexn removed the pending release A fixed issue that'll be released in an upcoming update label Jan 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants