katspaugh / wavesurfer.js

Audio waveform player
https://wavesurfer.xyz
BSD 3-Clause "New" or "Revised" License
8.85k stars 1.64k forks source link

Plugins initialise twice and break in react wrapper #3731

Open jonom opened 6 months ago

jonom commented 6 months ago

Bug description

TLDR; I can't get the spectrogram plugin to work on the react wrapper. I haven't tested other types of plugins.

Note: I think this is an issue with https://github.com/katspaugh/wavesurfer-react rather than this repo, but there is no issues tab on the react repo so I'm posting here. Also, it might be able to be fixed in this repo by making plugins reusable - i.e., able to be re-initialised.

The issue seems to be that the plugin instance is mutated during initialisation, and if you attempt to initialise the plugin a second time it will cause an error. The problem seems to be in the useEffect hook which destroys and recreates the wavesurfer instance. In subsequent runs of the hook a new ws instance would get created but it would receive already-initialised plugin instances.

I found that if I memoised all of the props being passed to WavesurferPlayer I could get the plugin to work when building for production at least, but when running locally it still breaks since React throws in that 'helpful' extra render.

Environment

Minimal code snippet

import { useCallback, useRef, useState } from "react";
import WavesurferPlayer from "@wavesurfer/react";
import Spectrogram from "wavesurfer.js/dist/plugins/spectrogram.esm.js";
import createColormap from "colormap";

function AudioPlayer({ audioUrl }) {
  const [wavesurfer, setWavesurfer] = useState(null);
  const [isPlaying, setIsPlaying] = useState(false);
  const plugins = useRef(null);
  if (plugins.current === null) {
    plugins.current = [
      Spectrogram.create({
        labels: true,
        height: 200,
        splitChannels: false,
        colorMap: createColormap({
          colormap: "jet",
          nshades: 256,
          format: "float",
        }),
      }),
    ];
  }

  const onReady = useCallback((ws) => {
    setWavesurfer(ws);
    setIsPlaying(false);
  }, []);

  const onPlayPause = useCallback(() => {
    wavesurfer && wavesurfer.playPause();
  }, [wavesurfer]);

  const onPlay = useCallback(() => {
    setIsPlaying(true);
  }, []);

  const onPause = useCallback(() => {
    setIsPlaying(false);
  }, []);

  return plugins.current ? (
    <>
      <WavesurferPlayer
        height={100}
        waveColor="violet"
        url={audioUrl}
        onReady={onReady}
        onPlay={onPlay}
        onPause={onPause}
        plugins={plugins.current}
      />

      <button onClick={onPlayPause}>{isPlaying ? "Pause" : "Play"}</button>
    </>
  ) : null;
}

export default AudioPlayer;

Expected result

A player with a spectrogram.

Obtained result

TypeError: Failed to execute 'appendChild' on 'Node': parameter 1 is not of type 'Node'.

jonom commented 6 months ago

FWIW I also tried adding the plugin in onInit and onReady but it didn't work - I got a regular player without the plugin rendered.

  const onReady = useCallback((ws) => {
    setWavesurfer(ws);
    setIsPlaying(false);
    ws.registerPlugin(
      Spectrogram.create({
        labels: true,
        height: 200,
        splitChannels: false,
        colorMap: createColormap({
          colormap: "jet",
          nshades: 256,
          format: "float",
        }),
      })
    );
  }, []);
brianlaw033 commented 6 months ago

I came across this issue too, and looks like its react's strict mode that leads to useEffect being ran twice The solution for me is to run destroy() on unmount

katspaugh commented 6 months ago

Good catch! Looks like this ref usage leads to different lifecycles between a wavesurfer instance and a plugin instance.

As a workaround, try using the useWavesurfer hook instead of the component. See an example here: https://wavesurfer.xyz/examples/?react.js

jonom commented 5 months ago

Thanks for your replies! Back on this project today and I tried using the useWavesurfer hook but it has the same result because the component uses those hooks under the hood. Note that I tried with the Timeline plugin instead of Spectrogram and that worked fine, so I guess this issue is plugin-specific.

My solution for now is to modify useWavesurferInstance() to accept a getPlugins function:

    const ws = WaveSurfer.create({
      ...options,
      container: containerRef.current,
      plugins: options.getPlugins ? options.getPlugins() : options.plugins,
    });
  const wavesurferOptions = useMemo(
    () => ({
      container: containerRef,
      url: audioUrl,
      getPlugins: () => [
        Spectrogram.create(),
      ],
    }),
    [audioUrl]
  );

  const { wavesurfer, isPlaying, currentTime } =
    useWavesurfer(wavesurferOptions);

This way a fresh plugin instance gets created each time the player initialises.

robotastic commented 6 days ago

@jonom - thanks for that snippet, it got things working for me.

katspaugh commented 6 days ago

Thanks for your replies! Back on this project today and I tried using the useWavesurfer hook but it has the same result

I've just re-read your initial issue description and realized you already narrowed it down to the extra render React does in dev mode, so both the hook and the component have the same problem.

I guess one workaround would be to init/destroy plugins in onInit and onDestroy but maybe there's some clever way to handle this in the lib itself.