Skip to content

fix: allow message in .destroy#4247

Open
CarrettaRiccardo wants to merge 2 commits intokatspaugh:mainfrom
CarrettaRiccardo:fix-abort-message
Open

fix: allow message in .destroy#4247
CarrettaRiccardo wants to merge 2 commits intokatspaugh:mainfrom
CarrettaRiccardo:fix-abort-message

Conversation

@CarrettaRiccardo
Copy link
Copy Markdown

Currently, aborting throws an error in console because no message was passed to abort, which is inside .destroy method (https://developer.mozilla.org/en-US/docs/Web/API/AbortController/abort#parameters)

Implementation details

Added optional message prop to .destroy() method

In src/wavesurfer.ts

/** Unmount wavesurfer */
 public destroy(message?: string) {
   this.emit('destroy')
   this.abortController?.abort(message)
   this.plugins.forEach((plugin) => plugin.destroy())
   this.subscriptions.forEach((unsubscribe) => unsubscribe())
   this.unsubscribePlayerEvents()
   this.timer.destroy()
   this.renderer.destroy()
   super.destroy()
 }

Currently, aborting throws an error in console because no message was passed to abort, which is inside .destroy method (https://developer.mozilla.org/en-US/docs/Web/API/AbortController/abort#parameters)
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enhances the destroy() method by adding an optional message parameter that gets passed to AbortController.abort(). This allows callers to provide a custom abort reason when destroying the WaveSurfer instance.

Key changes:

  • Added optional message?: string parameter to the destroy() method signature
  • The message is now forwarded to abortController?.abort(message)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/wavesurfer.ts Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@katspaugh
Copy link
Copy Markdown
Owner

Good catch! Do we need to pass a message or can we just hard-code something like "Wavesurfer instance was destroyed"?

@CarrettaRiccardo
Copy link
Copy Markdown
Author

Good catch! Do we need to pass a message or can we just hard-code something like "Wavesurfer instance was destroyed"?

Well, i think hardcoding might be okay too! Just want that console error to not print ;)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants