-
Notifications
You must be signed in to change notification settings - Fork 1
fix: resolve 101/150 errors by aligning WebView baseUrl and origin in inline mode #74
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
…igin in inline mode - Set baseUrl to the default `https://localhost/` for inline WebView - Default playerVars.origin to `https://localhost` in inline WebView - Wrap certain dev logs with DEV so they only run in development - Add TSDoc for playerVars.origin and webViewUrl props
WalkthroughAligns inline WebView baseUrl and IFrame origin to https://localhost by default, ensures baseUrl is always set in inline mode, adds dev-only logging for errors, removes thirdPartyCookiesEnabled={false}, and updates TSDoc for origin/webViewUrl. Example app swaps a test video. Changeset adds patch bumps for two packages. Changes
Sequence Diagram(s)sequenceDiagram
actor App
participant YV as YoutubeView (RN)
participant WV as WebView (inline)
participant HTML as Local HTML
participant YT as YouTube IFrame API
App->>YV: render with useInlineHtml
YV->>WV: loadSource{ baseUrl: https://localhost/ }
WV->>HTML: render local HTML
HTML->>YT: create IFrame{ playerVars.origin: https://localhost }
note right of HTML: Origin matches WebView baseUrl
YT-->>HTML: Player ready / events
HTML-->>YV: postMessage events
YV-->>App: emit events (error logs only in DEV)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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: |
a4a3cc6
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0c4a9a83.react-native-youtube-bridge-example.pages.dev |
| Branch Preview URL: | https://fix-webview-baseurl-origin-m.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: 2
🧹 Nitpick comments (8)
packages/react-native-youtube-bridge/src/hooks/useYouTubePlayer.ts (1)
43-45: Align other console.error calls behind DEV checks (optional consistency update)For consistency with the newly added guard in useYouTubePlayer.ts and to avoid noisy production logs, you may want to wrap other
console.errorcalls in__DEV__checks as well. Below are some of the unguarded instances found:
- packages/web/src/hooks/useWebView.ts
· console.error('Failed to send WebView message:', error) (line 30)- packages/react/src/hooks/useYoutubeVideoId.ts
· console.error('Invalid YouTube source: ', sourceValue) (lines 42 & 61)- packages/core/src/WebYoutubePlayerController.ts
· console.error('YouTube API failed to load after timeout') (line 41)
· console.error('YouTube player error:', event.data) (line 133)
· console.error('Progress tracking error:', error) (line 206)- packages/react-native-youtube-bridge/src/hooks/youtubeIframeScripts.ts
· console.error('Progress tracking error:', error) (line 33)
· console.error('Final progress error:', error) (line 68)
· console.error('onPlayerReady error:', error) (line 100)
· console.error('onPlayerStateChange error:', error) (line 148)
· console.error('onPlayerError error:', error) (line 176)
· console.error('onPlaybackRateChange error:', error) (line 192)
· console.error('onPlaybackQualityChange error:', error) (line 209)
· console.error('onAutoplayBlocked error:', error) (line 225)- example/src/App.tsx
· console.error('Player error:', error) (line 152)This refactoring is purely optional and can be deferred if quieter production logs aren’t a priority right now.
packages/react-native-youtube-bridge/src/types/youtube.ts (2)
57-59: Clarify origin/baseUrl matching details (port and trailing slash)Small doc tweak to prevent common 101/150 pitfalls: explicitly call out port matching and trailing slash differences.
Apply within this block:
- * In this case, the origin of `webViewUrl` MUST match the YouTube IFrame API `origin` - * (e.g. baseUrl `https://localhost/` ⇄ origin `https://localhost`). - * + * 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). + *
55-55: Use the TSDoc tag “@remarks” (plural) instead of “@remark”TSDoc’s canonical tag is
@remarks. Consider updating for consistency/readability across docs..changeset/cool-pets-do.md (1)
6-11: Tighten wording: use “DEV”, and mention port/trailing-slash nuanceMinor editorial polish to make the intent crystal-clear and aligned with the code.
Apply:
-fix: resolve 101/150 errors by aligning WebView baseUrl and IFrame origin in inline mode +fix: resolve YouTube 101/150 errors by aligning WebView baseUrl and IFrame origin in inline mode -- Wrap certain dev logs with DEV so they only run in development +- Wrap certain dev logs with __DEV__ so they only run in development - Add TSDoc for playerVars.origin and webViewUrl props + +Notes: +- When using a custom baseUrl like https://localhost:8081/, the IFrame API `origin` must be https://localhost:8081 (port must match). +- Use a trailing slash for `baseUrl` (e.g., https://localhost/), but never for `origin` (scheme + host [+ port] only).packages/core/src/types/index.ts (1)
168-175: Docs: emphasize exact origin match (including port) and slash behaviorThis reduces ambiguity that often leads to 101/150 errors.
Apply:
- /** - * This parameter provides an extra security measure for the IFrame API and is only supported for IFrame embeds. - * - * @remark - * - When `useInlineHtml` is `true` (iOS/Android inline WebView), if not provided, the library defaults this to `https://localhost` and sets the WebView `baseUrl` accordingly so that the document origin and this value match. - * - When `useInlineHtml` is `false` (remote WebView), if not provided, the external page URL defaults to `https://react-native-youtube-bridge.pages.dev` and this value follows that URL. If you pass a custom `webViewUrl` (base URL), `origin` should follow that same origin. - * - In all cases, this value MUST exactly match the document's origin that hosts the iframe for the YouTube IFrame API to function correctly. - */ + /** + * This parameter provides an extra security measure for the IFrame API and is only supported for IFrame embeds. + * + * @remarks + * - Inline WebView (`useInlineHtml: true`): if not provided, defaults to `https://localhost` and the WebView `baseUrl` is set accordingly so the document origin and this value match. + * - Remote WebView (`useInlineHtml: false`): if not provided, the external page defaults to `https://react-native-youtube-bridge.pages.dev` and `origin` follows that URL. If you pass a custom `webViewUrl` (base URL), set `origin` to the same origin. + * - The `origin` MUST exactly equal the document’s origin that hosts the iframe, including port when present (e.g., `https://localhost:8081`). Do not include a trailing slash in `origin`. A trailing slash is required on `baseUrl`. + */packages/react-native-youtube-bridge/src/YoutubeView.tsx (3)
43-46: Good: always providebaseUrlfor inline HTML; consider normalizing the trailing slashSetting
baseUrlby default aligns with the IFrameoriginand helps avoid 101/150. To prevent edge cases when a consumer supplies a custom base URL without a trailing slash (Android in particular is picky), normalize it here.Apply this diff:
- return { - html: createPlayerHTML(), - baseUrl: webViewBaseUrl ?? 'https://localhost/', - }; + return { + html: createPlayerHTML(), + baseUrl: webViewBaseUrl + ? (webViewBaseUrl.endsWith('/') ? webViewBaseUrl : `${webViewBaseUrl}/`) + : 'https://localhost/', + };Optional follow-up: if
playerVars.originis not provided, you could derive it from thewebViewBaseUrl’s origin to keep them in lockstep (e.g., new URL(baseUrl).origin), while retaining thehttps://localhostdefault. I can draft that change if you want.
113-115: Dev-only logging is appropriate; include the raw payload for easier triageWhen JSON parsing fails, seeing the raw
event.nativeEvent.datagreatly speeds up debugging in development.Apply this diff:
- if (__DEV__) { - console.error('Error parsing WebView message:', error); - } + if (__DEV__) { + console.error('Error parsing WebView message:', error, event?.nativeEvent?.data); + }
178-180: Dev-only WebView error log: capture structured details from nativeEventLogging the structured fields (description, code, url) makes production issues reproducible from dev logs.
Apply this diff:
- if (__DEV__) { - console.error('WebView error:', error); - } + if (__DEV__) { + console.error('WebView error:', { + description: error?.nativeEvent?.description, + code: error?.nativeEvent?.code, + url: error?.nativeEvent?.url, + }); + }
📜 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 (7)
.changeset/cool-pets-do.md(1 hunks)example/src/App.tsx(1 hunks)packages/core/src/types/index.ts(1 hunks)packages/react-native-youtube-bridge/src/YoutubeView.tsx(3 hunks)packages/react-native-youtube-bridge/src/hooks/useCreateLocalPlayerHtml.ts(1 hunks)packages/react-native-youtube-bridge/src/hooks/useYouTubePlayer.ts(1 hunks)packages/react-native-youtube-bridge/src/types/youtube.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
.changeset/cool-pets-do.md (2)
packages/react-native-youtube-bridge/src/modules/WebviewYoutubePlayerController.ts (2)
updateCallbacks(162-164)WebviewYoutubePlayerController(4-170)packages/web/src/YoutubePlayer.tsx (2)
YoutubePlayer(11-237)message(136-223)
packages/core/src/types/index.ts (3)
packages/web/src/YoutubePlayer.tsx (1)
YoutubePlayer(11-237)packages/react-native-youtube-bridge/src/YoutubeView.web.tsx (1)
YoutubeView(9-92)packages/core/src/WebYoutubePlayerController.ts (1)
WebYoutubePlayerController(10-377)
packages/react-native-youtube-bridge/src/hooks/useYouTubePlayer.ts (3)
packages/core/src/WebYoutubePlayerController.ts (2)
event(132-148)player(197-209)packages/web/src/YoutubePlayer.tsx (3)
err(199-209)error(66-71)YoutubePlayer(11-237)packages/react-native-youtube-bridge/src/YoutubeView.web.tsx (1)
error(50-52)
packages/react-native-youtube-bridge/src/hooks/useCreateLocalPlayerHtml.ts (2)
packages/core/src/utils.ts (1)
escapeHtml(22-33)packages/web/src/YoutubePlayer.tsx (1)
YoutubePlayer(11-237)
packages/react-native-youtube-bridge/src/YoutubeView.tsx (2)
packages/web/global.d.ts (2)
Window(8-11)ReactNativeWebView(3-5)packages/web/src/hooks/useWebView.ts (6)
UseWebViewReturn(12-70)UseWebViewReturn(6-10)handler(35-59)message(15-33)setIsWebView(61-63)window(56-58)
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)
packages/react-native-youtube-bridge/src/hooks/useYouTubePlayer.ts (1)
43-45: Dev-only logging is correct and non-breakingWrapping the error log with
__DEV__cuts noise in production while preserving the emitted error. Runtime behavior remains intact. LGTM.example/src/App.tsx (1)
296-300: Demo video swap LGTMSwitching to “Viva la Vida” is fine and doesn’t affect examples’ behavior.
| * This parameter provides an extra security measure for the IFrame API and is only supported for IFrame embeds. | ||
| * | ||
| * @remark | ||
| * - When `useInlineHtml` is `true` (iOS/Android inline WebView), if not provided, the library defaults this to `https://localhost` and sets the WebView `baseUrl` accordingly so that the document origin and this value match. | ||
| * - When `useInlineHtml` is `false` (remote WebView), if not provided, the external page URL defaults to `https://react-native-youtube-bridge.pages.dev` and this value follows that URL. If you pass a custom `webViewUrl` (base URL), `origin` should follow that same origin. | ||
| * - In all cases, this value MUST exactly match the document's origin that hosts the iframe for the YouTube IFrame API to function correctly. | ||
| */ | ||
| origin?: string; |
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
Double-check that runtime defaults match these docs
Since this comment promises defaults, verify the implementation actually:
- injects
baseUrl: 'https://localhost/'for inline HTML when not provided, and - defaults
playerVars.origintohttps://localhostin inline mode.
Run:
🏁 Script executed:
#!/bin/bash
# Verify baseUrl defaulting for inline HTML
rg -n -C3 --type ts --type tsx "baseUrl.*https://localhost/?'|\"https://localhost/\"|baseUrl:"
# Verify origin default to https://localhost in inline path
rg -n -C3 --type ts --type tsx "origin.*https://localhost|safeOrigin.*localhost|playerVars\.origin"
# Spot where the WebView source is built to ensure baseUrl is always present
rg -n -C3 --type tsx "WebView|source:\s*\{[^\}]*html" packages/react-native-youtube-bridge/srcLength of output: 380
🏁 Script executed:
#!/bin/bash
set -e
# 1. Find all references to useInlineHtml to see how inline vs remote paths differ
rg -n -C3 "useInlineHtml" .
# 2. Locate where baseUrl is set or defaulted in the WebView source builder
rg -n -C3 "baseUrl" .
# 3. Look for playerVars.origin defaults in the code
rg -n -C3 "playerVars" .
# 4. Inspect all WebView component usages to see how source/html and baseUrl interact
rg -n -C3 "WebView" packages/react-native-youtube-bridge/srcLength of output: 68622
🏁 Script executed:
#!/bin/bash
set -e
# Inspect instantiation of YT.Player to verify default origin assignment
rg -n -C3 "new YT.Player" packages/react-native-youtube-bridge/src/hooks/useCreateLocalPlayerHtml.ts
# Search for origin usage in HTML generation
rg -n -C3 "origin" packages/react-native-youtube-bridge/src/hooks/useCreateLocalPlayerHtml.tsLength of output: 1491
🏁 Script executed:
#!/bin/bash
set -e
# Inspect import of escapeHtml in local HTML hook
sed -n '1,30p' packages/react-native-youtube-bridge/src/hooks/useCreateLocalPlayerHtml.tsLength of output: 1099
Ensure the documented origin default is actually applied in inline‐HTML mode
- The default of
baseUrl: 'https://localhost/'whenuseInlineHtmlistrueis implemented correctly inYoutubeView.tsx(…src/YoutubeView.tsx line 44) . - However, in the HTML generator hook (
useCreateLocalPlayerHtml), the code uses(…src/hooks/useCreateLocalPlayerHtml.ts lines 28, 108) .const safeOrigin = escapeHtml(origin) ?? 'https://localhost'; … ${safeOrigin ? `origin: '${safeOrigin}',` : ''}
BecauseescapeHtml(undefined)yields''(an empty string, notnull/undefined), the??fallback never runs andsafeOriginis falsy—so nooriginis injected.
Refactor required
Update the fallback to use || so that an empty string also triggers the default:
- const safeOrigin = escapeHtml(origin) ?? 'https://localhost';
+ const safeOrigin = escapeHtml(origin) || 'https://localhost';This ensures that when no origin is provided, the hook injects origin: 'https://localhost' into the player configuration, matching the documentation.
| } | ||
|
|
||
| const safeOrigin = escapeHtml(origin); | ||
| const safeOrigin = escapeHtml(origin) ?? 'https://localhost'; |
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.
Fix origin fallback: escapeHtml returns an empty string, so ?? 'https://localhost' never triggers
As implemented, escapeHtml(undefined) returns '', not undefined/null, so safeOrigin becomes '' and the origin playerVar is omitted. This defeats the PR’s goal to default the IFrame origin to https://localhost in inline mode and can still trigger 101/150 if baseUrl is set but origin is missing.
Apply this diff to ensure the default is actually applied and still sanitized:
- const safeOrigin = escapeHtml(origin) ?? 'https://localhost';
+ const safeOrigin = escapeHtml(origin ?? 'https://localhost');Optional hardening: strip a trailing slash if the caller passes one to keep the value a proper “origin”:
- const safeOrigin = escapeHtml(origin ?? 'https://localhost');
+ const safeOrigin = escapeHtml((origin ?? 'https://localhost').replace(/\/$/, ''));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const safeOrigin = escapeHtml(origin) ?? 'https://localhost'; | |
| const safeOrigin = escapeHtml(origin ?? 'https://localhost'); |
🤖 Prompt for AI Agents
In packages/react-native-youtube-bridge/src/hooks/useCreateLocalPlayerHtml.ts
around line 28, escapeHtml(undefined) returns an empty string so the nullish
fallback (?? 'https://localhost') never runs; change the logic to treat an empty
string as missing by checking the raw origin first (use origin ??
'https://localhost' or origin ? escapeHtml(origin) : 'https://localhost') so the
default origin is applied and still sanitized, and optionally trim any trailing
'/' from the resolved origin to ensure it remains a proper origin.
https://localhost/for inline WebViewhttps://localhostin inline WebViewSummary by CodeRabbit
Bug Fixes
Documentation
Chores