Обертка React для существующей библиотеки

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;

Давно написал какую-то обертку. Мне интересно, мог бы я сделать это лучше. Какие улучшения я мог бы внести или какие проблемы с кодом? Благодарю.

Answer 1

flvPlayerRef?

В конструкторе у вас есть

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

Это довольно сбивает с толку. Свойство является либо функцией, либо элементом, в зависимости от того, вызывалась ли оно как функция раньше, и в любом случае это не ссылка, поэтому оно также неправильно названо. Он также не используется где-либо еще в коде, и потребители экземпляра уже могут получить ссылку на <video>элемент через myRefсвойство.

Я бы удалил flvPlayerRefполностью, а также переименовал myRefнеинформативное имя свойства в videoRefили в flvPlayerRef.

На этом этапе вы можете сделать вещи краткими, используя поля класса вместо конструктора:

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

  componentDidMount() {
    // ...

Вы также можете рассмотреть возможность использования функционального компонента вместо компонента на основе классов, как предварительно рекомендует React для нового кода, но это не обязательно.

Разрушенный реквизит

Эту строчку трудно прочитать:

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

Когда нужно деструктурировать более 2 или 3 свойств, я бы рекомендовал поместить каждое в отдельную строку.

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

Но в этом случае значительная часть свойств используется только для передачи flvjs.createPlayerпозже. Рассмотрите возможность использования синтаксиса rest, чтобы собрать эти параметры в один объект, без необходимости указывать каждый из них отдельно:

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

enableErrorПеременная не используется. Если это сделано намеренно, лучше вообще не извлекать его из реквизита. А может вы хотели его назначить LoggingControl? + Изменить

flvjs.LoggingControl.enableError = false;

к

flvjs.LoggingControl.enableError = enableError;

Более приятный отступ Вместо того, чтобы создавать еще один блок отступа после проверки, поддерживается ли flvjs, вы можете рассмотреть возможность возврата раньше, если он не поддерживается:

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

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

Ранний возврат - это довольно приятно, особенно с более сложной логикой, которая в противном случае потребовала бы нескольких уровней отступов, которые могут быть довольно трудными для чтения.

Разнос Есть несколько мест , где я ожидал увидеть пространство , но не вижу какой - либо данный стиль кода в остальной части скрипта, или там , где я вижу место , где , вероятно , не должно быть, как const {type , url,, },{, (err)=>{, const { height, width,(вам нужен начальный / конечный пробел при деструктуризации и с объектами, или нет?).

Каким бы вы ни был стиль кода, было бы хорошо быть последовательным - подумайте об использовании ESLint, чтобы сохранить согласованность вашего стиля, автоматически исправлять ошибки и предупреждать вас о потенциальных ошибках, прежде чем они превратятся в ошибки выполнения.