Bacon.flatMap(x => Bacon.fromPromise(...)) is returning Bacon.fromPromise() not the value of the Promise

[gfarrell]

I am using flatMap and Promises to wait for an async operation. This works in another service I've written (with near-identical code). The code is like this:

someStream().flatMap(packet => Bacon.fromPromise( someAsyncFn(packet) )).log()

When I try this in the REPL, I get what I expect in the log (i.e. the result of the Promise). When I run this in my app, I see Bacon.fromPromise([object Promise]) in the log.

To make sure my problem was with flatMap() not with my code, I tried:

someStream().flatMap(packet => Bacon.once(1)).log()

and in the console I expect to see 1 but instead see Bacon.once(1). This suggests that flatMap() is behaving in the same way as map().

If this didn't work in both the REPL and in another service I have written with (functionally) identical code, I would have assumed that I've misunderstood how to use flatMap(), but the only place it doesn't work is in this service as described above.

Using baconjs 0.7.71.

[raimohanska]

Sounds like you have two separate Bacon objects in your application, which causes flatMap not to recognize the created stream as a Bacon observable.

[gfarrell]

This can't be an uncommon situation, though. I have a library producing the streams, so that has it's own baconjs module, and then the service itself includes Bacon.

What I don't understand, however, is why this doesn't happen in a service written with practically identical code, and structure. The same problem should apply.

[raimohanska]

Well it's an npm thing. If you end up with two separate Bacon objects that's bad. Not entirely sure how npm resolves deps. I heard there's npm-dedup that might help ya.

On 8.9.2015, at 18.58, Gideon Farrell notifications@github.com wrote:

This can't be an uncommon situation, though. I have a library producing the streams, so that has it's own baconjs module, and then the service itself includes Bacon.

What I don't understand, however, is why this doesn't happen in a service written with practically identical code, and structure. The same problem should apply.

— Reply to this email directly or view it on GitHub.

[gfarrell]

Sure, but doing an object comparison (I assume if(thing instanceof Bacon.Observable)) is not a great way to check inheritance for a library in Javascript, simply because Javascript is not a strict class-inheritance language, so this happens pretty easily.

Better, IMO, (although it seems uglier at first) is to simply have a property thing._isObservable which can be checked. This is more reliable for Javascript.

[johannhof]

:+1: on this, we literally forked Bacon to be able to test it in an environment where we can start several node instances to test how our streams behave when given different initial input. instanceof gave us massive headaches

[johannhof]

@raimohanska if you don't mind I think either @gfarrell or I would love to do a pull request adding thing._isObservable to improve this

[gfarrell]

I'm happy to make a pull request. I don't see how there could be a negative impact of this.

[gfarrell]

(although, @johannhof, I'm not very familiar with the Bacon codebase having only recently started using it, so if you know exactly where to make edits, it would probably be more efficient for you to do it)

[johannhof]

I'll try :)

Unfortunately it's not only about Observables but also Event, Bus etc...

[gfarrell]

Well, Bacon.Bus is an Observable right?

[gfarrell]

Incidentally, for reference:

helpers.coffee#12

isObservable = (x) -> x instanceof Observable

[gfarrell]

OK, there's an easy way to do this (I think). Given isObservable() checks inheritance exactly as I suspected, you only need to change that check to

isObservable = (x) -> x._isObservable === true

and then add _isObservable: true to the Observable class.

[raimohanska]

I agree that it might be a good idea to get rid of instanceof checks.

There's another thing there too, though: there's some global state in the UpdateBarrier object, that may or may not cause trouble when having multiple Bacon instances. Just pointing out that having two Bacon versions working together in the same env is a non-trivial problem.

[raimohanska]

Anyway, getting rid of instanceof is one step towards a better world, so Pull Requests are of course very welcome. The other thing is the global state, which can probably be dealt with too, but requires a bit more thought.

[gfarrell]

@raimohanska I have made the required changes, I think, but I seem to be getting a hell of a lot of failing tests that are surely unrelated. I'll make the PR and you can check it out.