Wrapper React pour la bibliothèque existante

Question

https://github.com/BingXiong1995/react-flv-player/blob/master/lib/wrapper/ReactFlvPlayer.js

import React, { Component } from 'react';
import flvjs from './flv.min';
import PropTypes from 'prop-types';


class ReactFlvPlayer extends Component {
  constructor(props) {
    super(props);
    this.myRef = React.createRef();
    this.flvPlayerRef = element => {
      this.flvPlayerRef = element;
    };
  }

  componentDidMount() {

    const {type , url, isLive, enableStashBuffer, stashInitialSize, hasAudio, hasVideo, handleError, enableWarning, enableError} = this.props;

    // 组件挂载后，拿到Ref进行操作
    if (flvjs.isSupported()) {
      const flvPlayer = flvjs.createPlayer({
        type,
        isLive,
        url,
        hasAudio,
        hasVideo
      },{
        enableStashBuffer,
        stashInitialSize
      });


      flvjs.LoggingControl.enableError = false;
      flvjs.LoggingControl.enableWarn = enableWarning;

      flvPlayer.attachMediaElement(this.myRef.current); // 将这个DOM付给第三方库
      flvPlayer.load();
      flvPlayer.play();
      flvPlayer.on('error', (err)=>{
        // console.log(err);
        handleError(err);
      });
    }
  }

  render() {
    const { height, width, isMuted,showControls } = this.props;
    return (
      <div>
        <video
          controls={showControls}
          muted={{isMuted}}
          ref={this.myRef}
          style={{height, width}}
        />
      </div>
    );
  }
}

ReactFlvPlayer.propTypes = {
  type: PropTypes.string,
  url: PropTypes.string.isRequired,
  isLive: PropTypes.bool,
  showControls: PropTypes.bool,
  hasAudio: PropTypes.bool,
  hasVideo: PropTypes.bool,
  enableStashBuffer: PropTypes.bool,
  stashInitialSize: PropTypes.number,
  height: PropTypes.string,
  width: PropTypes.string,
  isMuted: PropTypes.bool,
  enableWarning: PropTypes.bool,
  enableError: PropTypes.bool,
  handleError: PropTypes.func
};

ReactFlvPlayer.defaultProps = {
  type: 'flv',
  isLive: true,
  hasAudio: true,
  hasVideo: true,
  showControls: true,
  enableStashBuffer: true,
  stashInitialSize: 128,
  height: '100%',
  width: '100%',
  isMuted: false,
  handleError: (err)=>{console.log(err)},
  enableWarning: false,
  enableError: false
};

export default ReactFlvPlayer;

A écrit un wrapper il y a longtemps. Je me demande si j'aurais pu le faire d'une meilleure façon. Quelles sont certaines des améliorations que je pourrais apporter ou des problèmes avec le code. Merci.

Answer 1

flvPlayerRef?

Dans le constructeur, vous avez

this.myRef = React.createRef();
this.flvPlayerRef = element => {
  this.flvPlayerRef = element;
};

C'est assez déroutant. La propriété est soit une fonction, soit un élément, selon qu'elle a été appelée auparavant en tant que fonction, et de toute façon, ce n'est pas une référence, donc elle est également mal nommée. Il n'est également utilisé nulle part ailleurs dans le code, et les consommateurs de l'instance peuvent déjà obtenir une référence à l' <video>élément via la myRefpropriété.

Je supprimerais flvPlayerRefcomplètement et renommerais également le myRefnom de la propriété moins informative en videoRefou en flvPlayerRef.

À ce stade, vous pouvez rendre les choses concises en utilisant des champs de classe au lieu d'un constructeur:

class ReactFlvPlayer extends Component {
  videoRef = React.createRef();

  componentDidMount() {
    // ...

Vous pouvez également envisager d'utiliser un composant fonctionnel au lieu d'un composant basé sur les classes, comme React le recommande provisoirement pour le nouveau code - mais ce n'est pas obligatoire.

Accessoires déstructurés

Cette ligne est difficile à lire:

const {type , url, isLive, enableStashBuffer, stashInitialSize, hasAudio, hasVideo, handleError, enableWarning, enableError} = this.props;

Quand il y a plus de 2 ou 3 propriétés à déstructurer, je recommande de mettre chacune sur une ligne distincte

const {
  type,
  url,
  isLive,
  // ...
} = this.props;

Mais, dans ce cas, une fraction significative des propriétés n'est utilisée que pour être transmise flvjs.createPlayerplus tard. Pensez à utiliser la syntaxe rest pour rassembler ces options dans un seul objet, sans avoir à les spécifier individuellement:

const {
  enableStashBuffer,
  stashInitialSize,
  handleError,
  enableWarning,
  enableError,
  ...createPlayerOptions
} = this.props;

La enableErrorvariable n'est pas utilisée. Si c'est délibéré, mieux vaut ne pas l'extraire des accessoires en premier lieu. Ou peut-être vouliez-vous l'attribuer LoggingControl? Changement

flvjs.LoggingControl.enableError = false;

à

flvjs.LoggingControl.enableError = enableError;

Indentation plus agréable Plutôt que de créer un autre bloc d'indentation après avoir vérifié si flvjs est pris en charge, vous pouvez envisager de revenir plus tôt s'il n'est pas pris en charge:

componentDidMount() {
  if (!flvjs.isSupported()) {
    return;
  }
  const {
    enableStashBuffer,
    stashInitialSize,
    handleError,
    enableWarning,
    enableError,
    ...createPlayerOptions
  } = this.props;

  const flvPlayer = flvjs.createPlayer(
    createPlayerOptions,
    {
      enableStashBuffer,
      stashInitialSize
    }
  );
  // etc

Revenir tôt est assez agréable, surtout avec une logique plus complexe qui nécessiterait autrement plusieurs niveaux d'indentation, ce qui peut devenir assez difficile à lire.

Espacement Il y a quelques endroits où j'attendre à voir un espace , mais tu ne vois pas donné le style de code dans le reste du script, ou lorsque je vois des espaces où il ne devrait probablement pas tout, comme const {type , url,, },{, (err)=>{, const { height, width,(voulez-vous un espace de début / de fin lors de la déstructuration et avec des objets, ou non?).

Quel que soit votre style de code, il serait bon d'être cohérent - pensez à utiliser ESLint pour garder votre style cohérent, pour corriger les choses automatiquement et vous avertir des bogues potentiels avant qu'ils ne se transforment en erreurs d'exécution.