Adds Passthrough representable

[Tyler-Keith-Thompson]

Linked Issue: Closes #21

Checklist:

• Did you sign the Contributor License Agreement?
• Is the linter reporting 0 errors?
• Did you comply with our styleguide?
• Is there adequate test coverage for your new code?
• Does the CI pipeline pass?
• Did you update the documentation?
• Did you update the sample app?
• Do we need to increment the minor/major version?
• Did you change the public API?
• Have you done everything you can to make sure that errors that can occur are compile-time errors, and if they have to be runtime do you have adequate tests and documentation around those runtime errors? For more details.

---

If Public API Has Changed:

• Did you deprecate (rather than remove) any old methods/variables/etc? Our philosophy for deprecation.
• Have you done the best that you can to make sure that the compiler guides people to changing to the new API? (Example: the renamed attribute)
• If necessary, have you tested the upgrade path for at least N-1 versions? For example, if data persists between v1 and v2 then that upgrade should be tested and as easy as we can make it.

[codecov-commenter]

Codecov Report

Merging #79 (5a1698e) into main (0bea914) will decrease coverage by 0.72%.
The diff coverage is 96.10%.

@@            Coverage Diff             @@
##             main      #79      +/-   ##
==========================================
- Coverage   98.49%   97.76%   -0.73%     
==========================================
  Files          86       89       +3     
  Lines        3844     5095    +1251     
==========================================
+ Hits         3786     4981    +1195     
- Misses         58      114      +56     

Impacted Files	Coverage Δ
.../Extensions/DeprecatedWorkflowUIKitAdditions.swift	0.00% <0.00%> (ø)
...IKitTests/Storyboard/StoryboardLoadableTests.swift	87.50% <42.85%> (-5.49%)	⬇️
...ources/SwiftCurrent_UIKit/StoryboardLoadable.swift	85.71% <50.00%> (-5.96%)	⬇️
...rrent/Protocols/PassthroughFlowRepresentable.swift	80.00% <80.00%> (ø)
ExampleApps/SwiftUIExample/Views/MFAView.swift	90.00% <100.00%> (-0.63%)	⬇️
...iftCurrent_UIKitTests/GenericConstraintTests.swift	100.00% <100.00%> (ø)
...mple/SwiftCurrent_UIKitTests/ModalStyleTests.swift	99.04% <100.00%> (ø)
...Current_UIKitTests/UIKitConsumerAbandonTests.swift	99.13% <100.00%> (ø)
...tCurrent_UIKitTests/UIKitConsumerLaunchTests.swift	98.73% <100.00%> (ø)
...ent_UIKitTests/UIKitConsumerPersistenceTests.swift	99.39% <100.00%> (ø)
... and 12 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d92f59...5a1698e. Read the comment docs.

[Tyler-Keith-Thompson]

Alternative Approaches Considered:

• There was an approach where a new type could be declared called Passthrough. This idea was discarded for 2 important reasons, the first is that it'd only work if both Workflowinput == Passthrough and WorkflowOutput == Passthrough, but there was no reasonable way to enforce that always happened. The second is that the generic constraints we have in place are already sort of hard to wrap your head around, they would've positively exploded with a new type like that.
• There was the possibility of making an input and output of Never mean Passthrough but this was rejected because Never has a specific connotation right now (if my input type is Never it means I'll happily drop your arguments on the floor). This is purposeful because it may be a legitimate usecase in a Workflow. So abusing Never to somehow work with this felt wrong.
• There was the possibility of looking at both the Input and Output of a FlowRepresentable and if they were both AnyWorkflow.PassedArgs considering that to mean Passthrough but that was discarded because it felt like the behavior of automatically passing data through that FlowRepresentable was a bit magic.

Design Choices:

• A new protocol was created named PassthroughFlowRepresentable, while it is a lengthy name consumers of the library would be very clear in their intent and would be aware of what that does (mostly, remove boilerplate)
• Very special and deliberate attention was payed to the current tests and current generic constraints. They were not fully fleshed out, so a big part of this PR was adding tests around how things should've behaved already. To that end GenericConstraintTests have been added to SwiftCurrent, SwiftCurrent_UIKit, and SwiftCurrent_SwiftUI
• Because thenProceed(with:) reads so much better than thenPresent and because thenPresent means a whole different series of generic constraints it was decided to deprecate (WARNING, not error) those methods and use the rename attribute to let Xcode tell consumers how to update
• There's a commit in here called "Xcode driven development" where after that deprecation was added I just clicked "fix" on Xcode and it correctly applied all changes, which was beautiful
• For SwiftUI ONLY* Because our SwiftUI implementation differentiates the first item in the workflow from the rest there was a deliberate choice made not to support arguments in a WorkflowLauncher and the very next thenProceed(with:) having an input type of Never. If users really want the behavior of launching a workflow with arguments and then IMMEDIATELY dropping them on the floor they can make their first FlowRepresentable take a WorkflowInput of AnyWorkflow.PassedArgs and simply not pass them forward. However it seemed there was no legitimate reason to allow this, it feels very much like an error case.
• For SUBSEQUENT calls to thenProceed(with:) if one item passes forward data and the next has a WorkflowInput of Never the old behavior is honored and that data is dropped on the floor. There may be several legitimate reasons for doing this, there may be skips that happen, there may be a reason to transform that data. It makes a lot more sense once you've gotten past the first item in a Workflow
• Generic constraints have been pushed probably to their limit with UIKit. It seems more clear than ever that we should try an approach where we use a modifier-like syntax for launchStyle and persistence. However, that felt WAY outside of the scope of this PR. Once that decision is made you should be able to deprecate some of the methods on Workflow now and rename them much like thenPresent was deprecated as part of this PR

[Tyler-Keith-Thompson]

NOTE:

• Look at how cool MFAView is now, WAY less boilerplate, same experience
• The public API was changed it was NOT broken. A warning deprecation was added instead. Wanted to make sure people didn't think we had to rev the major version for this, it's just a minor version rev.
• Code coverage was slightly reduced because some code was deleted, or deprecated. However coverage is great on all the new code
• There's a valid argument to write tests specifically for the deprecated methods to make sure they continue to work, however you'll have to deal with warnings in the tests. For now they are uncovered.

[Tyler-Keith-Thompson]

Rules for Generic Constraints

• If a WorkflowInput is Never it means either no arguments or some arguments can be passed to the FlowRepresentable, but any arguments passed will be ignored and will not be passed to the next item.
• if a WorkflowInput is AnyWorkflow.PassedArgs it means either no arguments or some arguments can be passed to the FlowRepresentable, and it'll handle those arguments somehow, either by passing them on, transforming them, or dropping them
• If a WorkflowInput is a concrete type (like say, a String) it means a FlowRepresentable will only take that type, if something that isn't a String is passed to it, or if nothing is passed to it, there'll be a runtime error.
  if aFlowRepresentableis aPassthroughFlowRepresentableit means it will not deliberately handle arguments, it'sWorkflowInputandWorkflowOutputare *both* considered to be anAnyWorkflow.PassedArgsbut the library will handle passing its arguments forward. ThesePassthroughFlowRepresentables simply call proceedInWorkflow()` and do not specify arguments.

In UIKit

Valid Workflows FORMAT: {LAUNCH_ARGS} >> INPUT>Output ~> Next Item

• {String}>> String>Never ~> AnyWorkflow.PassedArgs>AnyWorkflow.PassedArgs ~> Never>Never
• AnyWorkflow.PassedArgs ~> String>Never (NOTE: Fails if argument passed was not a string, or was .none)
• {AnyWorkflow.PassedArgs}>> String>String ~> String>Int ~> Int>Never
• {AnyWorkflow.PassedArgs}>> String>String ~> PASSTHROUGH ~> String>Int

Compile time works, but fails at runtime:

• {Int}>> String>String ~> String>Int ~> Int>Never
• {Int}>> Int>String(SKIPPED) ~> String>Int ~> Int>Never
• {Never}>> String>String ~> String>Int ~> Int>Never

In SwiftUI

Valid Workflows FORMAT: {LAUNCH_ARGS} >> INPUT>Output ~> Next Item

• {String}>> String>Never ~> AnyWorkflow.PassedArgs>AnyWorkflow.PassedArgs ~> Never>Never
• AnyWorkflow.PassedArgs ~> String>Never (NOTE: Fails if argument passed was not a string, or was .none)
• {AnyWorkflow.PassedArgs}>> String>String ~> String>Int ~> Int>Never
• {AnyWorkflow.PassedArgs}>> String>String ~> PASSTHROUGH ~> String>Int
• {Never}>> Never>String ~> Never>Never

Compile time works, but fails at runtime:

• {Int}>> Int>String(SKIPPED) ~> String>Int ~> Int>Never

Invalid Workflows that could've worked in UIKit

• {Never}>> String>String ~> String>Int ~> Int>Never

[Richard-Gist]

Looks good so far, but I want to play with it a bit before I sign off.

[Richard-Gist]

The failed test after the merge was due to an exceeded timeout in SwiftCurrent_SwiftUITests.PersistenceTests testRemovedAfterProceeding_OnAllItemsInAWorkflow which exceeded the timeout of 0.5 seconds for the Inspection at line 189 The total test time for the failed test was 4.886 seconds. The logs are not showing me anything more of interest, other than all tests ran slow (but not that slow). Re-running the tests to pass as I think this was a system hiccup.

[Tyler-Keith-Thompson]

Missing: PassthroughFlowRepresentable does not yet play nice with StoryboardLoadable

[Richard-Gist]

Having PassthroughFlowRepresentable work with StoryboardLoadable is going to be a requirement for me. Without it, I can't use it in our example app to showcase a simple way to leverage it.

[Richard-Gist]

So I'm trying to test it with our example app:

class TermsOfServiceViewController: UIViewController, PassthroughFlowRepresentable, StoryboardLoadable {
    weak var _workflowPointer: AnyFlowRepresentable?

    @IBAction private func accept(_ sender: Any) {
        proceedInWorkflow()
    }
}

And when I add it to the SetupViewController:

launchInto(
    Workflow(LocationsViewController.self)
        .thenProceed(with: PickupOrDeliveryViewController.self)
        .thenProceed(with: TermsOfServiceViewController.self) // <----- just added it in the middle somewhere
        .thenProceed(with: MenuSelectionViewController.self, flowPersistence: .persistWhenSkipped)
        .thenProceed(with: FoodSelectionViewController.self)
        .thenProceed(with: ReviewOrderViewController.self),
    args: locations,
    withLaunchStyle: .navigationStack)

I get a fatalError when it is time to show the screen:
SwiftCurrent_UIKit/StoryboardLoadable.swift:78: Fatal error: The StoryboardLoadable protocol provided a default implementation if this initializer so that consumers didn't have to worry about it in their UIViewController. If you encounter this error and need this initializer, simply add it to TermsOfServiceViewController

You may find it helpful to go ahead and add a ToS screen to the example app to test out consuming it in a Storyboards example. It would also give your feature a cool shoutout on why it's awesome.

[Richard-Gist]

Found a typo. If this was a copy paste thing, it's ok to just update this code, but updating where you copied it from would be appreciated.

[Richard-Gist]

I'm still having trouble using this in the UIKitExample app. Using the code I posted above, here's the results:

Simulator.Screen.Recording.-.iPod.touch.7th.generation.-.2021-07-26.at.13.06.53.mp4

[Tyler-Keith-Thompson]

Woah...there was a deep underlying issue with SwiftCurrent altogether here. Short version? When the input type was AnyWorkflow.PassedArgs it was not using the factory method. Meaning all the StoryboardLoadable protocol things simply don't work with AnyWorkflow.PassedArgs, you can discover this yourself if you simply don't use PassthroughFlowRepresentable.

I can try to fix that as part of this issue, but it feels like this'll get stuck on behalf of a bug that has nothing to do with this new feature for a while.

[Tyler-Keith-Thompson]

Code that is the problem, in AnyFlowRepresentable.swift

/**
     Creates an erased `FlowRepresentable` by using its initializer
     - Parameter type: The `FlowRepresentable` type to create an instance of
     - Parameter args: The `AnyWorkflow.PassedArgs` to create the instance with. This ends up being cast into the `FlowRepresentable.WorkflowInput`.
     */
    public init<FR: FlowRepresentable>(_ type: FR.Type, args: AnyWorkflow.PassedArgs) {
        argsHolder = args
        switch args {
            case _ where FR.WorkflowInput.self == Never.self:
                var instance = FR._factory(FR.self)
                _storage = AnyFlowRepresentableStorage(&instance)
            case _ where FR.WorkflowInput.self == AnyWorkflow.PassedArgs.self:
                // swiftlint:disable:next force_cast
                var instance = FR(with: args as! FR.WorkflowInput) // <-- THIS IS WRONG
                _storage = AnyFlowRepresentableStorage(&instance)
            case .args(let extracted):
                guard let cast = extracted as? FR.WorkflowInput else { fatalError("TYPE MISMATCH: \(String(describing: args)) is not type: \(FR.WorkflowInput.self)") }
                var instance = FR._factory(FR.self, with: cast)
                _storage = AnyFlowRepresentableStorage(&instance)
            default: fatalError("No arguments were passed to representable: \(FR.self), but it expected: \(FR.WorkflowInput.self)")
        }
        _storage._workflowPointer = self
        isPassthrough = FR.self is _PassthroughIdentifiable.Type
    }

[Richard-Gist]

Wow that's not great. Thank you for tracking that down. I've already created an Issue to fix that particular bug from your comment.

[Richard-Gist]

Having PassthroughFlowRepresentable work with StoryboardLoadable is going to be a requirement for me. Without it, I can't use it in our example app to showcase a simple way to leverage it.

Since you tracked this down to a pre-existing bug that is now tracked with issue #80 , I'm good with what I've reviewed.

[Richard-Gist]

@wiemerm When you're good with this could you ramp the minor version? This enhances the API so we should do a minor bump, but it's not a breaking change so no need for a major bump.

[Richard-Gist]

Thank you @wiemerm