Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make mergeMapConcurrently a public API #505

Open
pirelenito opened this issue Nov 18, 2017 · 6 comments
Open

Make mergeMapConcurrently a public API #505

pirelenito opened this issue Nov 18, 2017 · 6 comments

Comments

@pirelenito
Copy link

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
Copy link
Collaborator

TylorS commented Nov 18, 2017

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
Copy link
Contributor

axefrog commented Nov 18, 2017

@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
Copy link
Collaborator

TylorS commented Nov 18, 2017

Sorry, I meant concatMap

@pirelenito
Copy link
Author

Thanks @TylorS,

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

@pirelenito
Copy link
Author

@briancavalier
Copy link
Member

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants