Inspector emissary for view modifier

[gili-labs]

@nalexn I have created this PR with the changes I made to InspectionEmissary to support view modifiers. It works great. However, the testViewModifierOnFunction test fails. I left it this way on purpose in order that you can look at the difference between this and the InspectionEmissaryForViewModifier. I do not see a difference. I dumped the body of the modifier in the debugger, and they seem identical and cannot ascertain why the ViewInspector would treat them differently.

[gili-labs]

Note, I have tried searching for a second text string in the context of the async calls and have found none. As I said, I'm trying ascertain the difference between the on function and the use of an InspectionForViewModifier object.

[nalexn]

At the first glance, I cannot grasp why this happens - really bizarre. I'm about to release a new version - wanted to include this PR as well, but it seems like it deserves further investigation and possibly a revisited approach - I'd ideally come to the { self.inspection.visit(self, $0) } syntax for ViewModifiers, which is more concise and non-recursive. This may as well fix this issue.

[nalexn]

Reference to the related discussion: #108

[gili-labs]

I have a thought and want to run it by you. The on method and inspection methods for view modifiers both expect the view modifier's body by calling self.body(content: content). However, I'm thinking this returns a new view created by body(content:), rather than the view originally created when the modified view called modifier(_:). Hence, I'm thinking you're right, we need to figure out how to extract the body from an instance of view modifier. I think this might present a problem as well, because when we create an instance of a view modifier and pass it to modifier(_:), it doesn't create a view, rather SwiftUI at some point calls the instance's body(content:) method and encapsulates it in a ModifiedContent view.

[gili-labs]

@nalexn: I think I solved the problem with dynamic inspection of view extensions. I having a working on method, but I to refactor the inspection method. Stay tuned.

[gili-labs]

@nalexn: Please review the changes I made to this PR. These changes fix the on function and the inspector emissary supporting view modifiers.

[gili-labs]

@nalexn: Don't merge this yet. I'm having issues defining inspection emissaries in a way that doesn't require the runtime to import ViewInspector.

[gili-labs]

The last commit resolves the problem I was experiencing. However, I had to eliminate InspectionEmissaryBase, which results in some code duplication. I was trying to avoid this duplication by introducing a base protocol, but this introduced issues that I didn't understand. I'm going to revisit this once there's a version of Swift that unlocks existentials.

[nalexn]

Hey - reviewed the code - looks good and works great! The only concern is the requirement to refactor the ViewModifier to return a custom view.

I thought a bit how can we eliminate the recursive body(content) call, or at least mitigate unwanted side effects. In our discussion, I referred to a content being a stub, which remains a stub in runtime, so an idea came to my mind - what if we don't need to rely on SwiftUI to provide the content parameter and pass in ours? In fact, I already do this for a simple unwrap of ViewModifiers.

So if we add this extension:

extension ViewModifier {
    func body() -> Any {
        return body(content: _ViewModifier_Content<Self>())
    }
}

We'll be able to construct the Body at any moment we want, including when the ViewModifier is hosted on screen and ready for async inspection.

That is, if we previously had

struct InspectableTestModifier: ViewModifier, Inspectable {
    
    var didAppear: ((Self.Body) -> Void)?
    
    func body(content: Self.Content) -> some View {
        HStack {
            EmptyView()
            content
                .padding(.top, 15)
        }
        .onAppear { self.didAppear?(self.body(content: content)) }
    }
}

We can do instead

struct InspectableTestModifier: ViewModifier, Inspectable {
    
    var didAppear: ((Self) -> Void)?
    
    func body(content: Self.Content) -> some View {
        HStack {
            EmptyView()
            content
                .padding(.top, 15)
        }
        .onAppear { self.didAppear?(self) }
    }
}

just like for an arbitrary custom view, but for the on function take an additional step and unwrap the content with body() call.

I'd want to experiment more with this approach, as it promises a unified approach for Views and ViewModifiers without requiring a refactoring of the ViewModifier's body.

[gili-labs]

Looks like a good idea.

[nalexn]

Have a look at this slight refactoring. The simple tests pass, but I want to add your tests with more sophisticated ViewModifier's structure to assure there are no issues like you've encountered

[gili-labs]

Wow, you made it work! I went down this path at one point, but couldn't quite make it do the right thing. By doing this, does one still have to refactor the view out of the body of the view modifier? And you mentioned something above about requiring an extension to ViewModifier. Where do you define that?

[nalexn]

By doing this, does one still have to refactor the view out of the body of the view modifier?

Nah, I really want to avoid it, and so far it works without it. I'm now refactoring the InspectionEmissary to remove the V: View & Inspectable requirement and make it work for both View and ViewModifier - this coming along nicely so far and looks promising. I have only 1 hour left today, will try to complete it early this week

And you mentioned something above about requiring an extension to ViewModifier. Where do you define that?

Ended up simply calling body(content: _ViewModifier_Content<Self>()) once again in the code. Might refactor this in an extension down the road

[nalexn]

Hey @gili-labs , I think I got to a workable solution in this branch.

[nalexn]

@gili-labs I tested it with your examples and it works as expected, I'll likely be preparing a release soon with this code. I think I should close this PR without merging, but I'll give you a shout-out in the release notes as your work played an essential role in solving this challenging task!

[gili-labs]

This sounds great! Are you planning on releasing today?

[nalexn]

I want to review and merge another PR, plus check if I can put in some more "low hanging fruits". Can easily defer it for a couple of days.

[nalexn]

This is resolved now in v0.8.0