Make YTM proxy optional in synced-lyrics-plugin#4448
Conversation
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds a configuration option to the Synced Lyrics plugin enabling direct YouTube Music API calls via app authentication instead of routing through an external proxy. The feature includes a new boolean config field, menu checkbox toggle, conditional fetch logic in the provider, and localized UI strings. ChangesYTM Direct Browse Fetch Feature
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/plugins/synced-lyrics/providers/YTMusic.ts`:
- Around line 146-156: The method fetchBrowseDirect currently throws if the
'ytmusic-app' element is missing; change it to mirror fetchNext by returning
null when document.querySelector<MusicPlayerAppElement>('ytmusic-app') is not
found and only call app.networkManager.fetch when app exists. Then update
search() to guard the awaited result from fetchBrowseDirect (check for null
before destructuring the BrowseData) so that when useYTMLyricsWithoutProxy is
enabled and the element is absent the provider returns null instead of throwing;
reference fetchBrowseDirect, fetchNext and search when making these changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b7236153-2dad-48ab-9917-3bc14f4a74d4
📒 Files selected for processing (5)
src/i18n/resources/en.jsonsrc/plugins/synced-lyrics/index.tssrc/plugins/synced-lyrics/menu.tssrc/plugins/synced-lyrics/providers/YTMusic.tssrc/plugins/synced-lyrics/types.ts
For the jurisdictions the YTM proxy is not needed there's now an option to go directly to the "/browse" API skipping the proxy. Also helps a recent downtime on the proxy.
Summary by CodeRabbit