Search code examples
reactjsreduxreact-reduxredux-thunk

Should I use ComponentDidMount or mergeProps of connect function for data fetching?


I use react with redux and have a component which displays some dataSet fetched from external source. My current code looks like:

const ConnectedComponent = connect(
  state => ({
    dataSet: state.dataSet
  }),
  dispatch => ({
    loadData: () => {
      ...
      fetch data and dispatch it to the store
      ...
    }
  })
)(MyComponent); 

class MyComponent extends Component {
  ...
  componentDidMount() {
    const { dataSet, loadData } = this.props;
    if (!dataSet) {
      loadData();
    }
  } 
  ... 
  render () {
    const { dataSet } = this.props;
    if (dataSet) {
      // render grid with data
    } else {
      // render Loading...
    }
  }
}

The code above works but I'm wondering would it be better to get rid of componentDidMount and just check for data and load it from within connect function? The code might looks like:

const ConnectedComponent = connect(
  state => ({
    dataSet: state.dataSet
  }),
  dispatch => ({ 
    dispatch 
  }),
  (stateProps, dispatchProps) => {
    const { dataSet } = stateProps;
    const { dispatch } = dispatchProps;

    if (!dataSet) {
      // fetch data asynchronously and dispatch it to the store
    } 

    return {
      ...stateProps
    };
  }
)(MyComponent); 

class MyComponent extends Component {
  render () {
    const { dataSet } = this.props;
    if (dataSet) {
      // render grid with data
    } else {
      // render Loading...
    }
  }
}

The latter code looks more attractive to me because of MyComponent becomes simpler. There is no passing of execution first forward from connected component to presentational and then backward, when componentDidMount detects that there are no data ready to display.

Are there some drawbacks of such approach?

PS: I use redux-thunk for an asynchronous fetching.


Solution

  • The second approach, as separation of concepts, is potentially a good solution, because of the layers and responsibility separations - ConnectedComponent is responsible for data fetching, while MyComponent acts as presentational component. Good!

    But, dispatching actions in connect mergeProps doesn't seem a good idea, because you introduce side effects.

    Also, other drawback I'm seeing, is that the flow of fetching and returning data would be repeated across your different pages (components). Generally speaking the following flow would be repeated:

    1. Connected components call the API for the needed Entities.
    2. While fetching the Entities, we’re showing a Loader.
    3. When the data is available, we pass it to the Presentational components.

    Because of the above drawbacks, I can suggest you to organize and reuse your data fetching flow in a HOC.

    Here's a pseudo code and flow (taken from my article) that addresses the above drawbacks:

    * I've been using it last 1 year and continue stick with it.

    Fetcher HOC:

    import authorActions from 'actions/author'
    
    const actions = {
      'Author': authorActions
    }
    
    export default (WrappedComponent, entities) => {
    
      class Fetcher extends React.Component {
        // #1. Calls the API for the needed Entities.
        componentDidMount () {
          this.fetch()
        }
    
        fetch () {
          const { dispatch } = this.props
    
          entities.forEach(name => dispatch(actions[name].get()))
        }
    
        render () {
          const { isFetching } = this.props
    
          // #2. While fetching the Entities, we're showing an Loader.
          if (isFetching) return <Loader />
    
          return <WrappedComponent {...this.props} />
        }
      }
    
      const mapStateToProps = state => {
        const isFetching =  entities
          .map(entity => state[entity].isFetching)
          .filter(isFetching => isFetching)
    
        return { isFetching: isFetching.length > 0 }
      }
    
      return connect(mapStateToProps)(Fetcher)
    }
    

    Usage:

    const MyComponent = ({ authors }) => <AuthorsList authors={authors} />
    
    const mapStateToProps = state => ({
      authors: state.authors
    })
    
    const Component = connect(mapStateToProps)(MyComponent)
    export default Fetcher(Component, ['Author'])
    

    Here you can read the article and deep dive into its ideas and concepts:

    * Fetcher concept is addressed at Lesson #2: Containers on steroids

    Long-term React & Redux SPA — Lessons learned