Open adrianmcli opened 7 years ago
Awesome! Thanks for taking an interesting in the project.
Was there anything you found confusing about the examples currently in the README? Especially re: not being clear that fromComponent
needs to be used.
I'll add this as an example to the README soon. Or you can open a pull request of course! :)
Yeah, I can probably put out a PR this weekend. Do you have any comments on my code though? I'm pretty new to RxJS, so I'm not sure if I'm doing things quite the right way yet.
From a code correctness perspective, there are two things that I see:
I don't believe that the .share
operators are necessary in this case, since we only have one subscriber. Even if we had multiple subscribers, the .share
operator should be used judiciously.
Make sure you unsubscribe from your observables on component unmount to help prevent memory leaks
Doesn't the fact that there are two subscription statements (see the bottom of componentDidMount()
) mean that the originating stream (i.e. this.inputEvents$
) is being subscribed to twice?
Did I not do this in componentWillUnmount()
?
I edited/updated my comment with more points before I saw your reply. I'll post them later.
Clearly I misread/misremembered your original post, I'm sorry! My second point is totally wrong, you are unsubscribing correctly.
When you create this.idle$
, it's a new observable, so the .share
on this.inputEvents$
does not act on this. You are still subscribing twice I think (I may be wrong on this).
Subscribing twice, in general, is fine though. Usually I only use .share
if I run into problems with subscribing creating some sort of side effect, but in this case it's fine.
It seems to work without the .share()
so I suppose we can take it out.
I'm actually also doubting if I'm unsubscribing properly now. I think I might need to save the subscription object from calling .subscribe()
and then call .unsubscribe()
on that specific subscription object, but I'm not sure.
That's correct! I missed that. Yes, you need to keep a reference to what is returned by .subscribe()
Revised example:
import React from 'react';
import { observeComponent, fromComponent } from 'observe-component/rxjs';
// Create the component with the listeners we want
const TextArea = observeComponent('onInput')('textarea');
export default class MyComponent extends React.Component {
constructor() {
super();
this.state = { typing: false };
}
componentDidMount() {
// Create stream for when user is typing
const inputEvents$ = fromComponent(TextArea, 'onInput');
// Create stream for when user has been idle for 1 second
const idle$ = inputEvents$.debounceTime(1000);
// Subscribe to invoke the streams and save the subscriptions
this.inputEventsSub = inputEvents$.subscribe(() => this.setState({ typing: true }));
this.idleSub = idle$.subscribe(() => this.setState({ typing: false }));
}
componentWillUnmount() {
// Unsubscribe from the streams when component unmounts
this.inputEventsSub.unsubscribe();
this.idleSub.unsubscribe();
}
render() {
return (
<div>
<TextArea cols="30" rows="10" />
<div>{this.state.typing ? 'User is typing...' : 'User is idle'}</div>
</div>
);
}
}
I had a little bit of trouble getting started in the beginning. I didn't realize that
fromComponent
andobserveComponent
were both needed. In any case, I've created the following example for other people who are also using React and RxJS v5 with this library.@Lokeh feel free to add this example to your docs, and thanks for the library! Please let me know if there can be any improvements to the following code.