baconjs / bacon.js

Functional reactive programming library for TypeScript and JavaScript
https://baconjs.github.io
MIT License
6.47k stars 330 forks source link

Include Bacon.Model in core? #305

Closed raimohanska closed 9 years ago

raimohanska commented 10 years ago

As mentioned in #304, a "settable property" would be nice.

The Bacon.Model library contains such a thing, i.e. the Bacon.Model object that has a .set(newValue) method. It also contains a bunch of extra functionality like 2-way bindings with Lenses, which probably isn't core Bacon.js stuff.

Should we include a simplified Bacon.Model in the core and have the Bacon.Model library add the extras on top?

chris-martin commented 10 years ago

I vote yes. This feature seems pretty inherent to FRP, and I found myself looking for it almost immediately when first trying to apply Bacon to my app.

In case some precedent would help make this case, I should point out that Bacon.Model corresponds to the Var type in Deprecating the Observer Pattern:

abstract class Signal[+A]
class Var[A](init: A) extends Signal[A] {
    def update(newValue: A): Unit = ...
}
jimmydivvy commented 10 years ago

Bacon.Model provides a mutable value with side effects, and might confuse the intended way to use BaconJS for new users. Since a lot of people will be coming from an MVC background, their first instinct will be to simply update a model with set inside a stream - at which point you might as well just skip Bacon entirely.

Bacon.Model does have a use, but ideally scan or withStateMachine should be encouraged as a method of maintaining state. Keeping Bacon.Model as a separate project hopefully makes it clear it's not part of the normal use-case.

(If I understand it correctly, #304 could be resolved by simply connecting the full network before starting to push events, so it's not really needed to solve that scenario?)

I'm still new to FRP, so I might be completely wrong. In which case ignore everything I just said.

chris-martin commented 10 years ago

I'm not an FRP expert myself, so be wary of my comments as well.

I agree that you're making a mistake if you call set from a stream reaction. Whether omitting the feature is worth trying to prevent users from shooting themselves in the foot in this way, I can't say.

set breaks you out of FRP, for sure. I'm using Models because they're quite handy for debugging; I can open the console and perturb with their values at will. I'm not sure how I'd do that otherwise.

sgerace commented 10 years ago

Having just started using Bacon.js on any serious scale, I will agree with @chris-martin that I almost immediately started looking for the functionality provided by Bacon.Model when beginning to use Bacon.js. I agree that there are situations where calling set is a major anti-pattern, however, there are some cases (most notably the one illustrated in this Bacon.js FAQ item) where it seems that Bacon.Model is the preferred way of doing things.

sgerace commented 10 years ago

I'll amend my initial thoughts by pointing out that currently if you take a Bacon.Model and transform it by an observable function, like observable.map, what you get out is not a Bacon.Model and is instead a Bacon.Property. If Bacon.Model is to be included into Bacon.js, it should be more closely integrated into the underlying observable system so that way everything is consistent (and you don't "lose" the Bacon.Model functionality through transformations). OR, at the very least make that clear in the documentation.

raimohanska commented 10 years ago

Well you can't get a Model by using map. Map uses a one-way mapping function so it can't sync values to both directions. Model.lens is the two-way map, if you like.

On 29.5.2014, at 16.48, Sal Gerace notifications@github.com wrote:

I'll amend my initial thoughts by pointing out that currently if you take a Bacon.Model and transform it by an observable function, like observable.map, what you get out is not a Bacon.Model and is instead a Bacon.Property. If Bacon.Model is to be included into Bacon.js, it should be more closely integrated into the underlying observable system so that way everything is consistent (and you don't "lose" the Bacon.Model functionality).

— Reply to this email directly or view it on GitHub.

sgerace commented 10 years ago

@raimohanska True, good point. I was confused for awhile in my code because Bacon.Model is technically an instanceof Bacon.Property so there were quite a few places I was expecting things to be models but where in fact vanilla properties (because they were mapped).

phadej commented 10 years ago

I disagree with @zoomzoom83. Calling set on a model in the side-effect of EventStream is really what I want to do sometimes.

For example in (data-flow) scenarios I end up tying the knot using Bus, modifying Simple counter example

var initialValue = 0;
var minValue = -5;
var maxValue = 5;

var $countBus = new Bacon.Bus();

var $count = $countBus.skipDuplicates().toProperty(initialValue);

var $plusActive = $count.map(function (c) { return c < maxValue; });
var $minusActive = $count.map(function (c) { return c > minValue; });

var $plus = $("#up").asEventStream("click").filter($plusActive);
var $minus = $("#down").asEventStream("click").filter($minusActive);
var $events = $plus.map(1).merge($minus.map(-1));

// tie the knot
$countBus.plug($events.scan(initialValue, function (x, y) { return x + y; }));

$count.assign($("#counter"), "text");
$plusActive.not().assign($("#up"), "attr", "disabled")
$minusActive.not().assign($("#down"), "attr", "disabled");

With Bacon.Model I'd have:

$count = new Bacon.Model(0);

//....

// essentially `.set`
$count.apply($plus.map(function (unused) { return function (x) { return x + 1; }; }));
$count.apply($minus.map(function (unused) { return function (x) { return x - 1; }; }));

//...

With complicated state transformations the latter approach IMHO is much easier to understand. No hidden state; mutations of global state are explicit.

The .set inside a stream will propagate the change everywhere. The most simple FRP/data-flow example:

a = 0;
b = 0;
c = a + b; // c = 0
a = 1;
// what is c?

works with properties (and models), not eventstreams. And uses .set. Thus for new (to FRP) people, models and properties might be the most natural thing to start with.

So I'd like having Bacon.Model in core.

P.S. I dislike scan and withStateMachine, for philosophical reasons. They hide state. Yet that might be sometimes practical.

chris-martin commented 10 years ago

Epilogue to my usage story:

I quickly strayed away from Bacon because I think FRP is too clunky in any language with such a shit lambda syntax. My app is now Angular, and there's one place I'm using Bacon.

Abstractly, the data type I need for this purpose has these methods:

That's all I really need - a value that can be set, gotten, and listened on. And that's what Model is to me.

That said, it's pretty easy to get around without Model, using Bus like @phadej said.

var bus = new Bacon.Bus();
var property = bus.toProperty();

That's what I'm doing, and it's not too bad. The biggest problem in my mind are:

I guess my tl;dr argument is: If you're going to have Bus, you might as well have Model too.

RoboTeddy commented 10 years ago

@chris-martin -- if you want concise lambda functions, consider using ES6 arrow functions or coffeescript