React wrapper untuk library yang sudah ada

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;

Menulis beberapa pembungkus lama sekali. Saya bertanya-tanya apakah saya bisa melakukannya dengan cara yang lebih baik. Apa saja perbaikan yang dapat saya lakukan atau masalah dengan kode. Terima kasih.

Answer 1

flvPlayerRef?

Dalam konstruktor, Anda punya

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

Ini cukup membingungkan. Properti bisa berupa fungsi atau elemen, bergantung pada apakah itu disebut sebagai fungsi sebelumnya, dan bagaimanapun, itu bukan ref, jadi itu juga salah namanya. Ini juga tidak digunakan di tempat lain dalam kode, dan konsumen instance sudah bisa mendapatkan referensi ke <video>elemen tersebut melalui myRefproperti.

Saya akan menghapus flvPlayerRefsepenuhnya, dan juga mengganti nama nama properti yang kurang informatif myRefmenjadi videoRefatau menjadi flvPlayerRef.

Pada titik itu, Anda bisa membuat semuanya ringkas dengan menggunakan bidang kelas alih-alih konstruktor:

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

  componentDidMount() {
    // ...

Anda juga dapat mempertimbangkan untuk menggunakan komponen fungsional daripada komponen berbasis kelas, seperti yang secara tentatif direkomendasikan oleh React untuk kode baru - tetapi itu tidak diperlukan.

Alat peraga rusak

Baris ini sulit dibaca:

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

Jika ada lebih dari 2 atau 3 properti yang akan dihancurkan, saya sarankan untuk meletakkan masing-masing pada baris terpisah

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

Namun, dalam kasus ini, sebagian besar properti digunakan hanya untuk diteruskan flvjs.createPlayernanti. Pertimbangkan untuk menggunakan sintaksis istirahat untuk mengumpulkan opsi tersebut menjadi satu objek, tanpa harus menentukan masing-masing satu per satu:

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

The enableErrorvariabel tidak digunakan. Jika itu disengaja, lebih baik tidak mengekstraknya dari alat peraga di tempat pertama. Atau mungkin Anda bermaksud untuk menugaskannya LoggingControl? Perubahan

flvjs.LoggingControl.enableError = false;

untuk

flvjs.LoggingControl.enableError = enableError;

Indentasi yang lebih bagus Daripada membuat blok indentasi lain setelah memeriksa apakah flvjs didukung, Anda dapat mempertimbangkan untuk kembali lebih awal jika tidak didukung:

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

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

Mengembalikan lebih awal cukup bagus, terutama dengan logika yang lebih kompleks yang seharusnya memerlukan beberapa level lekukan, yang bisa menjadi sangat sulit untuk dibaca.

Spasi Ada beberapa tempat di mana saya harapkan untuk melihat ruang tetapi tidak melihat mengingat gaya kode di sisa script, atau di mana saya melihat ruang di mana ada mungkin tidak boleh ada, seperti const {type , url,, },{, (err)=>{, const { height, width,(apakah Anda ingin spasi di depan / belakang saat merusak dan dengan objek, atau tidak?).

Apa pun yang Anda inginkan untuk gaya kode Anda, sebaiknya konsisten - pertimbangkan untuk menggunakan ESLint agar gaya Anda tetap konsisten, untuk memperbaiki berbagai hal secara otomatis, dan memperingatkan Anda tentang potensi bug sebelum berubah menjadi kesalahan waktu proses.