Minimizing API surface area by removing some methods

[rpominov]

I've watched a great talk by @sebmarkbage http://www.youtube.com/watch?v=4anAwXYqLG8 lately, and also work on a React+Flux project with relatively large amount of boilerplate code. And after thinking about it for some time I'm convinced that boilerplate code is not a problem at all, as far as it stays simple.

Kefir has some methods that are basically short hands for two lines of boilerplate code, and I now think it was a mistake to introduce them. It doesn't worth it to maintain docs/tests for them, and also library users will only benefit from not having them.

For example we have .tap which can be easily replaced by .map. Compare this two code snippet:

stream.tap(function(x) {
  sideEffect(x);
});

stream.map(function(x) {
  sideEffect(x);
  return x;
});

First of all, do we need .tap when .map works just fine for this use case? But also, which snippet is easier to understand if you didn't read docs for neither .map nor .tap? To me it pretty clear what happening in .map version, but for .tap I'd better check the docs.

So I think we should remove some methods from the API, here is the list:

• repeatedly (alternative Kefir.repeat(function() {return Kefir.sequentially(/*...*/)}))
• mapTo
• pluck
• invoke
• not
• timestamp
• tap
• and
• or

Of course the removing won't be fast to not break existing code, I think we'll only deprecate them now, but remove only in v3.0.0 or later.

[denyskoch]

I love the idea behind http://moutjs.com/:

every piece of code or simple functions are own modules, so you need to include them, if you use them. the resulting bundle.js (if you are using browserfy) is minimal, based on used features.

why not use this pattern?

want only use streams, just require('kefir/streams') ahh need tap? just require('kefir/utils/tap') oder just require('kefir/utils')

[rpominov]

The file size isn't my concern here. For short answer we just don't need these methods, for long re-read my original comment :)

[ZeroFactor]

Haven't watched the video, skimmed the transcript, just barely starting with Kefir.

One of the things I really like is how these convenience methods (tap, or, and, etc.) provide built-in documentation for code written using them.

I checked out Kefir a while ago and back then I did feel like the library had a high api surface area. However, coming back to it more recently helped me cement my understanding of streams, particularly because the api provides a good translation between English and the impact of each method on the streams. The excellent documentation helps with this too.

If you decide to ditch the convenience methods, please consider maintaining a separate library that will add them back in when included.

[rpominov]

• Deprecated methods now warn in console when called.
• Docs on deprecated methods moved to separate file https://github.com/pozadi/kefir/blob/master/deprecated-api-docs.md

I'll release a minor version with this changes a bit later.

[appedemic]

I think keeping the API small and simple is the way to go. Anything needed can be plugged into a map function. I personally like to use Ramda in combination with Kefir. With Ramda it's easy to move the deprecated functionality into the map function itself, where it belongs. Instead of observable.pluck('x') it would become observable.map(R.pluck('x'))

[monfera]

Just a 2c, I like how much quicker this parses

x.pluck('y')

than

x.map(R.pluck('y'))

There's something fluid about dropping in a .log() or .tap() anywhere in the plumbing for quick tests. Since FRP is meant to be temporally aware, timestamp() also feels pretty much a shared idiom.

However I also agree that it's nice to separate fundamental FRP-like concerns from, on one hand, shorthands, and on the other hand, other FP features such as transducers and generic FP like Ramda (a lot of FRP functions can be built up with a basic FRP plus transducers).

So maybe we can have the cake and eat it too, if the user of the Kefir library can say, e.g.,

Kefir.Observable.prototype.pluck = function(prop) {
    return this.map(R.pluck(prop));
};

which is similar to how it's implemented now, and Kefir.Observable is exported, so it's legal.

Observable.sampledBy also carried intent in a way I find useful: no need to look at the number of arguments, it immediately conveys that we're downsampling here. Again, pretty trivial to add in userland, but then library users will end up with conceptually similar constructs that however are named differently.

By the way, one of the things I like in Ramda, Highland etc. too is that they have large vocabularies, and so do Underscore, d3.js and most popular JS libraries.

[rpominov]

I think we should proceed and remove Kefir.fromSubUnsub(), alternative is Kefir.fromBinder().

[rpominov]

remove Kefir.fromSubUnsub()

done in v2 branch

[shamansir]

Some deprecated methods were quite nice for me, since for me, they make code readable.

Like .mapTo, yes—it's the shortcut for just a two-liner, but when you see you re-use same code, you find yourself in desire to wrap repeating lines in a function, consider:

streamOfTwo.map(function(x) { return 2; });
streamOfThree.map(function(x) { return 3; });
streamOfFour.map(function(x) { return 4; });

and:

streamOfTwo.mapTo(2);
streamOfThree.mapTo(3);
streamOfFour.mapTo(4);

and:

function toValue(value) { return ( function() { return value; } ) }
streamOfTwo.map(toValue(2));
streamOfThree.map(toValue(3));
streamOfFour.map(toValue(4));

Which one is nicer?

[rpominov]

This will look nicer with arrow functions stream.map(() => 2), and before that you can either use deprecated method or define your own helpers.

[shamansir]

Yeah, with arrows its the nicest way!

[rpominov]

Babel ftw :metal:

[ivan-kleshnin]

👎 for removing tap

It's a great debugging tool.

Now instead of x.tap(x => console.log(x)) we have to x.map(x => console.log(x); return x)

spy solves it partially but is not customizable (for example, I want to do next and it's only possible with map now)

console.log(event.target)
console.log(event.target.attributes)
console.log(event.target.attributes.href)

[rpominov]

You can write x.map(x => console.log(x); return x) as x.map(x => (console.log(x), x)) :)

[mAAdhaTTah]

Alternatively, you can bring it back yourself by writing your own helper function and using thru:

const tap = fn => obs => obs.map(x => (fn(x), x)).setName(obs, 'tap')

Usage:

obs.thru(tap(x => { /* do whatever */ }))

[ivan-kleshnin]

Yep. I just think those few bytes of tap was pretty worth it. It wasn't the best candidate to remove IMO.

[rpominov]

This is not really about bytes. IMO, making API as small as possible is beneficial for learning (see the video in original comment). Also tap encourages to add side effects into transformation chains, which is a bad practice. Side effects should live in observers.

[ivan-kleshnin]

Also tap encourages to add side effects into transformation chains, which is a bad practice. Side effects should live in observers.

Yes, but it's not true in ALL scenarios. I'm aware of couple of cases where it's really better to make side effects in tap. Debugging is just one of them. Anyway, it's not the biggest issue for me and I respect your right to have a different opinion.