Add UIViewController lifecycle signals

[jdmoreira]

(continued from iZettle/Presentation#24)

Hello friends 👋
I've noticed you were discussing adding signals for the UIViewController lifecycles methods.
Since I already had an implementation on my hard drive, I've decided it was better to just send a pull request.

What

Add signals to UIViewController so we can listen on lifecycle events.

How

Swizzling the lifecycle methods and calling a callbacker that lives inside UIViewController as an associated object.

[jdmoreira]

This particular test is very brittle (but not flaky - it works every time).
The main problem seems to be that this test target lacks an UIApplication. I got the hint about the window.hidden = false from https://stackoverflow.com/a/49872184
It works with a UINavigationController but a modal presentation doesn't work. I'm having some troubles understanding why, I think someone smarter than me should give it a try 😄

[CarlEkman]

I feel like this is a solid approach, and a nice addition to Flow 👍

[jdmoreira]

Something I noticed while working on this PR is that maybe it's easier to move Presentation/Presentation/AssociatedValues.swift into Flow, since associated objects are already being used in a few places here and Presentation could just use it if it's public.

[nataliq]

Hey,

Thanks for the work on this PR.

I would like to step back a little bit and focus on the problem that we are trying to solve.

The real need that we had at iZettle was to be able to respond to view controller lifecycle callbacks in cases where we didn't have a concrete implementation.

The swizzling approach, while interesting, solves for the case where you have a subclass of UIViewController.

The thing is:

• If you have a subclass of UIViewController, you don't need signals and can just respond directly to the events.
• The swizzling approach will mean that you can respond to the event in multiple places. This can lead to poor design decision and harder to debug code.
• In the iZettle use case, we have almost no subclasses of UIViewController at all.

I fully understand that this approach might make it easier for users of Flow who want to integrate it into an existing project with many UIViewController subclasses - but we should really evaluate if that's the use case we want to optimise for, especially when there's a cost to quality and maintainability attached.

Swizzling is a super useful tool when there is no safer option to get the information you need and I like using it too. However, it is dangerous and it's not something we should use only because it's convenient.

I think the beauty of Flow, in comparison to solutions such as Rx, is in its simplicity. We can offer the swizzling solution as a gist to those who want to apply it in their own project but I have significant concerns about integrating it in the framework.

[jdmoreira]

I would say iZettle uses a few UIViewControllers. At least the UIKit subclasses - like UINavigationController, UIPageViewController, etc...

From my limited understanding the frameworks always went to great lengths to avoid UIViewController, UIView and UIControl subclasses. We could have had the helpers for UIStackView in a StackView subclass. Also all the associated objects were created to avoid subclassing, they are a runtime hack as much as swizzling as far as I understand it.

Anyway, I would still understand if the choice is to not merge this PR and I would support that choice.

[mansbernhardt]

@nataliq & @jdmoreira Are you talking past each other?
@nataliq says: "The swizzling approach, while interesting, solves for the case where you have a subclass of UIViewController."

As far as I understand it, swizzling will work directly on instances on UIViewController as well as any subclass of there of. I think that is awesome!

Personally I have not run into any need for these signals. I would like to hear what use-cases you are seeing for these? On the other hand I have run into problem with other UIViewController customization that requires subclasses such as preferredStatusBarStyle.

I also agree with @jdmoreira that the Presentation encourage (or at least does not require) subclassing of view controllers to be able to present them. The reason for this is that it's really hard to build class hierarchies. This PR is a great example of this. If this was implemented in a subclass it would be really hard to access it for user already having a subclass (or using it on any of UIKits own subclasses). Hence it is really important to be able use these directly on UIViewController or any subclass of there of.

[mansbernhardt]

As for moving or exposing "Presentation/Presentation/AssociatedValues.swift" in Flow. I agree that these are really useful and we are re-implementing them in our different FWs as well as in our apps. But a tricky part with FWs, is to decide what goes in and what does not. Are associated values really a part of Flow?

"Flow is a Swift library for working with asynchronous flows and life cycles"

[jdmoreira]

I'm closing this one. Thanks for the feedback!