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.

Sign up for GitHub

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

Jump to bottom

No onReady event firing in React when using pre-computed peaks #3744

Closed
njbair opened this issue Jun 14, 2024 · 5 comments · Fixed by #3762
Closed

No onReady event firing in React when using pre-computed peaks #3744

njbair opened this issue Jun 14, 2024 · 5 comments · Fixed by #3762
Labels
bug

Comments

@njbair
Copy link

njbair commented Jun 14, 2024

Bug description

When using @wavesurfer/react with pre-computed peaks, the onReady event is never triggered, so there's presumably no occasion to bind the wavesurfer object to state, and thus no opportunity to initialize other controls.

Environment

  • Browser: Chrome

Minimal code snippet

Clone this demo repo https://github.com/njbair/wavesurfer-react.git then run npm start and browse to localhost:3000. Open dev tools to monitor the JS console.

Expected result

We should get a ready event when the file loads.

Obtained result

  • We do not get a ready event when the file loads.
  • In the version without peaks, the ready event is triggered.
  • In the Vanilla JS versions, both with and without peaks, the ready event is triggered.
  • It's not included in this demo repo, but I first encountered this bug while using the useWavesurfer hook instead of the WavesurferPlayer component. This makes sense to me, since the component is just a basic wrapper around the hook.

Screenshots

Here is a screenshot of the console only displaying redraw and redrawcomplete events:

screenshot
@njbair njbair added the bug label Jun 14, 2024
@njbair
Copy link
Author

njbair commented Jun 15, 2024

Additional info: my suspicion is that this is a race condition between mounting the component and loading the audio. Since pre-calculated audio loads basically instantly, it looks like the wavesurfer library is emitting the ready event before the React component subscribes to it.

I'm testing out a local copy of both @wavesurfer/react and wavesurfer.js, adding some console output to confirm my suspicions, and it does indeed look like that's what's happening here:

Screenshot 2024-06-14 at 8 51 26 PM
@katspaugh
Copy link
Owner

I believe this was fixed in wavesurfer.js, the react package just needs to be updated.
@njbair
Copy link
Author

njbair commented Jun 19, 2024

I believe this was fixed in wavesurfer.js, the react package just needs to be updated.

The React package seems to do very little other than pulling in the wavesurfer.js core library and then React-ifying it.

I wonder if there's a way to wrap the wavesurfer ready event in a promise, then have that promise handled by useEffect? You would also need to defer loading of the URL until after that event is fired.

Looking at the wavesurfer.js API, I don't see a good place for useEffect to hook into. If you have any suggestions, I would be happy to take a stab at a PR to address this.

Thanks.
@katspaugh
Copy link
Owner

AFAIR this event is already promisified, even with predecoded peaks. Probably use effect is still being triggered too late.
@njbair njbair mentioned this issue Jun 21, 2024
Open
@katspaugh katspaugh mentioned this issue Jun 23, 2024
Merged
@katspaugh
Copy link
Owner

@njbair a fix is now live in 7.8.1. Cheers!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug
2 participants
@katspaugh @njbair