fix(musicKitHook, main): handle MusicKit instance re-attachment and SPA navigation#51
Merged
flexiondotorg merged 3 commits intomainfrom Mar 25, 2026
Merged
fix(musicKitHook, main): handle MusicKit instance re-attachment and SPA navigation#51flexiondotorg merged 3 commits intomainfrom
flexiondotorg merged 3 commits intomainfrom
Conversation
…ise volume state - Extract listener setup into attachToInstance function for re-use - Monitor for MusicKit instance replacement every 5 seconds and re-attach on change - Send initial volume state immediately to ensure MPRIS receives actual value - Convert polled volume updates to event-driven with polling fallback - Clear previous volume polling timer when re-hooking to prevent leaks Fixes race condition where MusicKit internally recreates instance, causing event pipeline to silently fail. Signed-off-by: Martin Wimpress <code@wimpress.io>
- Add hookScript parameter to setupNavigationHandlers function - Make did-navigate-in-page handler async and inject hookScript before navBarScript - Wrap both script injections in separate try-catch blocks to prevent failures from blocking each other - Improve reliability of hook attachment during single-page app navigation Signed-off-by: Martin Wimpress <code@wimpress.io>
Contributor
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="assets/musicKitHook.js">
<violation number="1" location="assets/musicKitHook.js:2">
P1: Guard against MusicKit being undefined before calling `MusicKit.getInstance()`; otherwise the hook can throw before the polling loop starts when MusicKit hasn’t loaded yet.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Prevent ReferenceError when hook executes before MusicKit loads by checking window.MusicKit exists before accessing MusicKit.getInstance(). Signed-off-by: Martin Wimpress <code@wimpress.io>
Contributor
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: This PR modifies core playback synchronization logic and refactors how the MusicKit instance is tracked, which is a critical path for the application's functionality.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MusicKit instance is replaced on sign out/in cycles, causing playback controls and status updates to fail. Additionally, SPA navigation requires re-injection of the navigation hook to maintain state synchronisation across page transitions.
Changes
attachToInstance()method for reusable instance attachment logicnewInstance !== currentInstance) for detection accuracyhookScripttosetupNavigationHandlersfor injection on in-page navigation eventsdid-navigate-in-pagewith error handling to maintain state across SPA transitionsTesting