-
Notifications
You must be signed in to change notification settings - Fork 1
fix: align inline baseUrl with IFrame origin #76
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
- add trailing-slash handling for WebView baseUrl in inline mode - propagate origin/playerVars into local HTML - refine YoutubeView WebView source resolution
🦋 Changeset detectedLatest commit: 279d5bf 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 |
WalkthroughAdds changesets for a patch release. Updates YoutubeView inline WebView base URL normalization, aligns iframe origin propagation (defaulting to https://localhost), refines WebView source resolution, and enhances error logging. Updates TSDoc for webViewUrl/inline behavior; no exported/public signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as App
participant YV as YoutubeView
participant WV as WebView (inline)
participant IF as YouTube Iframe
Note over YV: Compute baseUrl -> ensure trailing slash<br/>Default to https://localhost/ if missing
App->>YV: render({ webViewBaseUrl, origin, playerVars, useInlineHtml })
YV->>WV: source(html, baseUrl = normalizedBaseUrl)
YV->>WV: inject playerVars including origin (default https://localhost)
WV->>IF: load local HTML (uses playerVars.origin)
IF-->>WV: postMessage(player events)
WV-->>YV: onMessage(event)
Note over YV: handleMessage with improved error logging<br/>(logs error + raw event data)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Deploying react-native-youtube-bridge-example with
|
| Latest commit: |
279d5bf
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0be86008.react-native-youtube-bridge-example.pages.dev |
| Branch Preview URL: | https://fix-inline-origin-baseurl.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: 0
🧹 Nitpick comments (5)
.changeset/fresh-drinks-slide.md (1)
5-9: Changelog reads well; consider adding a concrete example to prevent origin/baseUrl confusion.Suggestion:
- Show a one-liner example pairing: baseUrl
https://localhost:8081/⇄ originhttps://localhost:8081.- Call out that if a custom baseUrl is used, origin must also be provided (or will fall back to
https://localhost).fix: align inline baseUrl with IFrame origin - add trailing-slash handling for WebView baseUrl in inline mode - propagate origin/playerVars into local HTML - refine YoutubeView WebView source resolution + + Example: + - Inline mode: source.baseUrl = `https://localhost:8081/` (note trailing slash) + - IFrame API: playerVars.origin = `https://localhost:8081` (no trailing slash)packages/react-native-youtube-bridge/src/hooks/useCreateLocalPlayerHtml.ts (2)
28-31: Origin fallback improved; also normalize and JS-escape the value.Good call switching to
||so an empty string falls back to localhost. Two follow-ups:
- Normalize away trailing slashes to match the “no trailing slash” guidance.
- Use JS-safe serialization (JSON.stringify) when inlining into the script, rather than HTML escaping, to avoid edge-case escaping issues inside JS strings.
- const safeOrigin = escapeHtml(origin) || 'https://localhost'; + // Normalize origin: trim and drop trailing slashes; fallback to localhost to keep in sync with baseUrl default. + const normalizedOrigin = (origin ?? '').trim().replace(/\/+$/, ''); + const safeOrigin = normalizedOrigin || 'https://localhost';And update the injection further below to serialize safely (outside this hunk):
// Before: ${safeOrigin ? `origin: '${safeOrigin}',` : ''} // After (JS-escape): ${safeOrigin ? `origin: ${JSON.stringify(safeOrigin)},` : ''}
28-31: Optional: derive fallback origin from the inline baseUrl when provided.Right now, if a user supplies a custom baseUrl but omits origin, we’ll still send
https://localhost, causing a mismatch and disabled JS API. Consider deriving the fallback origin from the inline baseUrl (when present) in YoutubeView.tsx and passing it viaplayerVars.origin. I’ve proposed a minimal change on that file’s comment to avoid changing this hook’s signature.packages/react-native-youtube-bridge/src/types/youtube.ts (1)
57-59: Doc update is clear; add one more constraint to prevent common pitfalls.Recommend adding: “baseUrl must be an absolute http(s) URL (not file:// or content://) in inline mode so the IFrame origin check can succeed.”
* In this case, the origin of `webViewUrl` MUST exactly match the YouTube IFrame API `origin`. * - Include the port when applicable (e.g. baseUrl `https://localhost:8081/` ⇄ origin `https://localhost:8081`). * - Use a trailing slash on the `baseUrl`, but not on `origin` (scheme + host [+ port] only). + * - baseUrl must be an absolute http(s) URL (not file:// or content://) so the IFrame API origin check can succeed.packages/react-native-youtube-bridge/src/YoutubeView.tsx (1)
43-49: Optional: Derive and Pass Fallback Origin from webViewBaseUrlThe trailing-slash normalization for
webViewBaseUrland the default baseUrl (https://localhost/) look solid. However, if a consumer supplies a customwebViewBaseUrlbut doesn’t includeoriginin theirplayerVars, the hook will fall back tohttps://localhost, leading to an origin mismatch and broken JS API calls.• In
YoutubeView.tsx(lines 43-49), when you callconst { videoId, playerVars } = useMemo(/* … */);and then
const createPlayerHTML = useCreateLocalPlayerHtml({ videoId, useInlineHtml, ...playerVars });consider injecting a derived
originwheneverplayerVars.originis falsy.
• Example change:- const { videoId, playerVars } = useMemo(() => { - return { videoId: player.getVideoId(), playerVars: player.getOptions() || {} }; - }, [player]); + const { videoId, playerVars } = useMemo(() => { + const options = player.getOptions() || {}; + let derivedOrigin: string | undefined; + if (!options.origin && webViewBaseUrl) { + const base = webViewBaseUrl.endsWith('/') ? webViewBaseUrl : `${webViewBaseUrl}/`; + try { + const u = new URL(base); + derivedOrigin = `${u.protocol}//${u.host}`; + } catch { + // ignore invalid URL; will use https://localhost + } + } + return { + videoId: player.getVideoId(), + playerVars: { ...options, ...(derivedOrigin ? { origin: derivedOrigin } : {}) }, + }; + }, [player, webViewBaseUrl]);This ensures the embedded player’s
originmatches the actual WebView origin instead of defaulting tohttps://localhost.
No other default fororiginis applied elsewhere in the codebase—bothuseCreateLocalPlayerHtml.tsandWebYoutubePlayerController.tsonly pass through whatever ends up inplayerVars.origin.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
.changeset/fresh-drinks-slide.md(1 hunks).changeset/slow-kings-lay.md(1 hunks)packages/react-native-youtube-bridge/src/YoutubeView.tsx(2 hunks)packages/react-native-youtube-bridge/src/hooks/useCreateLocalPlayerHtml.ts(1 hunks)packages/react-native-youtube-bridge/src/types/youtube.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
.changeset/slow-kings-lay.md (3)
packages/react-native-youtube-bridge/src/modules/WebviewYoutubePlayerController.ts (2)
updateCallbacks(162-164)WebviewYoutubePlayerController(4-170)packages/react-native-youtube-bridge/src/YoutubeView.web.tsx (2)
YoutubeView(9-92)state(47-49)packages/web/src/YoutubePlayer.tsx (1)
YoutubePlayer(11-237)
packages/react-native-youtube-bridge/src/YoutubeView.tsx (2)
packages/react-native-youtube-bridge/src/modules/WebviewYoutubePlayerController.ts (2)
updateCallbacks(162-164)WebviewYoutubePlayerController(4-170)packages/web/src/YoutubePlayer.tsx (3)
YoutubePlayer(11-237)message(136-223)err(199-209)
.changeset/fresh-drinks-slide.md (3)
packages/react-native-youtube-bridge/src/YoutubeView.web.tsx (1)
YoutubeView(9-92)packages/react-native-youtube-bridge/src/modules/WebviewYoutubePlayerController.ts (3)
updateCallbacks(162-164)WebviewYoutubePlayerController(4-170)constructor(9-11)packages/web/src/YoutubePlayer.tsx (1)
YoutubePlayer(11-237)
packages/react-native-youtube-bridge/src/hooks/useCreateLocalPlayerHtml.ts (1)
packages/core/src/utils.ts (1)
escapeHtml(22-33)
packages/react-native-youtube-bridge/src/types/youtube.ts (2)
packages/react-native-youtube-bridge/src/YoutubeView.web.tsx (1)
YoutubeView(9-92)packages/web/src/YoutubePlayer.tsx (1)
YoutubePlayer(11-237)
🔇 Additional comments (2)
.changeset/slow-kings-lay.md (1)
1-5: LGTM: patch metadata and doc note look good.Changeset correctly captures the TSDoc adjustment; nothing to fix here.
packages/react-native-youtube-bridge/src/YoutubeView.tsx (1)
115-120: Dev-only logging improvement is helpful.Adding the raw payload to the parse error log aids debugging without affecting production behavior.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores