2

I'm working on a HTMl5 video player for a French company. We use React and Redux to build the UI, and it works very well, it's very pleasant to code ! We currently use eslint-plugin-react to check React code style. Since last versions, the linter recommands to use pure functions instead of React Components (view the rule) but it raises some debates in my team.

We are already using pure function for very small components that render always the same things (for given props). No problems with that. But for bigger components, in my opinion, pure functions seem to make the code less elegant (and less uniform compared to other components).

This is an example of one of our components that should be changed :

const ControlBar = ({ actions, core, root }) => {
  const onFullscreenScreen = (isFullscreen) => {
    const playerRoot = root;
    if (isFullscreen && !screenfull.isFullscreen) {
      screenfull.request(playerRoot);
    } else {
      screenfull.exit();
    }
  };
​
  const renderIconButton = (glyph, action, label = false) => {
    let content = (
      <Button modifier="icon" clickCallback={ action }>
        <Icon glyph={ glyph } />
      </Button>
    );
    if (label) {
      content = <TooltipOrigin content={label}>{content}</TooltipOrigin>;
    }
    return content;
  };
​
  const renderPlayButton = () => {
    const { play, pause } = actions;
    const { playerState } = core;
    if (playerState === CoreStates.PAUSED) {
      return renderIconButton(playGlyph, play, 'lecture');
    }
    return renderIconButton(pauseGlyph, pause, 'pause');
  };
​
  const renderMuteButton = () => {
    const { mute, unmute } = actions;
    const { currentVolume } = core;
    if (currentVolume === 0) {
      return renderIconButton(muteGlyph, unmute);
    }
    return renderIconButton(volumeGlyph, mute);
  };
​
  const renderFullscreenButton = () => {
    const { isFullscreen } = core;
    if (!isFullscreen) {
      return renderIconButton(fullscreenGlyph, () => { onFullscreenScreen(true); });
    }
    return renderIconButton(fullscreenExitGlyph, () => { onFullscreenScreen(false); });
  };
​
  const { setCurrentVolume } = actions;
  const { currentVolume } = core;
  return (
    <div className={ style.ControlBar }>
      <div className={ style.audio }>
        { renderMuteButton() }
        <SoundBar setCurrentVolume={ setCurrentVolume } volume={ currentVolume } />
      </div>
      <div className={ style.controls }>
        { renderPlayButton() }
      </div>
      <div className={ style.settings }>
        { renderFullscreenButton() }
      </div>
    </div>
  );
};
​
ControlBar.propTypes = {
  actions: PropTypes.object.isRequired,
  core: PropTypes.object.isRequired,
  root: PropTypes.object.isRequired,
};
​
export default ControlBar;

versus :

export default class ControlBar extends Component {

  static propTypes = {
    actions: PropTypes.object.isRequired,
    core: PropTypes.object.isRequired,
    root: PropTypes.object.isRequired,
  };

  onFullscreenScreen(isFullscreen) {
    const playerRoot = this.props.root;

    if (isFullscreen && !screenfull.isFullscreen) {
      screenfull.request(playerRoot);
    } else {
      screenfull.exit();
    }
  }

  renderIconButton(glyph, action, label = false) {
    let content = (
      <Button modifier="icon" clickCallback={ action }>
        <Icon glyph={ glyph } />
      </Button>
    );
    if (label) {
      content = <TooltipOrigin content={label}>{content}</TooltipOrigin>;
    }
    return content;
  }

  renderPlayButton() {
    const { play, pause } = this.props.actions;
    const { playerState } = this.props.core;
    if (playerState === CoreStates.PAUSED) {
      return this.renderIconButton(playGlyph, play, 'lecture');
    }
    return this.renderIconButton(pauseGlyph, pause, 'pause');
  }

  renderMuteButton() {
    const { mute, unmute } = this.props.actions;
    const { currentVolume } = this.props.core;
    if (currentVolume === 0) {
      return this.renderIconButton(muteGlyph, unmute);
    }
    return this.renderIconButton(volumeGlyph, mute);
  }

  renderFullscreenButton() {
    const { isFullscreen } = this.props.core;
    if (!isFullscreen) {
      return this.renderIconButton(fullscreenGlyph, () => { this.onFullscreenScreen(true); });
    }
    return this.renderIconButton(fullscreenExitGlyph, () => { this.onFullscreenScreen(false); });
  }

  render() {
    const { setCurrentVolume } = this.props.actions;
    const { currentVolume } = this.props.core;
    return (
      <div className={ style.ControlBar }>
        <div className={ style.audio }>
          { this.renderMuteButton() }
          <SoundBar setCurrentVolume={ setCurrentVolume } volume={ currentVolume } />
        </div>
        <div className={ style.controls }>
          { this.renderPlayButton() }
        </div>
        <div className={ style.settings }>
          { this.renderFullscreenButton() }
        </div>
      </div>
    );
  }

}

We like the structure of a React Component. The PropTypes and default props can be inside the class thanks to ES7, it's not possible with pure functions. And, in this example particularly, we have many function to render sub-components.

We could simply disable this rule if we don't like, but we really want to understand that and we care about performance and React good practices. So, maybe you can help us.

How can you help me ? I would get other opinions about this interesting problematic. What are the arguments in favor of pure functions ? Maybe the solution is not to change ControlBar Component in pure function but just to improve it. In this case, what would be your advices to do that ?

Thanks a lot for your help !

Clarkie
  • 7,490
  • 9
  • 39
  • 53
  • 1
    This is an interesting discussion topic but probably isn't suited to the SO platform: http://stackoverflow.com/help/dont-ask – Clarkie Mar 14 '16 at 14:06
  • I understand. If someone wants to answer (or know the answer), I re-posted here : https://discuss.reactjs.org/t/prefer-pure-function-than-react-component/3459 – LudovicMnji Mar 16 '16 at 09:14

0 Answers0