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

Observable.map() should run once then fan-out #5

Closed
RickWong opened this issue Nov 17, 2015 · 7 comments
Closed

Observable.map() should run once then fan-out #5

RickWong opened this issue Nov 17, 2015 · 7 comments

Comments

@RickWong
Copy link

The map(callback) is called for each child subscription. That does not work for byte streams that can only be read once (Fetch API's response.json() for example can only be called once). Therefore the child observable should run the callback once and fan-out the result.

I've implemented it here: https://github.com/RickWong/fetch-observable/blob/master/src%2Flib%2FBetterObservable.js#L15 (this map() implementation also supports resolving Promises)

@RangerMauve
Copy link

As you can see in this discussion, that is by design. Apparently there's a different type called "subject" which provides the fan-out capability which is outside of this spec, AFAIK.

Personally, I share your use case, but it just so happens that that's what the semantics are around the name in the wild.

@RickWong
Copy link
Author

I think a cold/unicast map() function is useless. Then you might as well not implement map() at all, and just place the callback in the observer. The strength of a fan-out/multicast map() is that the original observable still acts the same for existing observers, while the newly returned observable adds value by running the callback first for its own observers.

@RangerMauve
Copy link

Yeah, it might make sense to post that in the issue in the actual spec's repo since I doubt many people are monitoring this repo for comments on the spec itself. :D

@RickWong
Copy link
Author

Unfortunately they decided to remove map() from the spec entirely in that thread. So it's going nowhere. All the talk about an Observable "interface" and "primitives" makes me believe they don't plan to build real applications with it.

@RangerMauve
Copy link

I think the decision to remove map was made in this issue but, yeah, I think it was sealed in that one.

As I understand, at the very least this will provide an inter-op for existing observable-like libraries if they all implement the spec. I think it'd still be valid if you posted your thoughts in there since comments always help in situations like this.

Also, it seems that the decision to not fan-out was made in order to make observables more functionally pure.

@zenparsing
Copy link
Owner

@RickWong I like how you've used subclassing to get the semantics that you want here. That's the kind of customization we want to allow for.

I removed "map" from the es-observable proposal because we are trying to specify a minimal protocol (and protocols are good things, right?) as a starting point. I would personally like to see the bind syntax proposal take off, so that you can create your observable chains by importing combinators, something like:

import { mapFan } from "observable-combinators";
let fannedObservable = makeObservable()::mapFan(x => y);

That way we wouldn't have to argue about what goes on Observable.prototype.

That said, I think there are still good reasons for making things like "map" and "filter" not support fan-out by default. Fanning out is a general-purpose combinator, so another strategy would be to just add the "fanOut" combinator explicitly at the points in your chain where you want to support it:

fetchObservable({ ... })
.map(response => response.json())
.unwrapPromises()
.fanOut();

@zenparsing
Copy link
Owner

Closing, please reopen if anyone would like to continue discussion.

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

3 participants