raquo / Airstream

State propagation and event streams with mandatory ownership and no glitches
MIT License
246 stars 28 forks source link

Airstream semantics update – RFC #93

Closed raquo closed 1 year ago

raquo commented 3 years ago

I am currently reworking Airstream behaviour in several big ways. Most of these changes will be released soon as 0.15.0, and will definitely affect your code. Please read this and provide feedback to make sure that the changes account for your use cases. If anything below sounds controversial or is devastating to your use case, please let me know. If anything sounds helpful, please let me know too. Much of this is very hard, so possibly not all of this will happen right away, so I need to know what to prioritize.

No more == checks in signals

This is a solution to #19 – signals won't compare the new value against the previous value before emitting anymore. This can break your code if you rely on signal's events being unique / deduplicated. Solution is to use the new distinct / distinctBy methods to add back uniqueness as needed. Laminar itself does not need you to do it, but you might need this to avoid making redundant Ajax requests or other side effects.

Signals will re-evaluate their current value after being re-started

This is a solution to #43.

Main caveat: only signals have a current value, so only signals that depend on other signals can do this. If your signal is stream.startWith(1), not much you can do about having missed events from stream, since streams don't replay their events for new subscribers. Same for stream.foldLeft(...)

Other caveat: there is no way SIGNAL.foldLeft(...) will fall neatly into either old or new behaviour. I will probably need to make its behaviour configurable.

Observables will stop resetting their internal state when they're stopped

For example, when stream1.combineWith(stream2) is stopped and started again, currently it won't start emitting again until both stream1 and stream2 emit a value, because it "forgets" the previous events and is returned to an empty state similar to how it was originally created. That was the old idea, and I think we're done with it now.

Similarly, when a streamOfStreams.flatten is stopped, it "forgets" which stream it was supposed to mirror, so when it starts up again it does nothing until streamOfStreams emits again.

Things like this really mess with your ability to re-mount Laminar components. The idea with all these changes is that with the new default behaviour you should be able to re-mount a component and it should start working again from where you left off, pretty much, perhaps updating some signals' data if possible (see above).

emitOnce param default will be switched from false to true, or even removed (for streams like EventStream.fromValue). I'm thinking to maybe implement an evalOnStart(:=> myStream) function that returns an observable that re-creates and mirrors myStream every time it's started. That would help with a bunch of different streams like fromValue, periodic, fromFuture, etc.

Anything that has a resetOnStop param will have it removed or default to false (well I guess that's just EventStream.periodic()`

The main edge case here are async streams like ajax / delay / debounce. They will probably get their own treatment, but not sure what it is yet. With async logic it's really hard to come up with something that would work universally well, especially if you consider all the possible timing edge cases.

KeepAlive

One thing that annoys me about ownership lifecycles today it the inability to easily adjust them: #70

For example, say you make an ajax POST request using a stream, and you want to update your app's data store or cookies or something once the request finishes. But what if the user unmounts the component where this logic is defined while the request is still in flight? If the cookie-updating logic was bound inside that component, it will be deactivated once the component is unmounted, and will never happen. Alternatively, if you use unsafeWindowOwner for that logic, it might cause a memory leak if you're not careful to dispose of the subscription, which is not something you should be worrying about with Airstream.

So I'm thinking to provide some kind of keepAlive(maxEvents, maxSeconds) method that would keep the subscription running. I haven't figured out how to actually implement it yet in order for it to be the most useful, whether it should be on the observer side or the observable side, or a property of the Subscription class exposed conveniently via some helper in Laminar.

Have you run into this issue yourself?

Split operator will always return a Signal (not a stream)

You can still call split on streams, but it will return a signal whose initial value will be an empty collection. I went mad getting it to work properly, and now it does, but I don't think I can implement a properly semantic stream for this. It's really complicated. If you need a stream, just call .changes (Laminar doesn't care anymore so not sure why you'd need that). I guess Airstream could have done that for you but semantically the output of split is really a Signal as it relies heavily on its internal state.

Also, split will always provide a Signal as the third param in the render callback.

Observable completion

I don't think this will make it into v0.15.0, but while I have your attention, I will eventually also try to implement observable completion feature, see #23 #33 #1. Basically that means that once it's known that an observable won't ever emit any new values (not even after a restart), it proactively kills all of its subscriptions.

The interactions of observable completion feature with other features are really complicated. What I'd like from you is to tell me whether you want this feature, and if so, why. One argument I heard is that it will allow a concat method on streams – if that's the kind of feature that you want, please let me know how you plan to use it to help me visualize it better.

Dennis4b commented 3 years ago

I have 2 questions/comments regarding the signals (using Airstream in the context of Laminar):

1) A common usecase for me is:

child <-- someBoolean.signal.map{
    case false => emptyNode
    case true => div(.. /* some expensive component */ ..)
}

(perhaps with some laminext sugar or similar, but to show the idea)

Here I really only want to trigger when the signal changes, as it's inefficient to redraw for no reason, and any state in the big component is lost.

I'm not sure under what circumstances the signal would fire twice though? If I do someBoolean.set(true), can that result in multiple true events? If the price of this change is to modify this to someBoolean.signal.distinct.map{ that would be fine.

2) Something that is perhaps not too related to this issue:

I often unintentionally start with something like:

child <-- bigCollection.map(_.size).signal.map{
     case 0 => emptyNode
     case _ => div(... /* some expensive component */ ..)
}

and then realise that this redraws for any change in bigCollection.size.. so I need to use bigCollection.map(_.size > 0).map{ in order to have a signal that only fires when relevant.

I guess this is the correct and expected behaviour though, and something I should just learn to keep in mind.

raquo commented 3 years ago

If I do someBoolean.set(true), can that result in multiple true events?

In the new system if you set true N times in a row, you will get N events. In the old system you would only get one event because the subsequent ones would be auto-deduplicated

And yes, in the new system you'll need to say someBoolean.signal.distinct to avoid redundant renders in case you emit true twice in a row

Regarding scenario number 2 - Yes, that is expected, currently if you want a signal that emits when the map's emptiness changes, you need bigCollectionSignal.map(_.nonEmpty), and in the new system you ALSO need to add .distinct to it to avoid redundant renders when collection size changes

Also just FYI with this kind of logic it sounds like you would benefit from https://github.com/raquo/airstream#splitone in some cases. It's like split but for cases when you don't have a list, only single values. It does == checks internally and will continue doing so, just like split itself.

pbuszka commented 3 years ago

I think the new semantics for signals is a very good choice. It gives back control over == / distinct aspect which would be very useful for performance and/or complexity management. I found myself more then a few times in a limbo where I needed at the same time both a stream to handle the event semantics and signal to handle view aspect for the same data. In simple use cases you can go around this but in more complex scenarios (i.e. state mapping, multiple data sources combine, error handling, chains of async calls) it gets messy very quickly and sooner or later you come to the problem of "converting" streams to singals and vice-versa which is not sound at all.

raquo commented 3 years ago

Yes, same here, I do not appreciate having to shoehorn big states into streams just to avoid the cost of ==. The choice should be based on the concept, not on performance characteristics.

phfroidmont commented 3 years ago

Here are my thoughts:

raquo commented 3 years ago

@phfroidmont Good feedback, thanks!

pbuszka commented 3 years ago

follow-up on rest of the points.

ngbinh commented 3 years ago
raquo commented 3 years ago

@ngbinh That's the plan, currently I have the following methods drafted:

  /** Distinct events (but keep all errors) by == (equals) comparison */
  def distinct: Self[A] = distinctBy(_ == _)

  /** Distinct events (but keep all errors) by reference equality (eq) */
  def distinctByRef(implicit ev: A <:< AnyRef): Self[A] = distinctBy(ev(_) eq ev(_))

  /** Distinct events (but keep all errors) by matching key
    * Note: `key(event)` might be evaluated more than once for each event
    */
  def distinctByKey(key: A => Any): Self[A] = distinctBy(key(_) == key(_))

  /** Distinct events (but keep all errors) using a comparison function
    *
    * @param fn (prev, next) => isSame
    */
  def distinctBy(fn: (A, A) => Boolean): Self[A] = distinctTry {
    case (Success(prev), Success(next)) => fn(prev, next)
    case _ => false
  }

  /** Distinct errors only (but keep all events) using a comparison function
    *
    * @param fn (prevErr, nextErr) => isSame
    */
  def distinctErrors(fn: (Throwable, Throwable) => Boolean): Self[A] = distinctTry {
    case (Failure(prevErr), Failure(nextErr)) => fn(prevErr, nextErr)
    case _ => false
  }

  /** Distinct all values (both events and errors) using a comparison function
    *
    * @param fn (prev, next) => isSame
    */
  def distinctTry(fn: (Try[A], Try[A]) => Boolean): Self[A]
raquo commented 2 years ago

Unfortunately KeepAlive mechanics won't make it into 0.15.0. I've looked into it, and it's very doable, but it's just too much work for this iteration. Current status as follows:

Done:

I'm currently wrapping up https://github.com/raquo/Laminar/issues/95. Really happy with how that feature worked out. Now onto "the second 90%"...

TODO for v0.15.0:

So, the hard parts are done, but quite a lot of work still ahead. All of that will probably take me a few weeks.

fgoepel commented 2 years ago

That sounds like a good change to me. I just got bit by the Signal deduplicating events issue, because I didn't pay sufficient attention to the return type of 'startWith' and its implications.

raquo commented 1 year ago

I've implemented most things here in Airstream 15. Did not have time for observable completion and keepalive yet.