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

Signals are lost in some cases #271

Open
mehtonen-boox opened this issue Jan 18, 2018 · 10 comments
Open

Signals are lost in some cases #271

mehtonen-boox opened this issue Jan 18, 2018 · 10 comments

Comments

@mehtonen-boox
Copy link

mehtonen-boox commented Jan 18, 2018

I'm not sure if this is a bug, but I found this strange behavior of signals getting lost on some cases.

const pool = kefir.pool();
const prop = pool.toProperty(() => 0);
const later = kefir.later(100, null);

const stream = later
 .flatMapLatest(() => kefir.merge([prop.take(1), pool]))
 .log('stream');

pool.plug(stream.take(1).map(() => 1));

I would expect stream to log with 0 and 1, but only 0 gets logged. Or I'm I misunderstanding this somehow?

To clarify issue a little bit, if I change .flatMapLatest(() => kefir.merge([prop.take(1), pool])) to .flatMapLatest(() => prop) it will log 0 and 1. So it seems pool won't give signals in this case even though it should according to my understanding.

@ivan-kleshnin
Copy link

ivan-kleshnin commented Jan 20, 2018

  1. K.later should be called like K.later(time, value). Docs don't mention any defaults for this function.

  2. https://kefirjs.github.io/kefir/#later

laterKefir.later(wait, value)
Creates a stream that produces a single value after wait milliseconds, then ends.

@mehtonen-boox
Copy link
Author

@ivan-kleshnin issue was originally produced without using later. It was the most simplest way to make stream give signal async. I updated description and added something to highlight the problem a little bit more.

@ivan-kleshnin
Copy link

ivan-kleshnin commented Jan 22, 2018

Seems like pool and prop end up glued together into a single prop.

import K from "kefir"

let pool = K.pool()
let prop = pool.toProperty(() => 0)

let stream = K.later(100, null)
 .flatMapLatest(() => K.merge([prop.take(1), pool]))
 .log('stream')

pool.plug(K.constant(1))
pool.plug(K.constant(2))
pool.plug(K.constant(3))

//---3|

Using other properties in K.merge does not have this effect. I dunno why this happens.

@mehtonen-boox
Copy link
Author

@ivan-kleshnin in your case when flatMapLatest is run, all the constant signals are already sent. You will get only the latest value by using prop.take(1), which is 3. Pool won't get any new signals and therefor only 3 is logged. In this case this works as expected.

In my case pool should give signal with value of 1, because that signal is sent after flatMapLatest.

@ivan-kleshnin
Copy link

ivan-kleshnin commented Jan 22, 2018

Yep, makes sense. I never used plug for async stuff so I didn't get what you're trying to demonstrate here at first.

@tweinfeld
Copy link

@mehtonen-boox I think this is actually the expected result:
pool becomes active a split second after the prop constant is consumed by stream, at which point it will never receive any values from it via plug. That's because .merge() processes streams in order.
Evidently, if you switch the order in which you merge the streams, so that instead of:

Kefir.merge([prop.take(1), pool]);

You'd use

Kefir.merge([pool, prop.take(1)]);

stream would produce the expected result!

@mehtonen-boox
Copy link
Author

@tweinfeld Thank you! I didn't know the order matters while merging streams. 🤔 Now when I know this I might be able to avoid issues like this. 👍

@ivan-kleshnin
Copy link

ivan-kleshnin commented Mar 14, 2018

@mehtonen-boox it matters and there's also a difference between static functions and methods.

For example K.merge([x1, x2]) may work differently than x1.merge(x2), especially with cyclic dependencies...

@rpominov
Copy link
Member

there's also a difference between static functions and methods

This should be false, at least my intent was to make them exactly the same. For example x1.merge(x2) basically does K.merge([x1, x2]) internally:

kefir/src/index.js

Lines 305 to 307 in bacac22

Observable.prototype.merge = function(other) {
return merge([this, other])
}

@ivan-kleshnin
Copy link

ivan-kleshnin commented Mar 14, 2018

Sorry for bad description. What I wanted to say is that sometimes:

x.skip(1).merge(y)

won't work as you need and y.merge(x.skip(1)) is a different thing.
So your only option then may be a switch to statics:

K.merge([
  y,
  x.skip(1),
])

But it also boils down to order dependecy, yes.

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