Make mergeMapConcurrently a public API

pirelenito commented 6 years ago

Summary

While implementing a very simple photo library service, I have an observer containing the paths of photos available in the filesystem.

Given their path, I must enrich this data with metadata that I need to extract from the photos. If I just use a flatMap directly (which has a concurrency limit of Infinity) I crash my server:

photos(path.join(__dirname, '../pics'))
   .flatMap(enrichMetadata)
   .observe(f => console.log(f))

Searching for solutions and eventually at the internals of most I've found the implementations of flatMap and tried changing its Infinity to 1:

export function flatMap (f, stream) {
  return mergeMapConcurrently(f, 1, stream)
}

And all of the suddenly the metadata of the photos are being fetched one-by-one.

To make it work in my local setup, I'm importing it and using it directly:

const mergeMapConcurrently = require('most/lib/combinator/mergeConcurrently').mergeMapConcurrently

However, this is a very nasty short-term solution, so I'm suggesting either making mergeMapConcurrently public, or allow configuring concurrency value directly in flatMap.

Does it make sense?

I'm not sure what is the best approach here.

Versions

most.js: 1.7.2

btw, Most is awesome ❤️

TylorS commented 6 years ago

I agree we should make this API public 100%.

In your particular example of a concurrency of 1 you may want to look at switchLatest which is optimized specifically for a concurrency of 1.

axefrog commented 6 years ago

@TylorS The difference would be that mergeConcurrently will wait for the first stream to end before switching, whereas switchLatest will discard the old stream whether or not it has ended.

TylorS commented 6 years ago

Sorry, I meant concatMap

pirelenito commented 6 years ago

Thanks @TylorS,

I'm actually using a concurrency of 4 :) So I guess I need it to be public as well.

pirelenito commented 6 years ago

This is the code in question: https://github.com/pirelenito/lumine/blob/master/server/src/index.js#L15

briancavalier commented 6 years ago

Hey @pirelenito. I'm totally cool with making it public. Would you like to submit a PR to do that?

BTW, have you tried using map + mergeConcurrently? It's not exactly the same, but may work, depending on your use case.

Cheers!

cujojs / most

Make mergeMapConcurrently a public API #505

Summary

Versions