kefirjs / kefir

A Reactive Programming library for JavaScript
https://kefirjs.github.io/kefir/
MIT License
1.87k stars 97 forks source link

Integration with Recompose: props stream never ends #230

Closed jeron-diovis closed 7 years ago

jeron-diovis commented 7 years ago

Check out this fiddle:

https://jsfiddle.net/2j2esxyL/1/

Click "OFF" button and see "DONE" message in console.

That is, when <Counter/> unmounts, it unsubscribes from props$ stream, and finally callback is called. Which is very convenient, because it provides a reactive analogue of componentWillUnmount and so allows to do some clean up when needed.

Same example with Kefir:

https://jsfiddle.net/2j2esxyL/2/

Click "OFF" button and see no message in console. onEnd callback is not called.

As I understand, finally and onEnd are quite different. In case of Kefir, props$ isn't ended but just deactivated. Probably, I can consider that deactivation as an end (because props$ stream is guaranteed to be activated and deactivated only once) – but there is also no way to know that stream was deactivated, it's an internal mechanism. It is possible to patch _onDeactivation method, but obviously it's a dirty hack.

So, what can be done to get the same behaviour as with Rx?

rpominov commented 7 years ago

Hey! Not sure about deactivation, as far as I understand problem is that you wait for an end of observable combined from two other observables, and such an observable ends when all of it's sources end. In your case $props ends on unmoun, but timeElapsed$ stays alive. You can try something like this:

  const enhance = mapPropsStream(props$ => {
    const timeElapsed$ = Kefir.interval(1000).scan(x => x + 1, 0);
+   const propsEnd$ = props$.ignoreValues().concat(Kefir.constant(0));
    return props$.combine(timeElapsed$, (props, timeElapsed) => ({
      ...props,
      timeElapsed
    }))
+   .takeUntilBy(propsEnd$)
    .onEnd(() => console.log('DONE'))
  })

Although this also doesn't seem to work. Seems like props$ doesn't end on unmount either, and I'm not sure why or should it. I don't know much about Recompose, so this is as much as I can help for now, I hope this helps.

jeron-diovis commented 7 years ago

Closed with https://github.com/acdlite/recompose/issues/220. The problem was in recompose itself.