MichalZalecki / connect-rxjs-to-react

Connect rxjs to React component in Redux style... but without dispatch and constants.
187 stars 27 forks source link

Bug in connect function #2

Closed almeynman closed 8 years ago

almeynman commented 8 years ago

I have weird behaviour with your connect function

return class Connect extends React.Component {
      constructor(props) {
        super(props);
        state$.take(1).map(selector).subscribe(state => this.state = state);
      }

      componentDidMount() {
        this.subscription = state$.map(selector).subscribe(::this.setState);
      }

      componentWillUnmount() {
        this.subscription.unsubscribe();
      }

      render() {
        return (
          <WrappedComponent {...this.state} {...this.props} />
        );
      }
    };

I think initial value sets state twice in constructor and in componentDidMount

I suggest to change that to

return class Connect extends React.Component {
      constructor(props) {
        super(props)
        this.componentHasMounted = false
        this.subscription = state$.map(selector).subscribe(
          state => !this.componentHasMounted ? this.state = state : this.setState(state)
        )
      }
      componentDidMount() {
        this.componentHasMounted = true
      }
      componentWillUnmount() {
        this.subscription.unsubscribe()
      }
      render() {
        return (
          <WrappedComponent {...this.state} {...this.props} />
        )
      }
    }
MichalZalecki commented 8 years ago

You have older code. This is up-to-date version: https://github.com/MichalZalecki/connect-rxjs-to-react/blob/master/src/rx-state/connect.js#L6-L8

almeynman commented 8 years ago

Great!

MichalZalecki commented 8 years ago

I've actually should update the article :)