-
Notifications
You must be signed in to change notification settings - Fork 1
feat: support optional YouTube source for dynamic loading #66
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
- Extend YouTubeSource type to accept undefined values - Add defensive logic for undefined source handling - Enable async video ID loading patterns - Maintain backward compatibility with existing usage
🦋 Changeset detectedLatest commit: 1b6d919 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
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 update expands the flexibility of YouTube video source handling across several packages. The type definitions and related logic now support Changes
Sequence Diagram(s)sequenceDiagram
participant ReactComponent
participant useYouTubePlayer
participant useYouTubeVideoId
participant YoutubePlayer
participant WebView/DOM
ReactComponent->>useYouTubePlayer: Passes source (may be undefined)
useYouTubePlayer->>useYouTubeVideoId: Extracts videoId (returns string/null/undefined)
useYouTubeVideoId->>useYouTubePlayer: Returns videoId or undefined
useYouTubePlayer->>YoutubePlayer: Initializes with videoId (may be undefined)
alt videoId is defined
YoutubePlayer->>WebView/DOM: Sets up player with videoId
else videoId is undefined/null
YoutubePlayer-->>WebView/DOM: Skips setup, waits for valid videoId
end
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
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. 🪧 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-example with
|
| Latest commit: |
1b6d919
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fba8a209.react-native-youtube-bridge-example.pages.dev |
| Branch Preview URL: | https://feat-optional-youtube-source.react-native-youtube-bridge-example.pages.dev |
Deploying react-native-youtube-bridge with
|
| Latest commit: |
66610c1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c1cbebb6.react-native-youtube-bridge.pages.dev |
| Branch Preview URL: | https://feat-optional-youtube-source.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: 0
🧹 Nitpick comments (2)
packages/react-native-youtube-bridge/src/YoutubeView.tsx (1)
39-39: Address the exhaustive dependencies warning.The biome-ignore comment lacks a proper explanation. Consider either adding the missing dependencies to the dependency array or providing a clear explanation of why they should be omitted.
- // biome-ignore lint/correctness/useExhaustiveDependencies: <explanation> + // biome-ignore lint/correctness/useExhaustiveDependencies: webViewProps.source is intentionally excluded to prevent unnecessary re-renderspackages/react/src/hooks/useYoutubeVideoId.ts (1)
31-37: Consider simplifying with optional chaining.The dependency array logic correctly implements safe property access, but could be more concise using optional chaining.
- typeof source === 'string' - ? source - : source && 'videoId' in source - ? source.videoId - : source && 'url' in source - ? source.url - : null, + typeof source === 'string' + ? source + : source?.videoId || source?.url || null,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.changeset/eight-wolves-swim.md(1 hunks)packages/core/src/types/index.ts(1 hunks)packages/core/src/utils.ts(1 hunks)packages/react-native-youtube-bridge/src/YoutubeView.tsx(2 hunks)packages/react-native-youtube-bridge/src/YoutubeView.web.tsx(1 hunks)packages/react-native-youtube-bridge/src/hooks/useCreateLocalPlayerHtml.ts(1 hunks)packages/react-native-youtube-bridge/src/modules/YoutubePlayer.ts(1 hunks)packages/react-native-youtube-bridge/src/utils/youtube.ts(1 hunks)packages/react/src/hooks/useYoutubeOEmbed.ts(2 hunks)packages/react/src/hooks/useYoutubeVideoId.ts(3 hunks)packages/web/src/YoutubePlayer.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/react-native-youtube-bridge/src/utils/youtube.ts (1)
packages/core/src/types/index.ts (1)
YoutubePlayerVars(100-146)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages: react-native-youtube-bridge
🔇 Additional comments (24)
packages/web/src/YoutubePlayer.tsx (1)
46-46: LGTM! Defensive check prevents initialization with falsy video IDs.The addition of
!youtubeVideoIdto the conditional check correctly prevents player initialization when no valid video ID is available, supporting the asynchronous loading pattern while maintaining backward compatibility.packages/react-native-youtube-bridge/src/YoutubeView.web.tsx (1)
30-32: LGTM! Early return prevents player setup without valid video ID.The defensive check correctly prevents further player initialization when no valid video ID is available, aligning with the asynchronous loading pattern support.
packages/react-native-youtube-bridge/src/utils/youtube.ts (2)
5-5: LGTM! Parameter type expanded to support nullable video IDs.The parameter type change correctly supports the asynchronous loading pattern by accepting
nullandundefinedvalues.
11-11: Verified:undefinedreturn handled by downstream consumerI checked all usages of
getYoutubeWebViewUrland found a single consumer inpackages/react-native-youtube-bridge/src/YoutubeView.tsx(around line 37). The code wraps its result in a truthy check:
- If
useInlineHtmlis true, it always returns an inline HTML source.- Otherwise, it only constructs
{ uri: webViewUrl }whenwebViewUrlis truthy (if (webViewUrl)).Since
undefinedis falsy, the branch guards ensure we never pass an invalid URI to<WebView>. No further handling is required.packages/core/src/utils.ts (3)
13-13: LGTM! Parameter type expanded to include null.The type expansion correctly supports the broader pattern of accepting nullable video IDs throughout the codebase.
14-16: LGTM! Explicit falsy check improves readability.The explicit falsy check makes the logic clearer and ensures proper handling of null/undefined values before proceeding with regex validation.
19-19: LGTM! Nullish coalescing removal is correct.Removing the nullish coalescing operator is appropriate since the earlier falsy check guarantees that
videoIdis a defined string at this point.packages/react/src/hooks/useYoutubeOEmbed.ts (3)
24-24: LGTM! Parameter made optional to support async loading.Making the
urlparameter optional correctly supports the asynchronous loading pattern where the URL may initially be undefined.
30-32: LGTM! Early return prevents unnecessary fetch attempts.The defensive check correctly prevents the fetch operation from running when no valid URL is provided, avoiding potential errors and unnecessary network requests.
68-68: LGTM! Redundant conditional check removed.The unconditional call to
fetchOEmbed()is now safe since the early return guard ensures this code only runs with valid URLs.packages/react-native-youtube-bridge/src/hooks/useCreateLocalPlayerHtml.ts (3)
18-18: Type expansion correctly supports optional video IDs.The parameter type update to
string | null | undefinedaligns with the PR objective to support optional YouTube sources for dynamic loading.
20-22: Defensive check prevents unnecessary HTML generation.Good defensive programming - returning early when
videoIdisundefinedprevents unnecessary HTML generation and aligns with the new optional source support.
25-25: Enhanced error message with improved styling.The error message update includes inline CSS for better visual presentation (centered, white text) and clearer messaging ("Invalid YouTube ID" instead of "Invalid video ID").
packages/core/src/types/index.ts (1)
6-6: Type expansion enables flexible async loading patterns.The
YouTubeSourcetype expansion to includeundefinedvalues at multiple levels (entire source,videoId, andurlproperties) correctly supports the PR's goal of enabling asynchronous video ID loading while maintaining backward compatibility with existing string and object sources.packages/react-native-youtube-bridge/src/modules/YoutubePlayer.ts (3)
23-23: Property type updated for nullable video ID support.The
videoIdproperty type change tostring | null | undefinedaligns with the expandedYouTubeSourcetype and enables the class to handle optional video IDs.
26-29: Constructor properly accepts optional video IDs.The constructor parameter type update maintains consistency with the property type and enables instantiation with undefined video IDs for async loading scenarios.
31-33: Getter method signature updated for consistency.The
getVideoId()method return type update tostring | null | undefinedmaintains API consistency with the internal property type and constructor parameter.packages/react-native-youtube-bridge/src/YoutubeView.tsx (2)
40-50: Refactored source construction improves maintainability.The refactor centralizes the WebView source construction logic into a memoized value, making it more maintainable and eliminating the previous inline conditional expression. The logic properly handles both inline HTML and external URL cases with appropriate fallback to
undefined.
162-162: Direct usage of memoized source value.Using the
webViewSourcedirectly simplifies the JSX and leverages the centralized memoization logic..changeset/eight-wolves-swim.md (1)
1-22: Changeset accurately documents the feature addition.The changeset properly documents the new optional YouTube source support across all affected packages. The feature description is clear and the usage example effectively demonstrates the new async loading patterns enabled by the type system updates.
packages/react/src/hooks/useYoutubeVideoId.ts (4)
10-10: LGTM: Function signature correctly implements tri-state return type.The updated return type
string | null | undefinedproperly supports the new dynamic loading pattern while maintaining type safety.
13-16: LGTM: Defensive early return for optional source.The early return for falsy source values correctly handles the dynamic loading scenario where source may initially be undefined, preventing unnecessary processing.
41-52: LGTM: Explicit tri-state logic implementation.The explicit handling of
nullvsundefinedstates is well-implemented:
null: Invalid source triggers error callbackundefined: Missing source returns undefined (no error)- Valid string: Proceeds with validation
This design properly supports asynchronous loading patterns.
47-47: LGTM: Consistent error return values.Returning
nullinstead of empty string for error cases maintains consistency with the tri-state design and provides clearer error signaling.Also applies to: 66-66
Summary by CodeRabbit
New Features
Bug Fixes
Refactor