Open raimohanska opened 5 years ago
One option ... is to have Properties throw an error in cases where you re-subscribe after the last subscriber has been removed.
Maybe this is not so bad: Letting the application actively decide for lazyness or eagerness.
Let's follow this path for a moment:
How would I implement such a common pattern, like making a shared resource available as Property
to clients which come and go (i.e are created and destroyed)? I guess only by making the property artificially "eager" ("hot") with a dummy subscriber as suggested.
But then we'd loose that lazyness: Even when there are no "real" observers the whole expensive subscription and mapping machinery of the property is kept running.
If these useless computations are a problem for the application it could manage property lifetime itself like this
// pseudo-code
var prop = observers.length
.map(l => l > 0)
.skipDuplicates()
.flatMapLatest( areAny => areAny ? createProperty(...) : Bacon.never() );
observers
.onValue(o => prop.onValue(o));
On the other hand, when keeping the property eagerly up-to-date is no problem for the application, then it's just the "eager-making" the application is responsible for.
In the other case, if one wants to retain lazyness and be able to re-subscribe to property - I have no idea. On re-subscription one would have to travel backwards in time and replay all events which lead to the current value. Any .reduce
, .scan
etc. would make this impossible anyway.
:thinking:
And I totally agree that it's counterintuitive that something like this (from Bacon FAQ) does not work: ...
Why is it counterintuitive? Because the documentation says: "A property has the concept of "current value"." ? The Readme explains it a bit more: "So things that change and have a current state are Properties, while things that consist of discrete events are EventStreams. You could think mouse clicks as an EventStream and mouse position as a Property."
Then why not make all Properties eager? This would acknowledge that a Property ("thing with a state") has to be kept up-to-date regardless of its observers until it is garbage-collected by the JS runtime.
I think the current description of Properties does not tell the whole story anyway. Should it not rather be (emphasised text added, striked-out text goes away in case of all-eager properties):
Bacon.Property a reactive property. Has the concept of "current value". .... Note: depending on how a Property is created, it may or may not have an initial value. The current value is the initial value, or the most recent value ~until there are no more observers~. The current value is the first value published to a new observer and stays as its last (only) value after the stream has ended.
In the proposal
to have Bus::toProperty() always add a dummy subscriber to the resulting Property.
would var property = bus.toProperty('FIRST');
kind of mutate bus
? I.e. would I need to think about the internal state (eagerness) of bus when I do bus.push('SECOND')
? Then I'd rather prefer another type of immutable "Property-Bus", or otherwise reasoning about bus.push
would require knowledge about the timings of all bus.toProperty()
calls?
A lot of questions in a single comment by friend!
Making all properties eager would, if I understand your suggestion correctly, prevent them from being garbage-collected because they are referenced by the underlying streams keeping them up to date. Unless, of course, we somehow were able to use weak references for the supply chain. Unlikely so.
I don't really believe my patchy little suggestion above would be a good idea. Would make one case work better but still leave room for error in more complex setups. Let's just forget about it.
Considering the Lazyness of Properties an observation which is not clear to me:
Right now a subscription to an observable turns it's dependency tree of combined observables eager (i.e. stateful). With the direction from bottom to top.
/* p1 p2 ^
\ / | eagerness travels upwards
p1orp2 |
*/
var p1 = Bacon.repeatedly(11000, [true, false]).toProperty(false);
var p2 = Bacon.repeatedly(15000, [true, false]).toProperty(false);
var p1orp2 = p1.or(p2);
// this makes p1orp2 and its dependencies p1 and p2 stateful
p1orp2.onValue(x => x); // false, true, true, true, false, true, false, true ...
However, does the eagerness of the tree also travel down into the combined trunk? I mean, does the subscribing to p1
and p2
make p1orp2
eager too?
Without looking at the source code I find this is often the case. Although I would not expect that.
However I find the surprising behaviour in this particular example:
// p1, p2 and p1orp2 see above
var p1, p2, p1orp2 = ...
// just make the p1 and p2 dependencies eager
p1.or(p2).log("p1orp2");
Bacon.mergeAll(
Bacon.later(7000, "A"),
Bacon.later(12000, "B"),
Bacon.later(19000, "C"),
Bacon.later(35000, "D")
).flatMapConcat(
x => Bacon.once(x).holdWhen(p1.or(p2)).delay(500)
).log();
// p1orp2 false
// A <-- correct because no holding
// p1orp2 true, p1orp2 true, p1orp2 true <-- correct no emissions
// p1orp2 false B C <-- correct B is released now and C passes because no holding
// p1orp2 true D <--- WRONG D must not pass while p1 || p2 is true
// <end>
With bottom up eagerness the example works like expected. The code is nearly identical but one has to introduce a seemingly useless variable and not forget to make it eager explicitly.
// p1, p2 and p1orp2 see above
var p1, p2, p1orp2 = ...
// make the whole tree eager;
p1orp2.log("p1orp2");
Bacon.mergeAll(
Bacon.later(7000, "A"),
Bacon.later(12000, "B"),
Bacon.later(19000, "C"),
Bacon.later(35000, "D")
).flatMapConcat(
x => Bacon.once(x).holdWhen(p1orp2).delay(500)
).log();
// p1orp2 false
// A <-- correct because no holding
// p1orp2 true, p1orp2 true, p1orp2 true <-- correct no emissions
// p1orp2 false B C <-- correct because no holding anymore
// p1orp2 true
// p1orp2 false D <-- correct because no holding
//<end>
So if eagerness would propagate also from top to bottom one could write .holdWhen(p1.or(p2))
or .holdWhen(p1.and(p2))
or whatever combinator without the need to manage an extra instance of the combined observable.
Anyway I don't understand why sometimes a combined property seems to "work a little" even though
there is no "dummy" subscriber. I.e I wonder why the second example above not breaks down right away, but after some iterations? Why does .holdWhen(p1.or(p2))
let pass D
when p1
is true
?
var bus = new Bacon.Bus; var property = bus.toProperty('FIRST'); bus.push('SECOND'); property.onValue(function(val) { console.log(val); });
To me, it makes perfect sense that 'SECOND' will be missed. It’s maybe unintuitive at first, but once you realize how observables work, anything else would be unintuitive.
One option I'm half-seriously considering is to have Properties throw an error in cases where you re-subscribe after the last subscriber has been removed. This is probably always a bug in application code, because it can lead to propagating outdated values.
I prefer and rely on Property
retaining its state after the last subscriber unsubscribes. I understand how it can feel wrong, but I think throwing an error is a bit extreme.
How about simply dropping its state if it was obtained lazily? That's a can of worms, though. If a Property
has had many events and its initial state was created at initialization (e.g. constant(1)
), after all subscribers have unsubscribed, does the next subscriber get the initial value of 1
or the latest value? I think it should get the initial value.
To me, it makes perfect sense that 'SECOND' will be missed.
I still don't think that makes sense. How can the first item in a multicast property stream depend on the temporal order of subscriptions from other consumers in entirely different modules of the application? The current subscriber has no idea when other subscribers hook into the property stream.
How would you model such a trivial thing as a multicast property of the bowser's window size in baconjs?
var windowSize =
Bacon.fromEvent(window, 'resize')
.map(evt => ({width: evt.target.innerWidth, height: evt.target.innerHeight}))
.toProperty({width: window.innerWidth, height: innerHeight});
var unsubscribe = windowSize.onValue(console.log.bind(console));
In temporal subscription gaps to windowSize
the resize
event handler is removed and the property is not updated. Resubscription to windowSize
will definitely produce a faulty first value.
In my applications I carefully pay attention to manage a continuous lifetime of the such multicast properties. I think I am all-in for throwing an exception on resubscription to a property.
I still don't think that makes sense.
It makes sense to me because bus
has no subscribers when bus.push('SECOND');
is called.
How can the first item in a multicast property stream depend on the temporal order of subscriptions from other consumers in entirely different modules of the application? The current subscriber has no idea when other subscribers hook into the property stream.
(Emphasis mine)
Ah, now we're talking about something else – values being pushed to bus
when, through static analysis, we don’t know whether or not bus
has a subscription. Which segues into your example:
var windowSize =
Bacon.fromEvent(window, 'resize')
.map(evt => ({width: evt.target.innerWidth, height: evt.target.innerHeight}))
.toProperty({width: window.innerWidth, height: innerHeight});
var unsubscribe = windowSize.onValue(console.log.bind(console));
I think I am all-in for throwing an exception on resubscription to a property.
You should also be all-in for throwing an exception on the very first subscription to windowSize
. You correctly identified the issue of there potentially being a large temporal gap between the subscription count increasing from 0 to 1 after it had previous subscribers, thereby making the Property
’s last event stale. However, you have missed the fact that the very first subscriber to windowSize
may subscribe at any time, not necessarily immediately after windowSize
has been created. Therefore the value passed to .toProperty
is also potentially stale.
This illustrates the danger of .toProperty(initialValue)
existing at all.
Here’s my proposal:
Property
unsubscribes, clear its state, because it’s stale..toProperty(initialValue)
because initialValue
can be stale by the time the first subscriber subscribes..toProperty(getInitialValue)
to get a fresh initial value for the first subscriber.Using the above proposal, your Property
could be safely rewritten as follows:
var windowSize =
Bacon.fromEvent(window, 'resize')
.map(evt => ({width: evt.target.innerWidth, height: evt.target.innerHeight}))
.toProperty(() => ({width: window.innerWidth, height: innerHeight}));
When the subscription count increases from 0 to 1, the callback passed to toProperty
will be called and its result will seed the Property
with an initial value for subscribers. This will happen whether or not the Property
has previously had subscribers.
Alternatively, if there's a reasonably concise and expressive way to achieve this with current Bacon constructs, then .toProperty(getInitialValue)
may not be needed.
I still don't think that makes sense.
It makes sense to me because
bus
has no subscribers whenbus.push('SECOND');
is called.
That's the technical explanation of the behaviour but not the explanation why in the context of reactive streams one should expect such a behaviour.
Such a property stream is mutated from "live" to "lazy" at – if we're async – nondeterministic points in time, thus it is not const
and not referential transparent. This makes it hard to use correctly.
Add .toProperty(getInitialValue) to get a fresh initial value for the first subscriber.
Yes, making .toProperty
lazy could solve some problems. It's a good idea, but sometimes it's not possible to pull the current value into the property stream, for instance if it's derived from a WebSocket connection where the values are pushed into the stream.
Alternatively, if there's a reasonably concise and expressive way to achieve this with current Bacon constructs, then .toProperty(getInitialValue) may not be needed.
windowSize = Bacon.fromBinder(sink => {
sink({width: window.innerWidth, height: innerHeight});
window.addEventListener('resize', sink); // I left out mapping from ResizeEvent to {width: Number, height: Number}
return () => {
window.removeEventListener(sink);
};
})
.toProperty();
Should do the job but it is not as pretty as .toProperty(getInitialValue);
.
Edit:
I think in general getInitialValue
should return a promise.
Should do the job?
I thought “reasonably concise and expressive” implied “Don’t use Bacon.binder
” 😆
Yes, making
.toProperty
lazy could solve some problems. It's a good idea, but sometimes it's not possible to pull the current value into the property stream, for instance if it's derived from a WebSocket connection where the values are pushed into the stream.
The idea is to make .toProperty
get its value just in time, not lazily. This is so the initial value is sampled from the current state of some resource exactly at the point in time that the first subscriber subscribes, while guaranteeing that the subscriber receives its first event synchronously.
I think in general
getInitialValue
should return a promise.
I wouldn’t want to have to provide a Promise to
.toProperty` when I know I can sample the initial state synchronously. e.g. this is too much:
var windowSize =
Bacon.fromEvent(window, 'resize')
.map(evt => ({width: evt.target.innerWidth, height: evt.target.innerHeight}))
.toProperty(() => Promise.resolve({width: window.innerWidth, height: innerHeight}));
Also, I wouldn’t want .toProperty
to magically detect and unpack Promise
s to give me the flexibility of returning a value or a Promise
. It would be too much magic and I don’t think there’s a precedent in Bacon for dealing with Promise
s in this way. This is probably what Bacon.fromBinder
is for.
The idea is to make .toProperty get its value just in time, not lazily.
Lazy evaluation means that the value is computed when needed and not at the time when the property is defined. .toProperty(initialValue)
computes the initial value at definition time while toProperty(getInitialValue)
executes getInitialValue
every time the property is re-subscribed.
I wouldn’t want to have to provide a Promise to .toProperty` when I know I can sample the initial state synchronously.
There are plenty of examples in the browser or node.js APIs where you cannot pull a value synchronously (e.g. navigator.mediaDevices.enumerateDevices()
, retrieving data from IndexedDb, fetch
, navigator.geolocation.getCurrentPosition
, ...). How would you implement for example the getInitialValue
function for a GeolocationPosition
property?
Anyway – returning a promise or not – it does not solve the problem where the values are pushed into the property from elsewhere (e.g. WebSocket) and the current value cannot be pulled in. Contrary to intuition values are lost in a non-plausible way. Edit: Unless properties are made eager at definition time and maybe get a dispose()
method for deterministic clean-up.
How would you implement for example the
getInitialValue
function for a GeolocationPosition property?
Think about it for a moment. If the part of the stream before .toProperty
is emitting events as they occur, then .toProperty(somePromise)
is going to emit a duplicate event.
For example:
const windowSize = Bacon
.fromEvent(window, 'resize')
.map(evt => ({width: evt.target.innerWidth, height: evt.target.innerHeight}))
.toProperty(() => new Promise(resolve => (
const listener = evt => {
// Duplicate event - will also be emitted by fromEvent
resolve({width: evt.target.innerWidth, height: evt.target.innerHeight})
window.removeEventListener('resize', listener)
}
window.addEventListener('resize', listener)
))
This is a bit contrived, but I think it illustrates the point.
What I meant was something like
...
.toProperty(() => new Promise(resolve => navigator.geolocation.getCurrentPosition(resolve)));
Presumably, this is the part before .toProperty
:
const position$ = Bacon
.fromBinder(sink => {
const handle = navigator.geolocation.watchPosition(sink)
return () => Geolocation.clearWatch(handle)
})
What’s the point of asynchronously resolving the first event when we’re going to do it anyway via watchPosition
? Can’t we simply add .toProperty()
and be done with it? If we’re resolving the initial value asynchronously, it means it’s not available. It will be available later, so why not just subscribe to the underlying stream and get the initial value later anyway?
Yes, if we use Bacon.fromBinder
we might not need a promise-returning function in .toProperty()
.
I thought your point was not to use Bacon.fromBinder
. 😉
I made that Geolocation example just to demonstrate that getting initial values is sometimes asynchronous, opposite to my windowSize
example.
Imagine one had written a Bacon.fromGeolocationChangesEventStream()
. Then you'd perhaps make a property of it by
Bacon.fromGeolocationChangesEventStream()
.toPropery(() => new Promise(resolve => navigator.geolocation.getCurrentPosition(resolve)));
in order to have the current position asap and not wait for movement to get a fix on the location. (Just another made-up example, please take it with caution)
Imagine one had written a
Bacon.fromGeolocationChangesEventStream()
. Then you'd perhaps make a property of it byBacon.fromGeolocationChangesEventStream() .toPropery(() => new Promise(resolve => navigator.geolocation.getCurrentPosition(resolve)));
The first subscriber to Bacon.fromGeolocationChangesEventStream()
would presumably read and emit the initial state in the setup (on first subscription), thereby rendering .toProperty(promiseThatGetsLocation)
redundant.
Suppose we had Bacon.fromTextInputValue(element)
and it returns an EventStream
. I don’t know about you, but I’d expect the first subscriber to immediately (although slightly asynchronously, because it’s an EventStream
) receive an event containing the initial value. Its utility is compromised if the initial value of the input has to be retrieved manually.
Because it's fun, another example without Bacon.fromBinder
var clipboardText =
Bacon.fromEvent(document, 'paste')
.flatMapLatest(() => Bacon.fromPromise(
navigator.clipboard.readText()
))
.toProperty(() => navigator.clipboard.readText()); // <-- returns a promise
If Bacon.fromClipboard
existed, it would probably do the initial readText()
for the first subscriber on setup 😛
Hi guys! I wrote something on property reactivation on #770. What do you think?
I've read (and answered to) numerous complaints that code involving
bus.toProperty()
"does not work". And I totally agree that it's counterintuitive that something like this (from Bacon FAQ) does not work:You'd like the console to log 'SECOND' but it logs 'FIRST' instead.
What's happening here is that your Property won't get updated when there are no listeners. Before you add an actual listener using onValue, the Property is not active; it's not listening to the underlying Bus. It ignores input until someone's interested. So it all boils down to the "laziness" of all Bacon streams and properties. This is a key feature too: the streams automatically "plug in" to their sources when a subscriber is added, and "unplug" when there are no more subscribers.
A simple patch for this would be to have
Bus::toProperty()
always add a dummy subscriber to the resulting Property. Then, again, it would fail in setups like this:... so it's not such a great patch after all. Or is it? Because if you flipped it like
it would be good again!
The underlying problem is that the subscriber-count-based mechanism for change propagation in Bacon.js doesn't serve you well in cases when you create a stateful Property on top of a stateless EventStream unless you always have at least one subscriber on the Property.
One option I'm half-seriously considering is to have Properties throw an error in cases where you re-subscribe after the last subscriber has been removed. This is probably always a bug in application code, because it can lead to propagating outdated values.
Thoughts?