tsers-js / core

Transform-Signal-Executor framework for Reactive Streams
MIT License
145 stars 4 forks source link

Modularize mux and demux #17

Open tusharmath opened 8 years ago

tusharmath commented 8 years ago

mux and demux are some awesome util functions. It would make a lot of sense to take it out of this project as a standalone dependency.

milankinen commented 8 years ago

Hmmm. That's very interesting proposal. I don't see it impossible to extract core transforms into separate module, e.g @tsers/prelude. It might even clear the codebase.

tusharmath commented 8 years ago

@milankinen Does it have to be a part of tsers? I mean, it could be a completely different module which is stream agnostic too.

milankinen commented 8 years ago

mux and demux use only map and filter so that is indeed possible! Do you want to create a new npm module and copy those functions into it (it's fine for me)? Or do you want that I'll do that?

tusharmath commented 8 years ago

@milankinen I would be glad to create a separate module. What would you want to call it?

milankinen commented 8 years ago

That is a hard question. Something related to signal processing perhaps? I'm counting on your judgement in this matter. 😄

tusharmath commented 8 years ago

Here it is — https://github.com/tusharmath/muxer Will create a pull request with this as a dep.

milankinen commented 8 years ago

Nice 👍

tusharmath commented 8 years ago

@milankinen What is the purpose of demuxCombined in this module? I have not seen it being used in anywhere and quite a lot of test cases have been written for it.

ghost commented 8 years ago

@tusharmath check https://github.com/tsers-js/examples

milankinen commented 8 years ago

demuxCombined must be used if you have an observable containing an array of muxed observables (usually when you're dealing a list).

Like @pmros pointed out, an example usage can be found here: https://github.com/tsers-js/examples/blob/86810a6cbe76e63cf35493a7005fe9547ec62b48/examples/counter-list/index.js#L32-L43

tusharmath commented 8 years ago

@milankinen This should too be out of the package isn't it?

milankinen commented 8 years ago

It's up to you to decide that. 😄 I'm fine with both options (in/out).

There is not much code, though: https://github.com/tsers-js/core/blob/master/src/index.js#L72-L78