Skip to content
This repository has been archived by the owner on Sep 10, 2022. It is now read-only.

rx-recompose: On new props, only one stream gets updated when using combineLatest() #158

Closed
gregmuellegger opened this issue May 14, 2016 · 5 comments · Fixed by #160
Closed

Comments

@gregmuellegger
Copy link

Hi, here is a strange behaving component that I created with rx-recompose.
When I mount the component, I'll see in the console first, second, thrid. That's expected.

However, when new props are given (done with the button click from the second example), I only see third in the console. It seems that only the LAST use of props$ will be taken into account for the combineLatest call.

Am I getting things wrong here? Maybe I don't grasp RxJS enough so far to see the issue, but I would tremendously appreciate any hint here. My understanding is that the behaviour for the first props and the behaviour for new props should be the same.

I have sunken about 6 hours now on this 😁

const Example = createComponent(props$ => {
    return props$.tap(() => console.log('first')).combineLatest(
        props$.tap(() => console.log('second')),
        props$.tap(() => console.log('third')),
        (props, props2, props3) => {
            return <div>{props.value}</div>;
        }
    );
});

Here is how I mount it:

<div>
    <Example value={value} />
    <button onClick={() => setState({ value: value + 1 })}>Click</button>
</div>

When clicking the button, the counter will not increase, since only the third parameter in the propsToVdom function above will contain the correct value. The other two are kept with the old value.

@gregmuellegger
Copy link
Author

Oh, maybe I just did get the idea of the issue after 6 hours of glancing at it and five minutes after typing this ticket:

In createComponent there is this bit:

    // Stream of props
    props$ = Observable.create(observer => {
      this.propsObserver = observer
      this.propsObserver.onNext(this.props)
    });

That means for every subscription of the props$ stream, we override the this.propsObserver. And while doing the combineLatest, we have created more streams that all need one subscription.

My motivation to have multiple subscriptions is that I want to use RxJS to react to changes in a specific prop, like:

const searchQuery$ = props$.pluck('searchQuery').filter(query => query.length > 0);
const searchResults$ = searchQuery$.flatMapLatest(query => search(query) /* that returns a promise */);
return props$.combineLatest(searchQuery$, searchResults$, (props, query, results) => { ... });

@acdlite
Copy link
Owner

acdlite commented May 14, 2016

Good catch! I have a fix for this, I'll get it in soon.

@acdlite
Copy link
Owner

acdlite commented May 15, 2016

@gregmuellegger #160 should fix this. Need to write a test before merging. Sorry you spent so much time debugging this!

@acdlite
Copy link
Owner

acdlite commented May 15, 2016

@gregmuellegger Released in 0.6.2! I used your example as a unit test. Thanks again for your investigative work. :)

@gregmuellegger
Copy link
Author

Cool, thanks for the very quick fix! That's helping a lot.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants