React: How to wrap component with HOC? (access log

Question

I have a react (with redux & react-router) app that at various points needs to know whether or not a user is logged in based on the state of some data in the store and a bunch of other things tied to data in the store.

To achieve this, I've created a higher order component like so:

import React from 'react';
import {connect} from 'react-redux';

export function masterComponent(Component){

  class MasterComponent extends React.Component {

    constructor(props){
      super(props);
    }

    loggedIn(){
      return this.props.player.token? true : false
    }

    render(){
      return (
        <Component {...this.props} player={{loggedIn: this.loggedIn()}}/>
      );
    }
  }

  UserComponent.propTypes = {
    player: React.PropTypes.object
  };


  function mapStateToProps(state) {
    return state;
  }

  return connect(
    mapStateToProps
  )(MasterComponent);

}

My question is how is best to wrap this around my individual components? Currently I have implemented it so that each individual component is wrapped when passed to connect which works fine but feels a little inefficient. ie:

import {masterComponent} from '../HigherOrderComponents/MasterComponent';

export class ResultsPage extends React.Component {

  constructor(props){
.....
  }


  render(){
    return (
       .....
    );
  }
}



export default connect(
  mapStateToProps,
  mapDispatchToProps
)(masterComponent(ResultsPage));

Is there a better way to do this?

Answer 1

It really depends on your use case, I see 3 possible solutions. Code below is untested and semi-pseudo.

Your initial proposal, refactored

export function masterComponent(Component) {

  const MasterComponent = (props) => {
    const loggedIn = props.player.token? true : false;

    // initial solution would override player, so you need 
    // something like this
    const player = { 
      ...this.props.player,
      loggedIn,
    };

    return <Component {...this.props} player={player} />
  }

  // Important! Here is the crucial part of the refactoring, 
  // not the coding style changes. Map only the relevant 
  // state properties, otherwise the component would rerender 
  // whenever any state property changed. 
  return connect(state => ({ player: this.state.player }))(MasterComponent);
}

Redux Middleware (my personal choice)

function loginMiddleware() {
  return store => next => action => {
    if (action.type === 'ADD_CREDENTIALS') {
      const { player } = store.getState();
      loginService.setPlayer(player);
    }

    // removePlayer if action.type === 'LOGOUT';

    return next(action);
  }
}

and in any component you could use

loginService.getPlayer() 

to verify if somebody has logged in or not

Disadvantage compared to your approach: If action 'ADD_CREDENTIALS' has been dispatched, but the component that requests loginService.getPlayer() doesn't get rerendered / updated, you might end up with inconsistent state. I'm sure you could solve that with more specific knowledge of your application.

Context (my least favourite)

Use React context and pass the desired property to all other components.