YTMD/youtube-music
1
0
Fork 0
Code Issues 445 Pull requests 34 Projects Releases 87 Packages Wiki Activity Actions

Fixed missing videochange dataupdated event when using shuffle #3659

Merged
rewhex merged 5 commits from fix/missing-dataupdated-event into master 2025-09-07 04:43:48 +00:00
Conversation 8 Commits 5 Files changed 1 +26
rewhex commented 2025-07-21 15:14:27 +00:00 (Migrated from github.com)
Copy link

Fix issue where videochange-dataupdated event doesn't fire after shuffle

After triggering shuffle via the API, the videochange: dataupdated event sometimes fails to fire. To ensure the song info is still updated, a fallback using a timeout has been implemented.

Fix issue where videochange-dataupdated event doesn't fire after shuffle After triggering shuffle via the API, the videochange: dataupdated event sometimes fails to fire. To ensure the song info is still updated, a fallback using a timeout has been implemented.
copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed 2025-07-22 13:21:45 +00:00
copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment
Copy link

Pull Request Overview

This PR fixes an issue where the videochange-dataupdated event doesn't fire after shuffle operations, which could leave song info in an outdated state. The solution implements a fallback mechanism using timeouts to ensure song information is updated even when the expected event doesn't fire.

  • Adds timeout-based fallback logic to handle missing dataupdated events
  • Implements proper timeout cleanup to prevent memory leaks
  • Adds window unload event handler for cleanup
## Pull Request Overview This PR fixes an issue where the `videochange-dataupdated` event doesn't fire after shuffle operations, which could leave song info in an outdated state. The solution implements a fallback mechanism using timeouts to ensure song information is updated even when the expected event doesn't fire. - Adds timeout-based fallback logic to handle missing `dataupdated` events - Implements proper timeout cleanup to prevent memory leaks - Adds window unload event handler for cleanup
src/providers/song-info-front.ts
@ -222,7 +237,18 @@ export default (api: YoutubePlayer) => {
video?.addEventListener(status, playPausedHandlers[status]);
}
clearVideoTimeout(videoData.videoId);
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-07-22 13:21:44 +00:00
Copy link

The timeout value of 1500ms is a magic number. Consider extracting this to a named constant with a descriptive name like DATAUPDATED_FALLBACK_TIMEOUT_MS to improve code maintainability and make it easier to adjust if needed.

      }, DATAUPDATED_FALLBACK_TIMEOUT_MS);
The timeout value of 1500ms is a magic number. Consider extracting this to a named constant with a descriptive name like `DATAUPDATED_FALLBACK_TIMEOUT_MS` to improve code maintainability and make it easier to adjust if needed. ```suggestion }, DATAUPDATED_FALLBACK_TIMEOUT_MS); ```
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-07-22 13:21:45 +00:00
Copy link

The event listener is added but never removed. Consider storing a reference to enable cleanup, or use the { once: true } option if this is intended to be a one-time listener. This prevents potential memory leaks if this code runs multiple times.

  window.addEventListener('beforeunload', clearAllTimeouts, { once: true });
The event listener is added but never removed. Consider storing a reference to enable cleanup, or use the `{ once: true }` option if this is intended to be a one-time listener. This prevents potential memory leaks if this code runs multiple times. ```suggestion window.addEventListener('beforeunload', clearAllTimeouts, { once: true }); ```
JellyBrick (Migrated from github.com) reviewed 2025-07-28 14:11:35 +00:00
src/providers/song-info-front.ts
JellyBrick (Migrated from github.com) commented 2025-07-28 14:11:25 +00:00
Copy link

If dataupdated is not fired, videoData may be incompletely loaded. In this case, the behavior of some plugins may be broken.

If `dataupdated` is not fired, `videoData` may be incompletely loaded. In this case, the behavior of some plugins may be broken.
rewhex (Migrated from github.com) reviewed 2025-07-28 14:36:23 +00:00
src/providers/song-info-front.ts
rewhex (Migrated from github.com) commented 2025-07-28 14:36:23 +00:00
Copy link

Yet this is the best option, because without the fallback to first event the data isn't updated at all. Even if I press next song

Yet this is the best option, because without the fallback to first event the data isn't updated at all. Even if I press next song
rewhex (Migrated from github.com) reviewed 2025-07-28 15:16:41 +00:00
src/providers/song-info-front.ts
rewhex (Migrated from github.com) commented 2025-07-28 15:16:41 +00:00
Copy link

Seems like in the case when dataupdated isn't fired, dataloaded has all data just like the dataupdated, but someone else need to test and confirm it

Seems like in the case when `dataupdated` isn't fired, `dataloaded` has all data just like the `dataupdated`, but someone else need to test and confirm it
arjix reviewed 2025-07-29 19:44:35 +00:00
src/providers/song-info-front.ts
@ -224,2 +239,4 @@
clearVideoTimeout(videoData.videoId);
waitingEvent.add(videoData.videoId);
arjix commented 2025-07-29 19:44:34 +00:00
Owner
Copy link

This seems redundant, doesn't the window automatically destroy all related resources when it unloads?

This seems redundant, doesn't the window automatically destroy all related resources when it unloads?
arjix reviewed 2025-07-29 19:45:47 +00:00
src/providers/song-info-front.ts
@ -224,2 +239,4 @@
clearVideoTimeout(videoData.videoId);
waitingEvent.add(videoData.videoId);
arjix commented 2025-07-29 19:45:47 +00:00
Owner
Copy link

Or did I misunderstand this, and the event gets fired even when the window does not unload?

Or did I misunderstand this, and the event gets fired even when the window does not unload?
rewhex (Migrated from github.com) reviewed 2025-07-29 20:18:16 +00:00
src/providers/song-info-front.ts
@ -224,2 +239,4 @@
clearVideoTimeout(videoData.videoId);
waitingEvent.add(videoData.videoId);
rewhex (Migrated from github.com) commented 2025-07-29 20:18:16 +00:00
Copy link

Hmm, yeah, this one is redundant

Hmm, yeah, this one is redundant
JellyBrick (Migrated from github.com) approved these changes 2025-09-07 04:43:39 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
JellyBrick
Labels
Clear labels   
awaiting-reply

Awaiting reply

  
breaking changes

   
bug

Something isn't working

  
cannot-reproduce

Bug cannot be reproduced

  
dependencies

Pull requests that update a dependency file

  
documentation

   
duplicate

This issue or pull request already exists

  
electron-issue

It's an Electron or Electron related dependencies issue (not YTM-Desktop issue)

  
enhancement

New feature or request

  
fix-available

A fix to the issue is available in a new version

  
good first issue

Good for newcomers

  
help wanted

Extra attention is needed

  
invalid

This doesn't seem right

  
javascript

Pull requests that update javascript code

  
need more information

Need more information about the issue

  
need rebase

This plugin need rebase

  
official-youtube-music-issue

It's an YouTube's issue (not YTM-Desktop issue)

  
plugin request

An issue/discussion requesting for a new plugin or new functionality to an existing plugin.

  
question

Further information is requested

  
release

Release

  
security

   
stale

   
Status: blocked

There are something to do before this

  
typo

   
wontfix

This will not be worked on

  
ytmd-issue

An issue/discussion about ytm desktop, unrelated to th-ch/youtube-music

No labels
awaiting-reply
breaking changes
bug
cannot-reproduce
dependencies
documentation
duplicate
electron-issue
enhancement
fix-available
good first issue
help wanted
invalid
javascript
need more information
need rebase
official-youtube-music-issue
plugin request
question
release
security
stale
Status: blocked
typo
wontfix
ytmd-issue
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: YTMD/youtube-music#3659
No description provided.
Delete branch "fix/missing-dataupdated-event"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?