Add Optional Kleisli composition function

[inamiy]

Hi Pointfree team 👋

I came across with the composition usage of:

// 1. /RootAction.screen1: RootAction -> Screen1Action?
// 2. someTranslatorFunc: Screen1Action -> SomeOtherScreenAction?
reducer.pullback(action: /RootAction.screen1 .. someTranslatorFunc)

where someTranslatorFunc is an action-translation between 2 components which is NOT derived from enum case.

In this scenario, an ordinary Optional Kleisli composition becomes necessary (than the overload in Line 211), so I think it's worth adding it in this library.

What do you think?

[stephencelis]

Hey @inamiy! We recently wanted to reach for this exact functionality and were a little surprised it didn't exist, given the number of operator overloads defined. That said, we're a little worried about the operator space in the library as is, and the @_disfavoredOverloads it introduces. We have even considered removing some/all of the composition operators and recommend calling the .append method instead. Because of this, we're not sure it's a good idea to add even more surface area here, as useful as it can be.

[inamiy]

Hi @stephencelis ! Yes, I also agree that overloading another func .. will become more trickier.

IMO, the existing overload in L211:

https://github.com/pointfreeco/swift-case-paths/blob/2a12a39dc20334bbf75ef39142c15a40f521b703/Sources/CasePaths/Operators.swift#L211-L216

is also another form of Optional Kleisli composition that is actually more complicated than this PR's impl, which can be re-written as:

return lhs .. (/rhs).extract(from:)  // This `..` is overload in this PR

So I think, in essence, we want to first have this PR than L211 (since it's simpler), and may want to reconsider too many overload issue for both this PR and L211.

BTW I don't particularly have a new idea of this Kleisli composition method name at the moment (other than fish >=>), and since It will be a breaking change, it's no problem for me to suspend this PR until new method name is ready and major version is bumped at some point.

[inamiy]

Or maybe we can say that this PR's impl is more general that should actually sit in Prelude-level than in CasePath. Then it will make sense that L211 uses CasePath feature that needs to sit in this code, as is.

From user's point of view, however, introducing Prelude will often become too large, so I still feel more preferred to have this PR's impl in CasePath which will be nicer to also handle any extract-like methods and their composition.

Quite annoying judge... But what do you think?

[maximkrouk]

Afaiu it's similar to something I use, so maybe I can make another PR too. As you mentioned some people might not like using Prelude as a dependency (I like Prelude btw 🙂). So static factory method might be useful too + operator for those who okay with operators 🌚

extension CasePath {
  public static func concatenate<T0>(
    _ cp0: CasePath<Root, T0>,
    _ cp1: CasePath<T0, Value>
  ) -> CasePath {
    return cp0.appending(path: cp1)
  }
  // more overloads for more arguments
}

// Usage in TCA
pullback(
  state: \ParentState.child.derived
  action: .concatenate(
    /ParentAction.child,
    /ChildAction.derived
  )
)

// Or using global function (Overture-like style)
pullback(
  state: \ParentState.child.derived
  action: compose(
    /ParentAction.child,
    /ChildAction.derived
  )
)

The other way is to add CasePaths dependency to Prelude and Overture to keep this functional composition in these packages, maybe I like this approach even more.

[stephencelis]

So I think, in essence, we want to first have this PR than L211 (since it's simpler), and may want to reconsider too many overload issue for both this PR and L211.

I think we totally agree, we're just erring on the side of eliminating the existing surface area rather than introducing new/alternative surface area. Let us give it more thought, though! We'll discuss soon after the end-of-year holidays.

[stephencelis]

@inamiy Thanks again for bringing this up. We kept it open as a reminder of an ergonomic hole in the library, but we finally do have a vision to address the problem in a more natural way (you can check the case-key-paths branch for a preview). Going to close this as a result, and we will hopefully have a new release that addresses the problem in the coming weeks!