fix: youTube player state synchronization issues (onReady, mute) #2
Conversation
WalkthroughThe changes introduce a new Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant YoutubePlayer
participant Iframe/Web
participant YouTubeAPI
App->>YoutubePlayer: Mounts component, passes onReady handler
YoutubePlayer->>Iframe/Web: Loads YouTube player
Iframe/Web->>YouTubeAPI: Initializes player
YouTubeAPI-->>Iframe/Web: Player ready with state
Iframe/Web-->>YoutubePlayer: Sends 'ready' message with playerInfo
YoutubePlayer-->>App: Calls onReady(playerInfo)
App->>App: Updates state with playerInfo
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Deploying react-native-youtube-bridge with
|
| Latest commit: |
d8909d9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ff292667.react-native-youtube-bridge.pages.dev |
| Branch Preview URL: | https://fix-youtube-sync-issues.react-native-youtube-bridge.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/types/youtube.ts (1)
56-70:PlayerInfocould be stricter & readonlyAll fields are optional; in practice
duration,availablePlaybackRates, etc. are always available afteronReady.
Mark the truly-always fields as required (or wrap the type inReadonly<…>). This tightens type-safety and avoids a sea ofundefinedchecks downstream.-export type PlayerInfo = { +export type PlayerInfo = Readonly<{ availablePlaybackRates: number[]; duration: number; // … -}; +}>; // remaining keys stay optional as neededsrc/YoutubePlayer.tsx (1)
54-68: PassplayerInfodirectly & guard for undefinedCopy-pasting every field is repetitive and couples this block to future
PlayerInfochanges. A simple null-check plus direct pass keeps the handler resilient.- const { playerInfo } = data; - - setIsReady(true); - onReady?.({ - availablePlaybackRates: playerInfo.availablePlaybackRates, - availableQualityLevels: playerInfo.availableQualityLevels, - currentTime: playerInfo.currentTime, - duration: playerInfo.duration, - muted: playerInfo.muted, - playbackQuality: playerInfo.playbackQuality, - playbackRate: playerInfo.playbackRate, - playerState: playerInfo.playerState, - size: playerInfo.size, - volume: playerInfo.volume, - }); + const { playerInfo } = data; + + setIsReady(true); + if (playerInfo) { + onReady?.(playerInfo); + }example/src/App.tsx (3)
30-30:asynckeyword is unnecessary
handleReadyperforms noawait; dropasyncto avoid misleading callers.
110-113: Volume state can fall out-of-sync
setVolumeis invoked optimistically. If the underlying player rejects the value, UI will still display the new percentage. Consider awaiting the promise and updating state only on success.
126-133: RenameonPlay→togglePlayfor clarityThe handler toggles between play / pause; naming it explicitly communicates intent and avoids the momentary mental jump. Purely semantic but improves readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
example/src/App.tsx(6 hunks)src/YoutubePlayer.tsx(1 hunks)src/YoutubePlayer.web.tsx(1 hunks)src/hooks/youtubeIframeScripts.ts(1 hunks)src/index.tsx(1 hunks)src/types/iframe.d.ts(3 hunks)src/types/message.ts(2 hunks)src/types/youtube.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/types/message.ts (1)
src/types/youtube.ts (1)
PlayerInfo(56-70)
example/src/App.tsx (2)
src/index.tsx (1)
PlayerInfo(11-11)src/types/youtube.ts (1)
PlayerInfo(56-70)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (4)
src/types/youtube.ts (1)
22-22: Breaking-API change – bump minor/major version & update docs
onReadynow deliversPlayerInfo; all downstream callbacks/tests must be adapted.
Remember to list this in CHANGELOG and bump the public version accordingly.src/index.tsx (1)
11-12: Re-export LGTMRe-exporting
PlayerInfoaligns the public surface with the new type. 👍src/types/message.ts (1)
15-16: Message shape updated – ensure native/web consumers are in sync
playerInforeplaces separate fields. Verify all message handlers (RN WebView, web build) parse the new structure, otherwise they will silently ignore readiness data.src/types/iframe.d.ts (1)
59-65:onReady’sdatais alwaysnull— confirm intentYou changed the higher-level React wrapper to pass a
PlayerInfoobject, but herePlayerEvent<null>keeps the payload empty. If you eventually need the same info at the raw iframe-API layer, considerPlayerEvent<PlayerInfo | null>to stay consistent.
Otherwise, clarify with a comment that the details are exposed only viatarget.playerInfo.
Summary by CodeRabbit
PlayerInfotype for more comprehensive player event data.