Number of operator changes behavior of it's input stream.

[ENvironmentSet]

Here is simple example with take operator.

import xs from 'xstream';

const a = xs.of(1, 2, 3);
const b = a.take(1);

b.addListener({ next: x => console.log('b', x) });
a.addListener({ next: x => console.log('a', x) });
/** console output
* b 1
**/

Yes, this makes sense. since a is hot observable.
but, this doesn't make sense at all:

import xs from 'xstream';

const a = xs.of(1, 2, 3);
const b = a.take(1).take(1);

b.addListener({ next: x => console.log('b', x) });
a.addListener({ next: x => console.log('a', x) });
/** console output
* b 1
* a 1
* a 2
* a 3
**/

IMO, in this case, number of operator take should not be important to stream a.
because operators should not change it's input stream. they are tend to modify input stream's output, not input stream.

anyway, I need other opinions. should we consider this as a bug? or something else(something like UB)?

[staltz]

@ENvironmentSet Sounds like a bug in my opinion. Thanks for reporting

[ENvironmentSet]

@staltz Humm, this seems very complex, it related with termination of stream(_x and _remove and _stopID. these things make this happen). To solve this problem, we need to change the way of terminating and resetting a stream.

[ENvironmentSet]

@staltz I've made patch for this problem(one for Take, one for Stream).
first one fixes this problem partially(still has some edge cases), and the other fixes fundamental problem(with this patch, first one covers every edge case which I found.). this patch needs more review but if you are interested in it, I can share it on this thread.

PS. patch for fundamental problem breaks many tests since it changes some logic about terminating stream.

[staltz]

@ENvironmentSet I'm of course curious to see the patch. Note however that if many tests break, it's probably not a suitable change to make, since tests reflect real-world use of xstream too.

[ENvironmentSet]

@staltz Indeed every patch should not destroy legacy code as much as it can(for backward compatibility).
So, I will try to find better way which would fix this issue and protect our legacies.

anyway, here is the fundamental problem(this might need some conversation to say it is real one): termination logic of stream is not solid. (it can be asynchronous but also can be synchronous!)

[ENvironmentSet]

@staltz

Here, this is what i meant.
I hope that you see this.