fixing flatMap?

[yurique]

• some dependency bumps

[raquo]

@yurique Thanks, this looks good!

I moved the new flatMap into Observable as it doesn't need the implicit class anymore. I also changed its use of Observable to Self to make the API a bit more specific.

I looked at your #21 as well but I'm not convinced that Redefining FlattenStrategy to be FlatMapStrategy is an improvement. It seems that implementing flatten should be more straightforward in general, and the way it's defined now we save one allocation of Observable compared to #21 when doing .flatten (.map(identity)). Neither of these is a big deal, but I just don't see any advantage, please let me know if I'm missing something.

Also remember that the first type param of FlattenStrategy doesn't have to be Observable, it just so happens that only FlattenStrategy[Observable, ?, ?] are currently implemented. I will eventually implement more specialized strategies such as for Signal[Signal[_]], so the flatten / flatMap methods need to work with those possibilities.

I plan to merge this PR later this week, hopefully together with MemoizedCollection for Laminar children handling. Thanks again!

[yurique]

Cool! I'm looking forward to see it released :)

As for the other PR - the bottom line for me is this: as long as it is an internal implementation detail, it doesn't really matter.

My motivation was to simplify the FlattenStrategy:
from very complicated type parameters

 trait FlattenStrategy[-Outer[+_] <: Observable[_], -Inner[_], Output[+_] <: Observable[_]] { 

to quite simple:

 trait FlattenStrategy[Outer[_], Inner[_], Output[_]] { 

In the current state it has to be complicated as we declare a "type class" for a 2-level deep type, like EventStream[Future[A]], the #21 approach is to declare it for a couple of "shallow" types, like EventStream[A] and Future[B].

Another part of my thinking was that this is the approach (implementing flatMap and deriving flatten from it) taken by big FP libs, like cats:

trait FlatMap[F[_]] extends Apply[F] {
  def flatMap[A, B](fa: F[A])(f: A => F[B]): F[B]
  def flatten[A](ffa: F[F[A]]): F[A] =
    flatMap(ffa)(fa => fa)

(though it's a little bit different here, as the "inner" has to be the same as "outer")

Regarding an additional allocation - in my experience flatMap is used much more often than flatten, though I'm not sure what are the performance trade-offs between the current implementation and #21 .

As for the first param of FlattenStrategy - wouldn't it be possible to make a

  object SomeSignalStrategy extends FlattenStrategy[Signal, Signal, Signal]

?

But anyways, this is not critical for me :) (maybe there's someone out there who knows better?)

[raquo]

As for the first param of FlattenStrategy - wouldn't it be possible to make a
object SomeSignalStrategy extends FlattenStrategy[Signal, Signal, Signal]

It would be possible to instantiate such a strategy, but the flatMap method in #21 will not accept this strategy because it needs a FlattenStrategy[Observable, Inner, Output] which is not the same type.

[yurique]

I've been thinking about this a bit.

So, as for flatMap requiring FlattenStrategy[Observable, Inner, Output] - in that case it should be an extension method in the implicit class, I guess (as it's usually done when there are type classes involved).

On the other hand: should it actually be a flatMap? It makes sense for stream.flatMap(x => something: Stream[B]) and signal.flatMap(x => something: Signal[B]), but for mixing containers (like stream.flatMap(x => something: Signal[B]) or stream.flatMap(x => something: Future[B])) - maybe we should just have other functions (like switchFuture or something like that?)

[raquo]

I agree, it doesn't make much sense for flatMap output to be a Future because flatMap should not tacitly introduce subscriptions – that kind of thing should be managed more explicitly.

However, in addition to your examples it does make sense to go from Signal[EventStream[A]] to EventStream[B] – we already have SwitchStreamStrategy for that.

So the Outer and Output type params of FlattenStrategy don't have to be the same type, they just have to both be some kind of Observable. Which is exactly what the current FlattenStrategy API requires.

Note – I can't imagine how one would want to flatMap an EventStream into a Signal, but I'm not inclined to disallow that. Technically there's nothing wrong with that, your FlattenStrategy just needs to know what initial value to use for the output Signal.