-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add flexible source prop to support videoId and URL #24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Support string (videoId or URL) and object formats - Add useYouTubeVideoId hook for source parsing - Maintain backward compatibility with videoId extraction
🦋 Changeset detectedLatest commit: 60f0f4a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis change replaces the Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant YoutubePlayer
participant useYouTubeVideoId
participant validate/extract Utils
App->>YoutubePlayer: Passes source prop (string/object)
YoutubePlayer->>useYouTubeVideoId: Calls with source
useYouTubeVideoId->>validate/extract Utils: Validate or extract videoId
validate/extract Utils-->>useYouTubeVideoId: Returns videoId or undefined
useYouTubeVideoId-->>YoutubePlayer: Returns videoId (string or empty)
YoutubePlayer-->>App: Renders player with derived videoId
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: |
60f0f4a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ad51bffc.react-native-youtube-bridge.pages.dev |
| Branch Preview URL: | https://feat-flexible-source-prop.react-native-youtube-bridge.pages.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/YoutubePlayer.tsx (1)
44-44: Consistent source parsing implementation.The
useYouTubeVideoIdhook usage matches the web implementation pattern, ensuring consistent behavior across platforms. The extractedvideoIdcorrectly integrates with the existing validation and player logic.
🧹 Nitpick comments (5)
example/src/App.tsx (1)
177-177: Correct API migration maintaining backward compatibility.The change from
videoIdtosourceprop correctly demonstrates the API update. SincevideoIdis a string, it's fully compatible with the newYouTubeSourceunion type.Consider enhancing the example to showcase the new flexible source formats:
// Could add examples like: // source="https://www.youtube.com/watch?v=AbZH7XWDW_k" // source={{ videoId: "AbZH7XWDW_k" }} // source={{ url: "https://youtu.be/AbZH7XWDW_k" }}README.md (1)
43-47: Consider showing concrete source values in the basic usage example.The example uses
source={source}without defining what thesourcevariable contains. Consider showing concrete examples to make it clearer for developers:- <YoutubePlayer - source={source} // youtube videoId or url - // OR source={{ videoId: 'AbZH7XWDW_k' }} - // OR source={{ url: 'https://youtube.com/watch?v=AbZH7XWDW_k' }} - /> + <YoutubePlayer + source="AbZH7XWDW_k" // youtube videoId + // OR source={{ videoId: 'AbZH7XWDW_k' }} + // OR source={{ url: 'https://youtube.com/watch?v=AbZH7XWDW_k' }} + />src/hooks/useYoutubeVideoId.ts (2)
6-6: Provide explanation for the biome-ignore comment.The ignore comment should include a proper explanation of why exhaustive dependencies checking is disabled.
- // biome-ignore lint/correctness/useExhaustiveDependencies: <explanation> + // biome-ignore lint/correctness/useExhaustiveDependencies: Custom dependency array needed for source object comparison
21-23: Simplify the complex dependency array to improve maintainability.The current dependency array is complex and error-prone. Consider extracting the logic or using a more straightforward approach:
- ], [ - typeof source === 'string' ? source : 'videoId' in source ? source.videoId : 'url' in source ? source.url : null, - ]); + ], [source]);Since the
sourceobject reference would change when its properties change, using[source]should be sufficient and more readable.README-ko_kr.md (1)
43-47: Consider showing concrete source values in the Korean documentation as well.Similar to the English README, consider showing concrete examples instead of
source={source}:- <YoutubePlayer - source={source} // youtube videoId or url - // OR source={{ videoId: 'AbZH7XWDW_k' }} - // OR source={{ url: 'https://youtube.com/watch?v=AbZH7XWDW_k' }} - /> + <YoutubePlayer + source="AbZH7XWDW_k" // youtube videoId + // OR source={{ videoId: 'AbZH7XWDW_k' }} + // OR source={{ url: 'https://youtube.com/watch?v=AbZH7XWDW_k' }} + />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.changeset/three-papayas-design.md(1 hunks)README-ko_kr.md(5 hunks)README.md(5 hunks)example/src/App.tsx(1 hunks)src/YoutubePlayer.tsx(3 hunks)src/YoutubePlayer.web.tsx(1 hunks)src/hooks/useCreateLocalPlayerHtml.ts(0 hunks)src/hooks/useYoutubeVideoId.ts(1 hunks)src/modules/YouTubePlayerCore.tsx(1 hunks)src/types/youtube.ts(1 hunks)src/utils/validate.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/hooks/useCreateLocalPlayerHtml.ts
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/modules/YouTubePlayerCore.tsx (1)
src/types/youtube.ts (1)
YoutubePlayerProps(56-78)
src/YoutubePlayer.web.tsx (1)
src/types/youtube.ts (1)
YoutubePlayerProps(56-78)
src/hooks/useYoutubeVideoId.ts (2)
src/types/youtube.ts (3)
YouTubeSource(54-54)PlayerEvents(36-52)ERROR_CODES(107-118)src/utils/validate.ts (2)
validateVideoId(14-17)extractVideoIdFromUrl(4-12)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (15)
src/types/youtube.ts (2)
54-54: Well-designed flexible type definition.The
YouTubeSourceunion type provides excellent flexibility by supporting string video IDs, objects withvideoId, and objects withurl. This enables multiple input formats while maintaining type safety.
57-57: Clean API improvement with backward compatibility.Replacing
videoIdwithsourceinYoutubePlayerPropsprovides a more intuitive and flexible API. Since strings are supported in the union type, this maintains backward compatibility for existing string video ID usage.src/modules/YouTubePlayerCore.tsx (2)
2-2: Clean import simplification.Removing the
PlayerConfigalias import and defining it locally provides better separation of concerns.
5-7: Well-designed internal type that maintains clear boundaries.The local
PlayerConfigtype correctly excludes the flexiblesourceprop and requires a concretevideoIdstring. This maintains a clean separation where the core module works with validated video IDs while parent components handle source parsing and validation.src/YoutubePlayer.web.tsx (2)
5-5: Correct import for source parsing hook.Adding the
useYouTubeVideoIdimport enables flexible source format handling.
9-10: ```shell
#!/bin/bashPrint YouTubeSource, YoutubePlayerProps, and PlayerEvents definitions
rg -n "export type YouTubeSource" -A5 src/types/youtube.ts
rg -n "export type YoutubePlayerProps" -A5 src/types/youtube.ts
rg -n "export type PlayerEvents" -A5 src/types/youtube.ts</details> <details> <summary>src/YoutubePlayer.tsx (2)</summary> `6-6`: **Consistent import across platforms.** Adding the `useYouTubeVideoId` import maintains consistency with the web implementation. --- `16-16`: **Correct prop destructuring update.** Updating the destructuring from `videoId` to `source` aligns with the new API while maintaining all other props. </details> <details> <summary>README.md (1)</summary> `127-127`: **LGTM! Documentation consistently updated.** All examples throughout the documentation have been properly updated to use the new `source` prop instead of `videoId`. The changes are consistent and maintain clarity. Also applies to: 165-165, 185-185, 224-224 </details> <details> <summary>src/hooks/useYoutubeVideoId.ts (1)</summary> `7-23`: **LGTM! Source normalization logic is well-structured.** The source normalization logic correctly handles all three input formats (string, object with videoId, object with url) and provides appropriate fallback behavior. </details> <details> <summary>README-ko_kr.md (1)</summary> `127-127`: **LGTM! Korean documentation properly updated.** All examples in the Korean documentation have been consistently updated to use the new `source` prop, maintaining consistency with the English version. Also applies to: 165-165, 185-185, 224-224 </details> <details> <summary>.changeset/three-papayas-design.md (1)</summary> `1-12`: **LGTM! Well-documented breaking change.** The changeset properly documents this as a breaking change with clear information about: - The replacement of `videoId` prop with `source` prop - The flexible input formats supported - The new internal hook - Support for multiple YouTube URL formats The minor version bump is appropriate for this breaking change. </details> <details> <summary>src/utils/validate.ts (3)</summary> `1-2`: **LGTM! Comprehensive YouTube URL regex consolidation.** The consolidated regex pattern effectively matches multiple YouTube URL formats including: - youtu.be short URLs - Various youtube.com paths (embed, v, watch, shorts, live) - YouTube variants (nocookie, education) This is a good improvement over having multiple patterns. --- `4-12`: **LGTM! Improved function signature with better type safety.** The changes improve the function: - Optional parameter allows for more flexible usage - Returning `undefined` instead of `null` is more consistent with modern TypeScript practices - Early return for empty input prevents unnecessary regex execution --- `14-17`: **LGTM! Simplified regex maintains same validation logic.** The simplified regex `/^[\w-]{11}$/` is equivalent to the previous pattern but more concise. The `\w` character class includes `[a-zA-Z0-9_]`, so adding the hyphen covers the same validation requirements. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Summary by CodeRabbit
New Features
sourceprop for the YouTube player, accepting a video ID string, YouTube URL, or an object withvideoIdorurl.source.Breaking Changes
videoIdprop with the newsourceprop in the YouTube player component.Documentation
sourceprop.Bug Fixes