-
Notifications
You must be signed in to change notification settings - Fork 1
refactor: migrate monorepo #38
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
|
|
Caution Review failedThe pull request is closed. WalkthroughThis change restructures the project into a monorepo with distinct core, react, and web packages, centralizing types, constants, and utilities. It introduces new package configurations, refactors imports/exports, and updates build, lint, and typecheck scripts. The YouTube player logic is modularized, with enhanced type safety, error code management, and improved documentation across packages. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App/Example
participant RN as React Native Package
participant Core as Core Package
participant ReactPkg as React Package
participant Web as Web Package
App->>RN: Uses YoutubePlayer component
RN->>Core: Imports types, constants, utils
RN->>ReactPkg: Imports hooks (useYouTubeVideoId, useYouTubePlayer)
Web->>ReactPkg: Imports hooks
ReactPkg->>Core: Imports core logic/types
App->>Web: Uses YoutubePlayer (web)
Web->>Core: Uses types, constants, utils
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (57)
✨ 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: |
a10788d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://573fa6e8.react-native-youtube-bridge.pages.dev |
| Branch Preview URL: | https://refactor-monorepo.react-native-youtube-bridge.pages.dev |
Deploying react-native-youtube-bridge-example with
|
| Latest commit: |
89c0ffe
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://56a80029.react-native-youtube-bridge-example.pages.dev |
| Branch Preview URL: | https://refactor-monorepo.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: 7
🧹 Nitpick comments (10)
packages/react-native-youtube-bridge/.gitignore (1)
1-1: Align .gitignore with monorepo root patternsConsider extending this .gitignore to include other common build artifacts and OS files (e.g.,
node_modules/,dist/,.DS_Store) for consistency across all packages.packages/core/src/constants.ts (1)
14-15: Add unit tests for URL-matching regexThe
MATCH_URL_YOUTUBEregex is complex and critical to correct parsing. Consider adding targeted unit tests to cover all supported URL formats and prevent future regressions.packages/core/src/utils.ts (1)
13-16: Consider extracting video-ID length.
ThevalidateVideoIdfunction hardcodes11as the ID length. You may centralize this value (e.g.VIDEO_ID_LENGTH) inconstants.tsto keep your ID-validation logic and URL-matching constants in sync.web/tsconfig.app.json (1)
24-26: Centralize path mappings.
Defining the@react-native-youtube-bridge/webalias here is good for TS resolution. To avoid duplication across apps, consider moving all workspace-package path mappings into a shared basetsconfig.json, then extend it in your app-specific configs.web/package.json (1)
7-10: Streamlined build script is appropriate.Dropping the explicit
tsc -bin favor ofvite buildmatches the root workflow changes and speeds up the local iteration loop.Consider adding a
"type-check": "tsc --noEmit"script for local type validation if you want IDE/CLI type errors without a full CI run.packages/web/vite.config.ts (1)
7-15: Consider adding CommonJS format for broader compatibility.The library build configuration is well-structured with appropriate externals for React dependencies. However, consider adding CommonJS format alongside ES modules for better compatibility with older build tools and Node.js environments.
build: { lib: { entry: 'src/index.ts', - formats: ['es'], + formats: ['es', 'cjs'], }, rollupOptions: { external: ['react', 'react-dom'], }, },packages/react-native-youtube-bridge/CHANGELOG.md (1)
79-79: Minor style improvement suggestion.Consider using "resolve" instead of "fix" for more formal wording.
- - fix issue where seekTo doesn't work properly when paused without interval + - resolve issue where seekTo doesn't work properly when paused without intervalpackages/react-native-youtube-bridge/README-ko_kr.md (2)
27-27: Add alt text for better accessibility.Consider adding alt text to the image for better accessibility.
- <img src="./assets/example.gif" width="600" /> + <img src="./assets/example.gif" alt="React Native YouTube Bridge example demonstration" width="600" />
242-242: Consider using a markdown link for the bare URL.For better markdown formatting, consider wrapping the bare URL in a link.
-2. **웹뷰 모드** (`useInlineHtml: false`)는 외부 플레이어 페이지를 로드합니다. 기본 URI는 https://react-native-youtube-bridge.pages.dev 입니다. +2. **웹뷰 모드** (`useInlineHtml: false`)는 외부 플레이어 페이지를 로드합니다. 기본 URI는 <https://react-native-youtube-bridge.pages.dev> 입니다.packages/web/src/YoutubePlayer.tsx (1)
9-19: Consider extracting URL parameter parsing to a utility function.The URL parameter parsing logic could be extracted to a reusable utility function with proper type validation for better maintainability and type safety.
Consider creating a utility function:
type PlayerParams = { videoId: string; startTime: number; endTime?: number; autoplay: boolean; controls: boolean; loop: boolean; muted: boolean; playsinline: boolean; rel: boolean; origin: string; }; function parsePlayerParams(searchParams: URLSearchParams): PlayerParams { const safeParseNumber = (value: string | null, defaultValue: number = 0): number => { const parsed = Number(value); return Number.isNaN(parsed) ? defaultValue : parsed; }; const safeParseBool = (value: string | null): boolean => value === 'true'; return { videoId: searchParams.get('videoId') ?? '', startTime: safeParseNumber(searchParams.get('startTime')), endTime: searchParams.get('endTime') ? safeParseNumber(searchParams.get('endTime')) : undefined, autoplay: safeParseBool(searchParams.get('autoplay')), controls: safeParseBool(searchParams.get('controls')), loop: safeParseBool(searchParams.get('loop')), muted: safeParseBool(searchParams.get('muted')), playsinline: safeParseBool(searchParams.get('playsinline')), rel: safeParseBool(searchParams.get('rel')), origin: searchParams.get('origin') ?? '', }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
-
bun.lockis excluded by!**/*.lock -
example/bun.lockis excluded by!**/*.lock -
web/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (57)
-
.github/workflows/ci.yml(1 hunks) -
biome.json(1 hunks) -
example/babel.config.js(1 hunks) -
example/metro.config.js(1 hunks) -
example/package.json(1 hunks) -
example/tsconfig.json(1 hunks) -
lefthook.yml(1 hunks) -
package.json(3 hunks) -
packages/core/global.d.ts(1 hunks) -
packages/core/package.json(1 hunks) -
packages/core/src/YoutubePlayerCore.ts(3 hunks) -
packages/core/src/constants.ts(1 hunks) -
packages/core/src/index.ts(1 hunks) -
packages/core/src/types/iframe.ts(1 hunks) -
packages/core/src/types/index.ts(2 hunks) -
packages/core/src/types/webview.ts(1 hunks) -
packages/core/src/utils.ts(1 hunks) -
packages/core/tsconfig.json(1 hunks) -
packages/core/tsdown.config.ts(1 hunks) -
packages/react-native-youtube-bridge/.gitignore(1 hunks) -
packages/react-native-youtube-bridge/CHANGELOG.md(1 hunks) -
packages/react-native-youtube-bridge/README-ko_kr.md(1 hunks) -
packages/react-native-youtube-bridge/README.md(1 hunks) -
packages/react-native-youtube-bridge/babel.config.js(1 hunks) -
packages/react-native-youtube-bridge/package.json(1 hunks) -
packages/react-native-youtube-bridge/src/YoutubePlayer.tsx(1 hunks) -
packages/react-native-youtube-bridge/src/YoutubePlayer.web.tsx(1 hunks) -
packages/react-native-youtube-bridge/src/hooks/useCreateLocalPlayerHtml.ts(1 hunks) -
packages/react-native-youtube-bridge/src/index.tsx(1 hunks) -
packages/react-native-youtube-bridge/src/types/message.ts(1 hunks) -
packages/react-native-youtube-bridge/src/types/youtube.ts(1 hunks) -
packages/react-native-youtube-bridge/src/utils/youtube.ts(1 hunks) -
packages/react-native-youtube-bridge/tsconfig.build.json(2 hunks) -
packages/react-native-youtube-bridge/tsconfig.json(1 hunks) -
packages/react/package.json(1 hunks) -
packages/react/src/hooks/index.ts(1 hunks) -
packages/react/src/hooks/useYoutubePlayer.ts(3 hunks) -
packages/react/src/hooks/useYoutubeVideoId.ts(1 hunks) -
packages/react/src/index.ts(1 hunks) -
packages/react/tsconfig.json(1 hunks) -
packages/react/tsdown.config.ts(1 hunks) -
packages/web/global.d.ts(1 hunks) -
packages/web/package.json(1 hunks) -
packages/web/src/YoutubePlayer.tsx(1 hunks) -
packages/web/src/hooks/useWebView.ts(3 hunks) -
packages/web/src/index.ts(1 hunks) -
packages/web/src/types/message.ts(1 hunks) -
packages/web/tsconfig.json(1 hunks) -
packages/web/vite-env.d.ts(1 hunks) -
packages/web/vite.config.ts(1 hunks) -
tsconfig.base.json(1 hunks) -
tsconfig.build.json(0 hunks) -
web/package.json(1 hunks) -
web/src/App.tsx(1 hunks) -
web/src/utils/index.ts(0 hunks) -
web/tsconfig.app.json(1 hunks) -
web/vite.config.ts(1 hunks)
💤 Files with no reviewable changes (2)
- tsconfig.build.json
- web/src/utils/index.ts
🧰 Additional context used
🧬 Code Graph Analysis (5)
packages/core/global.d.ts (1)
packages/core/src/types/iframe.ts (1)
IframeApiType(4-6)
packages/core/src/YoutubePlayerCore.ts (1)
packages/core/src/types/index.ts (1)
YoutubePlayerConfig(102-106)
packages/react-native-youtube-bridge/src/types/youtube.ts (1)
packages/core/src/types/index.ts (3)
YouTubeSource(6-6)YoutubePlayerVars(71-100)PlayerEvents(53-69)
packages/web/src/YoutubePlayer.tsx (3)
packages/core/src/YoutubePlayerCore.ts (1)
setProgressInterval(338-350)packages/web/src/hooks/useWebView.ts (1)
useWebView(12-70)packages/react/src/index.ts (2)
useYouTubeVideoId(1-1)useYouTubePlayer(1-1)
packages/core/src/types/index.ts (2)
packages/core/src/constants.ts (1)
ERROR_CODES(1-12)packages/react-native-youtube-bridge/src/index.tsx (7)
ERROR_CODES(4-4)ProgressData(7-7)YouTubeError(9-9)PlayerState(5-5)PlaybackQuality(8-8)PlayerInfo(10-10)YoutubePlayerVars(6-6)
🪛 LanguageTool
packages/react-native-youtube-bridge/CHANGELOG.md
[style] ~79-~79: Consider using a different verb for a more formal wording.
Context: ...ive logic for cases without videoId - fix issue where seekTo doesn't work properl...
(FIX_RESOLVE)
🪛 markdownlint-cli2 (0.17.2)
packages/react-native-youtube-bridge/README-ko_kr.md
27-27: Images should have alternate text (alt text)
null
(MD045, no-alt-text)
242-242: Bare URL used
null
(MD034, no-bare-urls)
🔇 Additional comments (64)
packages/core/src/constants.ts (1)
1-12: Centralized YouTube error codes look solidThe
ERROR_CODESmapping cleanly centralizes code identifiers and is properly markedas const. This will improve consistency across packages.tsconfig.base.json (2)
5-5: Verify TS support formoduleResolution: "Bundler"Confirm that all build tools and CI environments use a TypeScript version ≥5.2 (or appropriate) so they support the
"Bundler"resolution strategy.
7-8: Reviewdeclarationvs.noEmitconfigurationHaving
"declaration": truetogether with"noEmit": truemay prevent declaration files from being generated. Ensure package-level build configs overridenoEmitwhen producing.d.ts.packages/react-native-youtube-bridge/src/utils/youtube.ts (1)
1-1: Verify core package import pathPlease confirm that the
@react-native-youtube-bridge/corepackage actually exportsYoutubePlayerVarsand that module resolution paths are configured accordingly.packages/web/vite-env.d.ts (1)
1-1: Add Vite client type declaration.
This file correctly enables TypeScript support for Vite-specific environment variables in the web package.packages/web/src/types/message.ts (1)
1-1: Centralize PlayerControls import.
Switching to@react-native-youtube-bridge/corefor thePlayerControlstype aligns with the monorepo’s modular architecture and avoids local duplication.example/package.json (1)
22-25: Verify example project configuration.
You’ve replacedreact-native-builder-bobwithbabel-plugin-module-resolverindevDependencies. Confirm that yourbabel.config.jsor.babelrcincludes themodule-resolverplugin setup and remove any stale Builder Bob references from the Metro configuration.packages/core/src/utils.ts (1)
1-11: Centralize YouTube URL regex.
ImportingMATCH_URL_YOUTUBEfromconstants.tsremoves inline duplication and improves maintainability. Ensure that the regex inMATCH_URL_YOUTUBEhas a capturing group for the video ID at index 1..github/workflows/ci.yml (1)
48-48: ```bash
#!/usr/bin/env bashEnumerate
buildscripts in each package.json (excluding node_modules)find . -type f -name 'package.json' ! -path '/node_modules/' | while IFS= read -r file; do
script=$(jq -r '.scripts.build // "MISSING"' "$file")
echo "$file -> $script"
done</details> <details> <summary>packages/web/global.d.ts (1)</summary> `1-1`: **Verify `ReceiveMessage` import path and global declaration** The import was updated to `./src/types/message`. Ensure that `message.ts` exports `ReceiveMessage` and that this `global.d.ts` is included by TypeScript (via `tsconfig.json` `include` or `typeRoots`). ```shell #!/bin/bash # Check that ReceiveMessage is exported correctly rg -q "export (type )?ReceiveMessage" packages/web/src/types/message.ts \ && echo "ReceiveMessage export found" || echo "Missing ReceiveMessage export"packages/web/src/index.ts (1)
1-1: Re-exportYoutubePlayerat package entry
This aligns the package’s public API with the new component. Ensure thatpackage.json’s"exports"or"main"field points to thisindex.tsso consumers can import it directly.example/tsconfig.json (1)
4-6: Let’s extract the fullextraNodeModulesblock to see if the alias is defined:#!/bin/bash # Extract extraNodeModules block from Metro config awk '/config\.resolver\.extraNodeModules/,/}/' example/metro.config.jspackages/react/src/index.ts (1)
1-1: To confirm exactly how (or if) these hooks are exported, let’s pull up their definitions inpackages/react/src/hooks/index.ts:#!/bin/bash rg -n "useYouTubePlayer" packages/react/src/hooks/index.ts || echo "useYouTubePlayer not found in hooks index" rg -n "useYouTubeVideoId" packages/react/src/hooks/index.ts || echo "useYouTubeVideoId not found in hooks index"packages/react/src/hooks/useYoutubeVideoId.ts (2)
1-7: LGTM! Import migration to core package looks correct.The centralization of imports from the
@react-native-youtube-bridge/corepackage aligns well with the monorepo structure. This properly abstracts shared types, constants, and utilities.
34-36: ```shell
#!/bin/bashDescription: Locate where ERROR_CODES is defined and confirm presence of key 1002
1. Find definition of ERROR_CODES constant
rg "ERROR_CODES" -n
2. Search for the literal key "1002" in the repo
rg "1002\s*:" -n
3. (Optional AST check) Ensure the object literal for ERROR_CODES includes 1002
ast-grep --pattern 'ERROR_CODES = { $$$ 1002:
$_ $ $$ }'</details> <details> <summary>biome.json (1)</summary> `9-11`: **LGTM! Biome configuration properly updated for monorepo.** The addition of `"packages/**/*"` to includes and `"dist"` to ignore is correct for the monorepo structure. This ensures Biome processes all package source files while excluding build artifacts. </details> <details> <summary>packages/core/tsconfig.json (1)</summary> `1-8`: **Verify base TypeScript configuration exists.** The configuration structure looks good for a monorepo package. Please ensure the base configuration file at `../../tsconfig.base.json` exists and contains appropriate settings. ```shell #!/bin/bash # Description: Verify the base TypeScript configuration exists # Expected: Should find the base tsconfig file fd --type f "tsconfig.base.json" --max-depth 1web/vite.config.ts (1)
10-12: LGTM! Monorepo aliases configured correctly.The aliases properly map scoped package names to their respective source entry points. This enables direct development against source files in the workspace.
Verify that the target index files exist:
#!/bin/bash # Description: Verify alias target files exist # Expected: All three index.ts files should exist echo "Checking alias target files:" ls -la packages/web/src/index.ts packages/core/src/index.ts packages/react/src/index.tspackages/react-native-youtube-bridge/src/YoutubePlayer.web.tsx (1)
1-6: LGTM! Import reorganization aligns with monorepo structure.The separation of imports is well-structured:
- Core types from
@react-native-youtube-bridge/core- React hooks from
@react-native-youtube-bridge/react- Component-specific types remain local
This maintains good separation of concerns across packages.
packages/react-native-youtube-bridge/babel.config.js (1)
2-11: Babel overrides correctly separate source from dependencies.Using
react-native-builder-bobfor your source files and@react-native/babel-presetfornode_modulesis the right approach to ensure consistent transpilation within the monorepo.packages/react-native-youtube-bridge/tsconfig.json (1)
2-6: Base config extension and compiler options look solid.Extending the root
tsconfig.base.jsonand setting"jsx": "react-native"with the chosen libs aligns with other packages in the monorepo.packages/react/tsdown.config.ts (1)
1-9: Consistenttsdownconfig for the React package.Defining both ESM and CJS outputs with
.d.tsgeneration aligns with other packages. This minimal setup is correct.packages/react/tsconfig.json (1)
2-9: React package TypeScript settings look good.Extending the base config, targeting
dist, and enabling"jsx": "react-jsx"is consistent and complete.packages/react/src/hooks/index.ts (1)
1-2: Centralized hooks export: Approved.
Re-exporting hooks simplifies imports for consumers and aligns with the monorepo modularization. Ensure the file name casing and paths (./useYoutubePlayerand./useYoutubeVideoId) exactly match your filesystem.packages/web/tsconfig.json (1)
1-9: Validate Web package TS config.
The config correctly extends the base, setsjsxtoreact-jsx, and scopes includes/excludes appropriately. Confirm thatglobal.d.tsexists at the expected path and that no test files or other directories should be excluded.packages/react-native-youtube-bridge/tsconfig.build.json (3)
8-8: Reviewlibordering.
TargetingESNext,DOM, andDOM.Iterableis appropriate for modern builds; just verify thatDOM.Iterableis needed by your consumer code.
23-23: Verify TS version forverbatimModuleSyntax.
The"verbatimModuleSyntax": trueflag requires TS 5+. Ensure your base config and CI environment are on a compatible version.
25-26: Include/exclude adjustments look good.
Explicitly including onlysrc/**/*and excludingliband test files keeps type-checking focused. Confirm no other directories (e.g., examples, web) need to be excluded in this package.packages/core/tsdown.config.ts (1)
1-10: Core package build config is correct.
Thetsdownsetup exports both ESM and CJS bundles with.d.tsand outputs todist, matching the repo’s build strategy.packages/react-native-youtube-bridge/src/hooks/useCreateLocalPlayerHtml.ts (2)
1-1: Consolidated core imports: Approved.
ImportingYoutubePlayerVars,escapeHtml,safeNumber, andvalidateVideoIdfrom the core package centralizes validation and sanitization logic. Confirm these symbols are exported by@react-native-youtube-bridge/core.
4-4: Relative import path update: Verify path.
Switching toimport { youtubeIframeScripts } from './youtubeIframeScripts'relies on the script file being adjacent. Double-check thatyoutubeIframeScripts.tsexists in the same directory.packages/react-native-youtube-bridge/src/YoutubePlayer.tsx (2)
1-2: Import restructuring looks good for monorepo organization.The imports have been correctly updated to use the new monorepo package structure, separating core functionality from React-specific hooks. This improves modularity and reusability across different platforms.
8-9: Local type imports maintained appropriately.Good decision to keep platform-specific types (
CommandType,YoutubePlayerProps) as local imports while moving shared functionality to the core package.packages/core/global.d.ts (1)
1-9: Global TypeScript declarations are well-structured.The global Window interface extension properly types the YouTube iframe API properties. Using a dedicated
global.d.tsfile is a best practice for organizing global type augmentations. The_ytApiPromisenaming with underscore prefix appropriately indicates internal usage.example/babel.config.js (1)
4-20: Babel configuration correctly updated for monorepo structure.The migration from dynamic to static configuration makes the setup more explicit and maintainable. The module resolver aliases properly map the monorepo packages to their source directories.
Verify that the alias paths are correct relative to the example directory:
#!/bin/bash # Verify that the aliased paths exist relative to the example directory cd example echo "Checking alias paths..." for path in "../packages/react-native-youtube-bridge/src/index" "../packages/core/src/index" "../packages/react/src/index"; do if [ -f "$path.ts" ] || [ -f "$path.tsx" ] || [ -f "$path.js" ]; then echo "✓ Found: $path" else echo "✗ Missing: $path" fi donepackages/core/src/types/webview.ts (2)
1-1: Import path simplification improves package organization.The change from
'./youtube'to'./'indicates better type organization within the core package, likely importing from a centralized index file. This aligns well with the monorepo restructuring goals.
42-50: MessageData union type is well-structured for cross-platform communication.The MessageData type union properly covers all webview communication scenarios and maintains type safety across different message types. This is appropriate for the core package as it provides shared communication contracts.
example/metro.config.js (1)
39-39: LGTM! Metro config simplified for monorepo.The removal of the explicit options object aligns with the monorepo restructuring. The
packageNameparameter is no longer needed since module resolution will be handled through the workspace configuration.packages/react/src/hooks/useYoutubePlayer.ts (2)
1-7: LGTM! Clean import restructuring for monorepo.The imports have been properly updated to use the centralized
@react-native-youtube-bridge/corepackage, which improves modularity and code organization.
11-11: LGTM! Consistent class naming throughout.The class name has been consistently updated from
YouTubePlayerCoretoYoutubePlayerCoreacross all references, maintaining consistency with the core package's naming convention.Also applies to: 38-38, 62-62
packages/web/src/hooks/useWebView.ts (3)
1-1: LGTM! Import updated for monorepo structure.The
MessageDataimport has been correctly updated to use the centralized core package.
4-4: LGTM! Import path corrected.The import path has been corrected from
'../types/messages'to'../types/message', which likely reflects a file rename or consolidation.
16-16: LGTM! Simplified WebView detection.The direct check for
window.ReactNativeWebViewremoves unnecessary abstraction and makes the code more straightforward and readable.Also applies to: 62-62
packages/core/src/YoutubePlayerCore.ts (3)
1-4: LGTM! Clean relative imports within the package.The imports have been properly updated to use relative paths within the core package, improving modularity and maintainability.
6-8: LGTM! Consistent type and class naming.The type alias has been updated to use
YoutubePlayerConfigand the class has been consistently renamed toYoutubePlayerCore, maintaining naming consistency across the monorepo.Also applies to: 10-10
375-375: LGTM! Export updated to match class name.The default export has been correctly updated to match the renamed class.
packages/core/package.json (1)
1-47: LGTM! Well-structured core package configuration.The package.json is properly configured for the monorepo's core package with:
- Appropriate entry points and exports for both ESM and CommonJS
- Private package flag for internal use
- Clean build and type-check scripts
- Comprehensive metadata and repository information
The configuration supports the modular architecture goals of the monorepo migration.
web/src/App.tsx (1)
1-7: Excellent simplification aligned with the monorepo architecture!The refactoring effectively delegates YouTube player logic to the dedicated web package, following the single responsibility principle.
packages/react-native-youtube-bridge/src/types/message.ts (1)
1-34: Well-structured type definitions for YouTube player messagingThe union types provide excellent type safety for message handling and command execution in the React Native bridge.
packages/web/package.json (1)
1-57: Well-configured web package for the monorepoThe package setup correctly uses workspace dependencies and appropriate build tooling with Vite. The entry points are properly configured for both CommonJS and ES module consumers.
packages/core/src/types/iframe.ts (1)
1-6: Good architectural decision removing global augmentationMoving from global
Windowinterface augmentation to an explicitIframeApiTypeinterface improves type safety and prevents global namespace pollution.packages/react-native-youtube-bridge/src/index.tsx (1)
1-13: LGTM! Clean refactoring to centralize types in core package.The refactoring to import types from
@react-native-youtube-bridge/coreinstead of local modules is well-executed. This centralization improves code organization and reusability across the monorepo.packages/core/src/index.ts (1)
1-17: Excellent core package structure with well-organized exports.The export organization is clean and logical:
- Types are grouped together (lines 1-12)
- WebView-specific types are separate (line 13)
- Core class, utilities, and constants are clearly defined (lines 14-16)
This structure will make the core package easy to consume and maintain.
packages/react-native-youtube-bridge/README.md (1)
1-274: Outstanding comprehensive documentation with excellent examples.This README provides thorough coverage of all library features:
- Clear installation and basic usage instructions
- Comprehensive event handling examples with proper
useCallbackusage- Detailed player control examples
- Configuration options for parameters and styling
- Advanced rendering modes and troubleshooting guidance
The documentation quality is exceptional and will greatly help users adopt the library.
packages/react-native-youtube-bridge/CHANGELOG.md (1)
1-92: Well-structured changelog with comprehensive version history.The changelog effectively documents the library's evolution with clear categorization of changes (breaking changes, features, patches) and good detail level.
packages/react-native-youtube-bridge/README-ko_kr.md (1)
1-274: Excellent Korean documentation with comprehensive coverage.The Korean README provides thorough translation of all features and examples, maintaining the same high quality as the English version. This will greatly help Korean-speaking developers adopt the library.
packages/web/src/YoutubePlayer.tsx (2)
27-89: Well-structured player configuration with proper event handling.The YouTube player configuration correctly uses the imported hooks and properly handles all events by sending messages through the WebView interface.
177-188: Clean component structure.The render method is simple and properly uses the ref from the YouTube player hook.
packages/react-native-youtube-bridge/src/types/youtube.ts (1)
1-50: Well-structured type definitions with comprehensive documentation.The
YoutubePlayerPropstype is properly defined with:
- Clear JSDoc comments explaining each property
- Platform-specific annotations for native-only properties
- Proper type imports from the core package
- Thoughtful property constraints (e.g., WebViewProps exclusions)
packages/react-native-youtube-bridge/package.json (1)
1-108: Properly configured package for the monorepo structure.The package.json correctly:
- Uses workspace dependencies for internal packages
- Defines appropriate peer dependency version ranges
- Configures react-native-builder-bob for building
- Includes all necessary files for distribution
package.json (2)
14-14: Verify the change frombiome checktobiome format.The lint script was changed from
biome check --write .tobiome format packages/** --write. This changes the behavior from running both linting and formatting to only formatting.Was this change intentional? If you want to maintain both linting and formatting, consider:
-"lint": "biome format packages/** --write", +"lint": "biome check packages/** --write",Or add separate scripts:
"lint": "biome format packages/** --write", +"lint:check": "biome check packages/**", +"format": "biome format packages/** --write",
2-29: Successful monorepo structure transformation.The package.json has been properly converted to a monorepo root with:
- Private flag to prevent accidental publishing
- Workspace configuration for package management
- Workspace-filtered scripts for building and type checking
- Removal of package-specific fields
packages/core/src/types/index.ts (2)
1-26: Excellent use of const assertions for type-safe error handling.The error types are well-defined with:
- Strong typing using the ERROR_CODES constant
- Clear type aliases for code and message
- Proper JSDoc documentation with reference links
27-100: Comprehensive type definitions for the YouTube player core.The type definitions are well-structured with:
- Clear enum for PlayerState
- Flexible YouTubeSource type supporting multiple input formats
- Detailed JSDoc comments explaining browser policy considerations
- Optional properties with sensible defaults
| glob: "*.{js,ts, jsx, tsx}" | ||
| run: npx tsc | ||
| run: bun run typecheck |
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 glob pattern spacing in types hook
The glob pattern "*.{js,ts, jsx, tsx}" contains spaces that will prevent matching .jsx and .tsx. Align it with the lint glob: "*.{js,ts,jsx,tsx}".
🤖 Prompt for AI Agents
In lefthook.yml at lines 8 to 9, the glob pattern for the `types` hook includes
spaces inside the braces, which breaks matching for `.jsx` and `.tsx` files.
Remove the spaces so the pattern reads `"*.{js,ts,jsx,tsx}"` to correctly match
all intended file extensions.
| "jsx": "react-native", | ||
| "lib": ["DOM", "DOM.Iterable", "ESNext"] | ||
| }, | ||
| "include": ["src/*"] |
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
Include pattern may be too restrictive.
"include": ["src/*"] only matches files directly under src/. To pick up nested directories, change it to:
- "include": ["src/*"]
+ "include": ["src/**/*"]📝 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.
| "include": ["src/*"] | |
| "include": ["src/**/*"] |
🤖 Prompt for AI Agents
In packages/react-native-youtube-bridge/tsconfig.json at line 7, the include
pattern "src/*" only matches files directly inside the src directory and misses
files in nested subdirectories. Change the include pattern to "src/**/*" to
recursively include all files in src and its subdirectories.
| } | ||
|
|
||
| window.onYouTubeIframeAPIReady = () => { | ||
| window.window.onYouTubeIframeAPIReady = () => { |
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 the duplicate window reference.
There appears to be a typo with the double window reference. This should likely be window.onYouTubeIframeAPIReady instead of window.window.onYouTubeIframeAPIReady.
- window.window.onYouTubeIframeAPIReady = () => {
+ window.onYouTubeIframeAPIReady = () => {📝 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.
| window.window.onYouTubeIframeAPIReady = () => { | |
| window.onYouTubeIframeAPIReady = () => { |
🤖 Prompt for AI Agents
In packages/core/src/YoutubePlayerCore.ts at line 52, fix the typo by removing
the duplicate window reference. Change `window.window.onYouTubeIframeAPIReady`
to `window.onYouTubeIframeAPIReady` to correctly assign the callback on the
global window object.
| "name": "@react-native-youtube-bridge/react", | ||
| "version": "0.0.1", | ||
| "description": "React implementation for react-native-youtube-bridge", | ||
| "private": true, |
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.
Conflicting package privacy settings
The package is marked as "private": true but has publishConfig.access: "public". If this package is intended for public npm publishing, remove the "private": true field.
- "private": true,Also applies to: 41-43
🤖 Prompt for AI Agents
In packages/react/package.json at lines 5 and 41-43, the package is marked as
"private": true but also has publishConfig.access set to "public", which is
conflicting. To fix this, remove the "private": true field if the package is
intended to be published publicly on npm, ensuring the privacy settings are
consistent with the publishConfig.
| if (message.id && window.ReactNativeWebView) { | ||
| window.ReactNativeWebView.postMessage( | ||
| JSON.stringify({ | ||
| type: 'commandResult', | ||
| id: message.id, | ||
| result, | ||
| }), | ||
| ); | ||
| } |
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.
Use sendMessage for consistency.
-if (message.id && window.ReactNativeWebView) {
- window.ReactNativeWebView.postMessage(
- JSON.stringify({
- type: 'commandResult',
- id: message.id,
- result,
- }),
- );
-}
+if (message.id) {
+ sendMessage({
+ type: 'commandResult',
+ id: message.id,
+ result,
+ });
+}📝 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.
| if (message.id && window.ReactNativeWebView) { | |
| window.ReactNativeWebView.postMessage( | |
| JSON.stringify({ | |
| type: 'commandResult', | |
| id: message.id, | |
| result, | |
| }), | |
| ); | |
| } | |
| if (message.id) { | |
| sendMessage({ | |
| type: 'commandResult', | |
| id: message.id, | |
| result, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In packages/web/src/YoutubePlayer.tsx around lines 164 to 172, the code uses
window.ReactNativeWebView.postMessage to send messages, but for consistency with
the rest of the codebase, replace this call with
window.ReactNativeWebView.sendMessage instead. Update the method call to use
sendMessage while keeping the message payload and structure unchanged.
| if (message.id && window.ReactNativeWebView) { | ||
| window.ReactNativeWebView.postMessage( | ||
| JSON.stringify({ | ||
| type: 'error', | ||
| id: message.id, | ||
| error: { code: -4, message: `Command not found: ${message.command}` }, | ||
| }), | ||
| ); | ||
| } | ||
| return; | ||
| } |
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.
Use the sendMessage function instead of direct WebView access.
The code directly accesses window.ReactNativeWebView to send error messages, which is inconsistent with the sendMessage function provided by useWebView hook. This violates the abstraction layer.
Replace direct WebView access with the sendMessage function:
-if (message.id && window.ReactNativeWebView) {
- window.ReactNativeWebView.postMessage(
- JSON.stringify({
- type: 'error',
- id: message.id,
- error: { code: -4, message: `Command not found: ${message.command}` },
- }),
- );
-}
+if (message.id) {
+ sendMessage({
+ type: 'error',
+ id: message.id,
+ error: { code: -4, message: `Command not found: ${message.command}` },
+ });
+}📝 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.
| if (message.id && window.ReactNativeWebView) { | |
| window.ReactNativeWebView.postMessage( | |
| JSON.stringify({ | |
| type: 'error', | |
| id: message.id, | |
| error: { code: -4, message: `Command not found: ${message.command}` }, | |
| }), | |
| ); | |
| } | |
| return; | |
| } | |
| if (message.id) { | |
| sendMessage({ | |
| type: 'error', | |
| id: message.id, | |
| error: { code: -4, message: `Command not found: ${message.command}` }, | |
| }); | |
| } | |
| return; | |
| } |
🤖 Prompt for AI Agents
In packages/web/src/YoutubePlayer.tsx around lines 112 to 122, the code directly
accesses window.ReactNativeWebView to post error messages, bypassing the
abstraction provided by the sendMessage function from the useWebView hook. To
fix this, replace the direct window.ReactNativeWebView.postMessage call with a
call to sendMessage, passing the same error message object serialized as JSON.
This ensures consistent use of the abstraction layer for sending messages.
| if (message.id && window.ReactNativeWebView) { | ||
| window.ReactNativeWebView.postMessage( | ||
| JSON.stringify({ | ||
| type: 'commandResult', | ||
| id: message.id, | ||
| result: r, | ||
| }), | ||
| ); | ||
| } | ||
| }) | ||
| .catch((err) => { | ||
| if (message.id && window.ReactNativeWebView) { | ||
| window.ReactNativeWebView.postMessage( | ||
| JSON.stringify({ | ||
| type: 'error', | ||
| id: message.id, | ||
| error: { code: -5, message: err?.message || String(err) }, | ||
| }), | ||
| ); | ||
| } | ||
| }); |
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 message sending to use the sendMessage function consistently.
Similar to the previous comment, these promise handlers should use the sendMessage function for consistency.
Apply this pattern throughout the promise handlers:
-if (message.id && window.ReactNativeWebView) {
- window.ReactNativeWebView.postMessage(
- JSON.stringify({
- type: 'commandResult',
- id: message.id,
- result: r,
- }),
- );
-}
+if (message.id) {
+ sendMessage({
+ type: 'commandResult',
+ id: message.id,
+ result: r,
+ });
+}And for the error case:
-if (message.id && window.ReactNativeWebView) {
- window.ReactNativeWebView.postMessage(
- JSON.stringify({
- type: 'error',
- id: message.id,
- error: { code: -5, message: err?.message || String(err) },
- }),
- );
-}
+if (message.id) {
+ sendMessage({
+ type: 'error',
+ id: message.id,
+ error: { code: -5, message: err?.message || String(err) },
+ });
+}📝 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.
| if (message.id && window.ReactNativeWebView) { | |
| window.ReactNativeWebView.postMessage( | |
| JSON.stringify({ | |
| type: 'commandResult', | |
| id: message.id, | |
| result: r, | |
| }), | |
| ); | |
| } | |
| }) | |
| .catch((err) => { | |
| if (message.id && window.ReactNativeWebView) { | |
| window.ReactNativeWebView.postMessage( | |
| JSON.stringify({ | |
| type: 'error', | |
| id: message.id, | |
| error: { code: -5, message: err?.message || String(err) }, | |
| }), | |
| ); | |
| } | |
| }); | |
| .then((r) => { | |
| if (message.id) { | |
| sendMessage({ | |
| type: 'commandResult', | |
| id: message.id, | |
| result: r, | |
| }); | |
| } | |
| }) | |
| .catch((err) => { | |
| if (message.id) { | |
| sendMessage({ | |
| type: 'error', | |
| id: message.id, | |
| error: { code: -5, message: err?.message || String(err) }, | |
| }); | |
| } | |
| }); |
🤖 Prompt for AI Agents
In packages/web/src/YoutubePlayer.tsx between lines 140 and 160, the code
directly calls window.ReactNativeWebView.postMessage to send messages in promise
handlers. Refactor these handlers to use the existing sendMessage function
instead, passing the appropriate message object to it. This ensures consistent
message sending throughout the file.
Summary by CodeRabbit
New Features
Documentation
Refactor
Bug Fixes
Chores