React wrapper per la libreria esistente

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;

Ha scritto dei wrapper molto tempo fa. Mi chiedo se avrei potuto farlo in un modo migliore. Quali sono alcuni dei miglioramenti che potrei apportare o problemi con il codice. Grazie.

Answer 1

flvPlayerRef?

Nel costruttore, hai

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

Questo è abbastanza confuso. La proprietà è una funzione o un elemento, a seconda che sia stata chiamata come funzione prima e, in entrambi i casi, non è un riferimento, quindi è anche denominata erroneamente. Inoltre, non viene utilizzato in nessun altro punto del codice e gli utenti dell'istanza possono già ottenere un riferimento <video>all'elemento tramite la myRefproprietà.

Rimuoverei flvPlayerRefcompletamente e rinominare anche il nome della proprietà meno informativo myRefin videoRefo in flvPlayerRef.

A quel punto, puoi rendere le cose concise usando i campi di classe invece di un costruttore:

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

  componentDidMount() {
    // ...

Puoi anche considerare l'utilizzo di un componente funzionale invece di un componente basato sulla classe, come React consiglia provvisoriamente per il nuovo codice, ma non è necessario.

Oggetti di scena destrutturati

Questa riga è difficile da leggere:

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

Quando ci sono più di 2 o 3 proprietà da destrutturare, consiglierei di metterle su una riga separata

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

Ma, in questo caso, una frazione significativa delle proprietà viene utilizzata solo per essere trasferita in un flvjs.createPlayersecondo momento. Prendi in considerazione l'utilizzo della sintassi rest per raccogliere queste opzioni in un singolo oggetto, senza dover specificare singolarmente ciascuna di esse:

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

La enableErrorvariabile non viene utilizzata. Se è intenzionale, è meglio non estrarlo dagli oggetti di scena in primo luogo. O forse intendevi assegnarlo LoggingControl? Modificare

flvjs.LoggingControl.enableError = false;

per

flvjs.LoggingControl.enableError = enableError;

Più bello indentazione Piuttosto che creare un altro blocco di rientro dopo aver controllato se flvjs è supportato, è possibile prendere in considerazione di tornare presto se è non supportato:

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

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

Tornare presto è piuttosto carino, specialmente con una logica più complessa che altrimenti richiederebbe più livelli di rientro, che può diventare piuttosto difficile da leggere.

Spaziatura Ci sono alcuni posti in cui mi si aspetterebbe di vedere uno spazio, ma non vedo alcun dato lo stile del codice nel resto dello script, o dove vedo spazi in cui probabilmente non dovrebbe essere alcun, come const {type , url,, },{, (err)=>{, const { height, width,(vuoi spazio iniziale / finale durante la destrutturazione e con oggetti, o no?).

Qualunque cosa tu voglia che sia il tuo stile di codice, sarebbe bene essere coerente: considera l'utilizzo di ESLint per mantenere il tuo stile coerente, per risolvere le cose automaticamente e avvisarti di potenziali bug prima che si trasformino in errori di runtime.