-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add V2 types and enhance TS Doc documentation #68
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
…Doc documentation - Add `YoutubePlayerEvents`, `YoutubeSource` type definition - Enhance TSDoc with comprehensive parameter descriptions and examples - Change type name `YouTubeError` to `YoutubeError` - Remove unused v1 export `PlayerEvents`, `PlayerControls` type
🦋 Changeset detectedLatest commit: c806af0 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 standardizes type names from "YouTube" to "Youtube" throughout the codebase, introduces new type definitions ( Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Hook as useYouTubePlayer / useYouTubeEvent
participant Types as YoutubeSource / YoutubePlayerEvents
participant Player as YoutubePlayer
Dev->>Hook: Calls hook with YoutubeSource, config, or event type
Hook->>Types: Uses new/renamed types for parameters and return values
Hook->>Player: Instantiates or interacts with YoutubePlayer instance
Note right of Hook: Enhanced JSDoc and usage examples guide Dev
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)
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 (
|
YoutubePlayerEvents, YoutubeSource types and enhance TS…
Deploying react-native-youtube-bridge-example with
|
| Latest commit: |
c806af0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://429073b0.react-native-youtube-bridge-example.pages.dev |
| Branch Preview URL: | https://feat-improve-types-and-docs.react-native-youtube-bridge-example.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: 3
🧹 Nitpick comments (1)
packages/react/src/hooks/useYoutubeVideoId.ts (1)
41-47: Duplicate error-logging & object allocationThe same
"Invalid YouTube source"block appears twice. Extract to a small helper to avoid duplication and minor GC churn.- console.error('Invalid YouTube source: ', sourceValue); - onError?.({ - code: 1002, - message: ERROR_CODES[1002], - }); + reportInvalidSource(sourceValue); ... - console.error('Invalid YouTube source: ', sourceValue); - onError?.({ - code: 1002, - message: ERROR_CODES[1002], - }); + reportInvalidSource(sourceValue);const reportInvalidSource = (value: unknown) => { console.error('Invalid YouTube source: ', value); onError?.({ code: 1002, message: ERROR_CODES[1002] }); };Also applies to: 60-66
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.changeset/heavy-loops-enjoy.md(1 hunks)packages/core/src/index.ts(1 hunks)packages/core/src/types/index.ts(4 hunks)packages/core/src/types/webview.ts(2 hunks)packages/react-native-youtube-bridge/src/hooks/useYouTubeEvent.ts(3 hunks)packages/react-native-youtube-bridge/src/hooks/useYouTubePlayer.ts(1 hunks)packages/react-native-youtube-bridge/src/index.tsx(1 hunks)packages/react-native-youtube-bridge/src/modules/YoutubePlayer.ts(1 hunks)packages/react-native-youtube-bridge/src/types/youtube.ts(1 hunks)packages/react/src/hooks/useYoutubeVideoId.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
packages/core/src/types/webview.ts (1)
packages/core/src/types/index.ts (1)
YoutubeError(36-44)
packages/react/src/hooks/useYoutubeVideoId.ts (2)
packages/react-native-youtube-bridge/src/index.tsx (1)
YoutubeSource(12-12)packages/core/src/types/index.ts (2)
YoutubeSource(15-15)PlayerEvents(76-113)
packages/react-native-youtube-bridge/src/hooks/useYouTubePlayer.ts (1)
packages/core/src/types/index.ts (3)
YoutubeSource(15-15)YoutubePlayerVars(119-171)YoutubeError(36-44)
🔇 Additional comments (21)
packages/core/src/types/webview.ts (1)
1-1: Looks good – rename propagated correctlyImport and interface update correctly reference the new
YoutubeErrortype. No further action needed.Also applies to: 15-16
packages/core/src/index.ts (1)
1-14: Export list contradicts the changeset –PlayerEvents/PlayerControlsstill exposedThe changeset claims these v1 types were removed, yet they remain exported here (lines 2 & 11).
Keeping them is fine for backward compatibility, but:
- If removal is intended, delete the exports and the underlying type definitions in
./typesand bump a major version.- If you want deprecation rather than removal, keep the export but add a JSDoc
@deprecatedtag so downstream users receive a warning.- type PlayerEvents, ... - type PlayerControls,Likely an incorrect or invalid review comment.
packages/react-native-youtube-bridge/src/index.tsx (1)
9-13: 👍 Type re-exports correctly aligned with core packageThe bridge now re-exports the updated
Youtube*types without the legacy ones.packages/react-native-youtube-bridge/src/hooks/useYouTubePlayer.ts (4)
2-2: LGTM! Type import updates align with naming consistency.The import statement correctly uses the renamed types
YoutubeErrorandYoutubeSourceinstead of the oldYouTubeErrorandYouTubeSourcefor consistency across the codebase.
9-35: Excellent documentation enhancement with comprehensive examples.The JSDoc documentation has been significantly improved with clear usage examples showing different ways to instantiate the player and interact with its methods. This will greatly help developers understand how to use the hook effectively.
37-37: Function signature correctly updated with new type.The function parameter type has been properly updated to use
YoutubeSourceinstead of the old type, maintaining consistency with the codebase-wide type renaming.
42-42: Callback parameter type correctly updated.The
onErrorcallback parameter type has been properly updated to useYoutubeErrorinstead of the oldYouTubeErrortype, maintaining consistency with the type renaming effort.packages/react-native-youtube-bridge/src/types/youtube.ts (2)
7-15: Excellent comprehensive documentation with clear usage examples.The JSDoc documentation provides a clear overview of the component with practical usage examples showing how to use the
YoutubeViewcomponent with the player instance.
38-73: Well-documented properties with platform-specific annotations.The property documentation has been enhanced with clear descriptions, platform-specific annotations, and helpful remarks explaining behavior and default values. The use of
@platformtags is particularly helpful for cross-platform development.packages/react-native-youtube-bridge/src/modules/YoutubePlayer.ts (3)
91-101: Excellent documentation enhancement for playback controls.The JSDoc documentation for the
play()method is clear and includes a practical usage example. This pattern is consistently applied across all methods, significantly improving the developer experience.
124-135: Comprehensive documentation for seekTo method.The
seekTomethod documentation properly documents both parameters with clear descriptions and includes a usage example showing the optionalallowSeekAheadparameter.
137-332: Consistent and comprehensive documentation across all methods.All remaining methods have been enhanced with consistent JSDoc documentation including:
- Clear parameter descriptions
- Practical usage examples
- Proper return value documentation
This documentation enhancement will significantly improve the developer experience without changing any functionality.
packages/react-native-youtube-bridge/src/hooks/useYouTubeEvent.ts (3)
12-18: Enhanced documentation with defaultValue parameter.The documentation has been improved to include the
defaultValueparameter and provides a clear usage example showing how to use the hook with default values.
32-41: Comprehensive callback usage example.The documentation provides an excellent example showing how to use the hook with callback functions, including practical usage of the player information in the callback.
55-59: Clear progress event documentation.The documentation for the progress event overload is clear and shows the throttling functionality with a practical example.
packages/core/src/types/index.ts (6)
3-4: Type name consistency improvements.The error code and message types have been properly renamed from
YouTubeErrorCodeandYouTubeErrorMessagetoYoutubeErrorCodeandYoutubeErrorMessagefor consistency with the codebase-wide naming convention.
6-15: Excellent documentation for YoutubeSource type.The
YoutubeSourcetype has been enhanced with comprehensive JSDoc documentation including practical usage examples showing all supported formats (string, videoId object, and URL object). This will greatly help developers understand the different ways to specify video sources.
36-44: YoutubeError type properly renamed and documented.The error type has been correctly renamed from
YouTubeErrortoYoutubeErrorand maintains proper documentation with reference to the YouTube API documentation.
119-171: Comprehensive documentation enhancement for player variables.The
YoutubePlayerVarstype has been significantly enhanced with detailed parameter descriptions, remarks explaining behavior, and default value annotations. This documentation will be invaluable for developers configuring the player.
89-91: Consistent error type usage in PlayerEvents.The
onErrorcallback parameter type has been properly updated to useYoutubeErrorinstead of the oldYouTubeErrortype, maintaining consistency with the type renaming effort.
223-223: YoutubePlayerEvents error type updated.The error event type in
YoutubePlayerEventshas been properly updated to useYoutubeErrorinstead of the old type, maintaining consistency across the event system.
| type YoutubeSource, | ||
| extractVideoIdFromUrl, |
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.
🛠️ Refactor suggestion
Update callback typing to match the new event type
onError is still annotated with PlayerEvents['onError'].
With the new naming it should reference YoutubePlayerEvents to avoid dragging the legacy type back in.
-import {
- ERROR_CODES,
- type PlayerEvents,
+import {
+ ERROR_CODES,
+ type YoutubePlayerEvents,
...
-const useYouTubeVideoId = (source: YoutubeSource, onError?: PlayerEvents['onError']): string | null | undefined => {
+const useYouTubeVideoId = (
+ source: YoutubeSource,
+ onError?: YoutubePlayerEvents['onError'],
+): string | null | undefined => {Also applies to: 10-10
🤖 Prompt for AI Agents
In packages/react/src/hooks/useYoutubeVideoId.ts at lines 4-5 and 10-10, update
the typing of the onError callback from PlayerEvents['onError'] to use the new
YoutubePlayerEvents type. This change ensures the code references the current
event type definitions and avoids using legacy types. Locate all onError
callback typings and replace PlayerEvents with YoutubePlayerEvents accordingly.
.changeset/heavy-loops-enjoy.md
Outdated
| - Add `YoutubePlayerEvents`, `YoutubeSource` type definition | ||
| - Enhance TSDoc with comprehensive parameter descriptions and examples | ||
| - Change type name `YouTubeError` to `YoutubeError` | ||
| - Remove unused v1 export `PlayerEvents`, `PlayerControls` type |
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.
Release note out of-sync with code
The note states that PlayerEvents / PlayerControls have been removed, but they are still exported in packages/core/src/index.ts. Update either the code or the changeset so they align before publishing.
🤖 Prompt for AI Agents
In .changeset/heavy-loops-enjoy.md around lines 8 to 11, the release note
mentions removing the `PlayerEvents` and `PlayerControls` exports, but these
types are still exported in packages/core/src/index.ts. To fix this, either
remove the `PlayerEvents` and `PlayerControls` exports from
packages/core/src/index.ts or update the changeset to reflect that they are
still exported, ensuring the release note and code are consistent before
publishing.
| export type YoutubeViewProps = { | ||
| /** | ||
| * @description The source of the video to play. | ||
| * The player instance. | ||
| * @example | ||
| * ```ts | ||
| * source: 'https://www.youtube.com/watch?v=AbZH7XWDW_k' | ||
| * source: { videoId: 'AbZH7XWDW_k' } | ||
| * source: { url: 'https://www.youtube.com/watch?v=AbZH7XWDW_k' } | ||
| * ```tsx | ||
| * const player = useYouTubePlayer('AbZH7XWDW_k'); | ||
| * | ||
| * <YoutubeView player={player} /> | ||
| * ``` | ||
| */ | ||
| source: YouTubeSource; | ||
| player: YoutubePlayer; |
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.
💡 Verification agent
🧩 Analysis chain
Type rename and API change - verify breaking change handling.
The type has been renamed from YoutubePlayerProps to YoutubeViewProps and the API has changed from accepting a source to accepting a player instance. This is a breaking change that shifts the component towards explicit player control.
This represents a breaking change in the API. Please verify that:
- This change is intentional and documented in migration guides
- All existing usage has been updated to use the new player-based approach
🏁 Script executed:
#!/bin/bash
# Description: Check for any remaining usage of the old YoutubePlayerProps type
# Expected: No occurrences should remain in the codebase
echo "Searching for old YoutubePlayerProps type usage..."
rg "YoutubePlayerProps" --type ts --type tsx
echo "Searching for old source prop usage in YoutubeView..."
rg "source.*YoutubeView|YoutubeView.*source" --type ts --type tsxLength of output: 396
We need to re-run the search without relying on ripgrep’s built-in “tsx” type (it isn’t recognized). Let’s use file globs and find+grep to ensure no leftover references:
🏁 Script executed:
#!/bin/bash
echo "Searching for old YoutubePlayerProps usage..."
find . -type f \( -name '*.ts' -o -name '*.tsx' \) -exec grep -Hn "YoutubePlayerProps" {} +
echo "Searching for <YoutubeView> usages with a ‘source’ prop..."
# Find files importing/using <YoutubeView>, then check for any source= assignment
grep -RIn "<YoutubeView" --include="*.ts" --include="*.tsx" . \
| cut -d: -f1 \
| uniq \
| xargs -r grep -Hn "source\s*=" --include="*.ts" --include="*.tsx"Length of output: 570
Let’s verify that the breaking change is documented in your Markdown docs/CHANGELOG:
#!/bin/bash
echo "Searching for a migration guide in docs..."
grep -RIn "migration" --include="*.md" .
echo "Searching CHANGELOG for YoutubeView or source prop removal..."
grep -RIn "YoutubeView" --include="CHANGELOG.md" .
grep -RIn "source" --include="CHANGELOG.md" .Let’s confirm this breaking change is documented in your markdown files (CHANGELOG or migration guides):
🏁 Script executed:
#!/bin/bash
echo "Searching for migration guide references in all .md files..."
find . -type f -iname "*.md" -exec grep -HIn "migration" {} +
echo "Searching for CHANGELOG entries about YoutubeView or source prop removal..."
find . -type f -iname "*changelog*.md" -exec grep -HIn "YoutubeView" {} +
find . -type f -iname "*changelog*.md" -exec grep -HIn "source" {} +Length of output: 7119
Let’s verify that the V2 migration guide clearly shows replacing the source prop with the new player-based API:
#!/bin/bash
echo "Checking migration guide for useYouTubePlayer instructions..."
grep -RIn "useYouTubePlayer" packages/react-native-youtube-bridge/docs/migration-v2.md
echo "Checking migration guide for <YoutubeView player= usage..."
grep -RIn "<YoutubeView\s*player" packages/react-native-youtube-bridge/docs/migration-v2.md
echo "Reviewing mentions of ‘source’ in the migration guide..."
grep -RIn "source" packages/react-native-youtube-bridge/docs/migration-v2.mdLet’s inspect the V2 migration guide to ensure it clearly covers replacing the source prop with the new player-based API:
#!/bin/bash
echo "1) Searching for useYouTubePlayer in migration guide..."
grep -RIn "useYouTubePlayer" packages/react-native-youtube-bridge/docs/migration-v2.md
echo "2) Searching for <YoutubeView player= usage in migration guide..."
grep -RIn "<YoutubeView\s*player" packages/react-native-youtube-bridge/docs/migration-v2.md
echo "3) Reviewing all ‘source’ occurrences in migration guide..."
grep -RIn "source" packages/react-native-youtube-bridge/docs/migration-v2.md🏁 Script executed:
#!/bin/bash
echo "1) Searching for useYouTubePlayer in migration guide..."
grep -RIn "useYouTubePlayer" packages/react-native-youtube-bridge/docs/migration-v2.md
echo "2) Searching for <YoutubeView player= usage in migration guide..."
grep -RIn "<YoutubeView\s*player" packages/react-native-youtube-bridge/docs/migration-v2.md
echo "3) Searching for ‘source’ in migration guide to confirm removal guidance..."
grep -RIn "source" packages/react-native-youtube-bridge/docs/migration-v2.mdLength of output: 1116
Migration guide updated; clarify removal of source prop
All code references to the old YoutubePlayerProps type and the source prop on <YoutubeView> have been removed, and the V2 migration guide exists under packages/react-native-youtube-bridge/docs/migration-v2.md, showing how to switch from:
YoutubePlayer→YoutubeView+useYouTubePlayer- Import replacement and usage of
useYouTubePlayer(videoId, config)
However, the guide still includes the legacy example with source={videoId} (line 26) without explicitly instructing users to drop that prop. Please update the migration steps to include a callout to remove the source prop from <YoutubeView> usage.
- Verified no remaining
YoutubePlayerPropsorsource=occurrences in code - Migration guide covers your new API but needs an explicit “remove
sourceprop” step
🤖 Prompt for AI Agents
In packages/react-native-youtube-bridge/src/types/youtube.ts around lines 16 to
26, update the migration guide to explicitly instruct users to remove the
deprecated `source` prop from `<YoutubeView>` usage. Add a clear callout in the
migration steps that highlights dropping the `source` prop when switching from
the old `YoutubePlayerProps` to the new `YoutubeView` and `useYouTubePlayer`
API, ensuring the guide no longer shows legacy examples with `source={videoId}`
without explanation.
YoutubePlayerEvents,YoutubeSourcetype definitionYouTubeErrortoYoutubeErrorPlayerEvents,PlayerControls,YoutubePlayerPropstypeSummary by CodeRabbit
Summary by CodeRabbit
YouTubeError→YoutubeError,YouTubeSource→YoutubeSource,YoutubePlayerProps→YoutubeViewProps).