Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Property current value handling in case of re-activation #770

Open
raimohanska opened this issue Aug 21, 2020 · 3 comments
Open

Property current value handling in case of re-activation #770

raimohanska opened this issue Aug 21, 2020 · 3 comments

Comments

@raimohanska
Copy link
Contributor

raimohanska commented Aug 21, 2020

Current behaviour or Properties when created with stream.toProperty() is that is a property is deactivated (all subscribers removed), it will maintain its current value while not connected to the underlying stream. This means that upon re-activation it will emit the possibly stale current value to it's new subscribers.

Here's a running example: https://codesandbox.io/s/property-with-eventstream-4h4sb?file=/src/App.tsx

This behavior is ok for some cases, but there are cases where you may prefer something else.

In some cases the current value could be obtainable with a single synchronous call. Use window scroll position for example. How to do that in Bacon.js? Well you can but it's not obvious. Here's how: https://codesandbox.io/s/topropertywithinitvaluefn-l0n8z?file=/src/App.tsx

Yet sometimes there's no feasible way to fetch a new value synchronously and you might prefer just not emitting a stale value, but instead emitting nothing until a fresh value can be provided. Also this is a simple solution in Bacon.js, which is far from obvious if you're unfamiliar with the library internals. Like this: https://codesandbox.io/s/topropertydropstalevalues-504gv

I'm considering whether these alternatives make sense to you as well and how should the API be shaped? This might be something for the 4.0 release.

In short, here are the implementations:

function toPropertyWithInitValueFn<A>(stream: B.EventStream<A>, initValueFn: () => A): B.Property<A> {
    return new B.Property(
        new B.Desc("Bacon", "toPropertyWithInitValueFn", []),
        sink => {
            sink(new B.Initial(initValueFn()));
            return stream.subscribeInternal(event => sink(event));
        }
    );
}

function toPropertyDropStaleValues<A>(stream: B.EventStream<A>): B.Property<A> {
    return new B.Property<A>(
        new B.Desc("Bacon", "toPropertyDropStaleValues", []),
        sink => stream.subscribeInternal(sink)
    );
}
This was referenced Aug 21, 2020
@steve-taylor
Copy link

toProperty<A>(initValueFn?: () => A): Property<A> should replace toProperty<A>(initialValue?: A): Property<A> on EventStream in Bacon 4, in addition to Property dropping stale state on deactivation. Those two changes go together.

I’m not sure about Bacon internals, but my guess is that a Property stores the current value somewhere and, with this change, it would additionally have to store the optional initValueFn. The current value would be unconditionally dropped on deactivation.

@raimohanska
Copy link
Contributor Author

To me it seems that there are genuine cases for all three alternatives

A) retain "stale" value / current behavior: the previous seen value may be the best initial value, to be updated when new data can be fetched. E.g. stock ticker with a timestamp, chat conversation history, web shop product catalog.
B) drop "stale" value and show nothing until a fresh value is fetched: in some cases you want only show confirmed up-to-date information. Cannot come up with a use case right now.
C) replace "stale" value with value from function: if it's possible, you should do it. Works for cases where the value is actually available all the time, like scroll pos. Not for cases where the value can only be accumulated from events and/or needs an async call to get.

Then there are also the Properties that are not just stream.toProperty() but built on an arbitrary mix of inputs. I'm actually unsure which property methods will cause state values to be retained even if their sources did drop the stale value. But I'd expect the resultant property to drop stale values if any of its sources so does.

Also I'd hope to have an explicit dropStaleValues or similar method to convert a Property to such form. And that would indeed prove hard if the underlying sources do this value retainment. So, we are indeed talking about major changes to how the library works and a lot of added test cases to ensure that all derived properties share the new behavior.

@semmel
Copy link
Member

semmel commented Aug 22, 2020

I hope that I understand the proposal and can express my observations adequately, otherwise please correct or ask me to clarify.

Considering toPropertyWithInitValueFn

Scenario: We have a getCurrentValue function which obtains the current value with a single synchronous call.
Behaviour: If the number of subscribers becomes a positive number, this function is invoked to yield the first value while in parallel the underlying EventStream and all dependencies are reactivated.
Effect:

  • The first subscriber gets reported an initial value which is determined exactly at the time he subscribes, followed by values obtained by the underlying EventStream.
    subscribing again 1 ← that's expected because Bacon.later(1000,…
  • Subsequent subscribers (while active) don't invoke getCurrentFn, they just get the "current accumulated" property value as initial value.
    value for the transient subscriber 2 ← that's unexpected because Bacon.later(3000,…

Although toPropertyWithInitValueFn somehow repairs resubscribing to a property where the subscriber count has dropped to zero, I still find the sequence of reported values not plausible.

It establishes somehow a method of obtaining values in parallel and concurrently to the underlying stream. I cannot see what implications this could have (Double initial values, expensive double calls of getCurrentFn, …?)

Calling getCurrentFn on every new subscriber, or make it work for Observables in general (e.g. refreshWith like startWith) might be worth thinking of. 🤔

Considering toPropertyDropStaleValues

Scenario: Values get pushed into our property from elsewhere (e.g. WebSocket chat contributions or stock ticker events).
Behaviour: If the number of subscribers becomes a positive number, not the last value from before the unsubscription is reported as initial value, but the delivery of values is delayed until a "fresh" value comes in. This is like killing the Property character and exposing just an EventStream. If the number of subscribers is above zero, it behaves like a good old Property.
Effect:

  • The first subscriber gets the same values reported as if subscribing to a plain EventStream or like property.skip(1).
    subscribed again 2 ← that's expected because Bacon.later(1000,… will restart Bacon.fromPoll after 1 sec.
  • A subsequent subscriber gets the "current accumulated" value of the property as initial value.
    value for the transient subscriber 2 ← perhaps unexpected on Bacon.later(3000,… but it's the current value now.

I am not convinced that this is an improvement, because

  • regardless of the actual temporal sequence of values the temporal sequence of subscriptions influences the reported sequence of values, and
  • this creates an Observable which is a mix of a Property and an EventStream.

Keeping the stale value toProperty(EventStream)

Scenario: Values get pushed into our property from elsewhere (e.g. chat history, stock ticker with timestamp)
Behaviour: event processing is deactivated when the number of subscribers is zero, while the current value is persisted.
Effect: First subscriber gets a "stale" value as initial value.

As long as the subscriber can gets a sense of the age of that initial event (or all events of that property), "staleness" is not a problem. The examples, you @raimohanska, gave are great!

For all other kinds of values one would have to take care not to drop the subscription count below zero. Perhaps the user could optionally "mark" properties as being not re-subscribable. E.g. makeThrowOnReSubscription(property): Property. If such a property is re-subscribed it throws an exception.

Btw.
I copied the examples to JS using fewer events to have not such a full console log here:
dropStaleValues demo, withInitFn demo, EventStream resubscription demo and Property resubscription demo
But no need to use these they're the same as the original ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants