ekazaev / route-composer

Protocol oriented, Cocoa UI abstractions based library that helps to handle view controllers composition, navigation and deep linking tasks in the iOS application. Can be used as the universal replacement for the Coordinator pattern.
MIT License
896 stars 63 forks source link

Finding an improved solution for conditional transition type selection of UIViewController in Swift #104

Closed IgorRatynski closed 10 months ago

IgorRatynski commented 10 months ago

I'm trying to implement a transition type depending on the presence of a specific UIViewController with a certain context.

// If I try to return `ContainerAction` (not any), I get an error
// Error: Use of protocol 'ContainerAction' as a type must be written 'any ContainerAction'
func navigationAction() -> any ContainerAction {
    // If the view controller hierarchy already contains an instance of `ClientManagerChatViewController`, 
    // but with a different context
    if let _ = ClassFinder<ClientManagerChatViewController, Context>().getViewController(with: context) {
        // If the view controller hierarchy already contains an instance of `ClientManagerChatViewController` 
        // with the same context
        if let _ = ClassWithContextFinder<ClientManagerChatViewController, Context>().getViewController(with: context) {
            // Do nothing, just return the current UIViewController
            return NavigationController.push()
        } else {
            // Replace the last navigation action with a new one, as the context is different
            return NavigationController.pushReplacingLast()
        }
    } else {
        // If the `ClientManagerChatViewController` isn't present in the hierarchy, perform the push navigation action
        return NavigationController.push()
    }
}

let step = StepAssembly(
    finder: ClassWithContextFinder<ClientManagerChatViewController, Context>(),
    factory: Factory(dependencies: deps)
)
.adding(LoginInterceptor<Context>())
// However, when attempting to use `any ContainerAction` I get an error:
// Error: Type 'any ContainerAction' cannot conform to 'ContainerAction'
.using(navigationAction()) // needs: NavigationController.push() || NavigationController.pushReplacingLast()
.from(GeneralStep.custom(using: ClassFinder<NavigationController, Context>()))
.assemble()

NavigationController it's child of UINavigationController As a result, I'm returning the entire Destination construction through an if-else statement instead of just selecting the type of navigation action. Could you possibly help me find a better way to solve this?

ekazaev commented 10 months ago

@IgorRatynski Hi

The problem is in swift types limitation here. You cant just use any ContainerAction there. Any implies completely any container actions while the assembly relies on its type to understand what type of the ContainerViewControllers are allowed. There are examples in the Example app how to write that. The easiest way if you dont know is to do that if on the top level and return 2 differnt chain the way you are already doing it. Have a look at the SwitchAssembly.

I am typing from my phone. When Ill have the access to my laptop ill try to provide you better answer.

IgorRatynski commented 10 months ago

I tried it, but it didn't work. I must have done something wrong. Okay, thanks for the quick reply!

ekazaev commented 10 months ago

@IgorRatynski Technically there is nothing wrong in returning 2 separate chains. The route composer is not rewritten to support new swift type erasure features yet. I tried it when they released that feature bot started to hit Segmentation fault at some point due to compiler issues. Probably in the future it can be possible. But Ill have a look if it can be simplified.

ekazaev commented 10 months ago

@IgorRatynski Thinking about it SwitchAssembly wont help you. If you want to simplify that you'll need to write a custom container action that would do either of actions depending on the result of the view controller presence. It will be even safer as your ClassWithContextFinder<ClientManagerChatViewController, Context> finder searches everywhere and not necessary at the UINavigationController that it going to push to. You still can use ClassWithContextFinder inside, just use the navigationController action receives as its starting point with option containing

IgorRatynski commented 10 months ago

@IgorRatynski Technically there is nothing wrong in returning 2 separate chains. The route composer is not rewritten to support new swift type erasure features yet. I tried it when they released that feature bot started to hit Segmentation error at some point due to compiler issues. Probably in the future it can be possible. But Ill have a look if it can be simplified.

@ekazaev As far as I'm concerned, it's not a good approach to code when I'm rewriting about 15 lines because of one line, if I can avoid doing that. If not, then yeah, nothing wrong with that : )

ekazaev commented 10 months ago

@IgorRatynski Something like:

public struct PushReplacingLastIfNeededAction<ViewController: UINavigationController>: ContainerAction {

        private let comparisonBlock: (UIViewController, UIViewController) -> Bool

        public init(comparing comparisonBlock: @escaping (UIViewController, UIViewController) -> Bool) {
            self.comparisonBlock = comparisonBlock
        }

        public func perform(embedding viewController: UIViewController,
                            in childViewControllers: inout [UIViewController]) {
            if let lastViewController = childViewControllers.last,
               comparisonBlock(lastViewController, viewController) {
                childViewControllers.removeLast()
            }
            childViewControllers.append(viewController)
        }

        public func perform(with viewController: UIViewController,
                            on navigationController: ViewController,
                            animated: Bool,
                            completion: @escaping (_: RoutingResult) -> Void) {
            var viewControllers = navigationController.viewControllers
            perform(embedding: viewController, in: &viewControllers)
            navigationController.setViewControllers(viewControllers, animated: animated)
            if let transitionCoordinator = navigationController.transitionCoordinator, animated {
                transitionCoordinator.animate(alongsideTransition: nil) { _ in
                    completion(.success)
                }
            } else {
                completion(.success)
            }
        }

    }

//...
        .using(PushReplacingLastIfNeededAction<UINavigationController>(comparing: { lastViewController, newViewController in
            return /* Cast view controllers, compare and return true if last one needs to go */
        }))
//...
ekazaev commented 10 months ago

As far as I'm concerned, it's not a good approach to code when I'm rewriting about 15 lines because of one line, if I can avoid doing that. If not, then yeah, nothing wrong with that : )

@IgorRatynski I dont want to fall into philosophy but as far as I remember R.Martin says something like: If the library in the project gives you too much or too little functionality, or conflicts with the expected functionality it consequently makes code dirtier. You can avoid this by simply applying patterns like Decorator, Adapter, Facade or others above this library/libraries.

In your case StepAssembly sits in the function like func goToChat(with context: WhateverContext) so the Facade pattern is already used. So if nobody digs there - it is fine :). But the solution with the custom action is better for sure.

Best of luck.

IgorRatynski commented 10 months ago

Thank you very much, @ekazaev, for your help with the problem and the library. Great job!

ekazaev commented 10 months ago

My pleasure. Best of luck.