Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugins initialise twice and break in react wrapper #3731

Open
jonom opened this issue Jun 4, 2024 · 7 comments
Open

Plugins initialise twice and break in react wrapper #3731

jonom opened this issue Jun 4, 2024 · 7 comments

Comments

@jonom
Copy link
Contributor

jonom commented Jun 4, 2024

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

  • Browser: Chrome

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 jonom added the bug label Jun 4, 2024
@jonom jonom changed the title Plugins initialise twice and break in react library Plugins initialise twice and break in react wrapper Jun 4, 2024
@jonom
Copy link
Contributor Author

jonom commented Jun 4, 2024

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
Copy link

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
Copy link
Owner

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
Copy link
Contributor Author

jonom commented Jun 11, 2024

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
Copy link

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

@katspaugh
Copy link
Owner

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.

@vlad-solomon
Copy link

@brianlaw033 Yup, I've come to the same conclusion as you and @jonom . It's React's Strict Mode that causes this. What I'm wondering is what do you mean by this?

The solution for me is to run destroy() on unmount

This solved your problem? Because I'm trying to do the same thing and it doesn't work. I don't want to disable strict mode because that catches plenty of bugs. Is there something I'm missing? Here's my code:

export default function Track({ index, audio }) {
    const video = useVideoStore((state) => state.video);
    const [wavesurfer, setWavesurfer] = useState(null);

    const onReady = (ws) => {
        setWavesurfer(ws);
    };

    useEffect(() => {
        return () => {
            if (wavesurfer) {
                wavesurfer.stop();
                wavesurfer.destroy();
            }
        };
    }, [video, wavesurfer]);

    return (
        <div className="h-10 border-b border-coal-800 last:border-0">
            <WavesurferPlayer
                interact={false}
                height={40}
                waveColor={waveColor}
                progressColor={progressColor}
                url={audio}
                autoplay={true}
                onReady={onReady}
            />
        </div>
    );
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants