-
Notifications
You must be signed in to change notification settings - Fork 1
fix: migrate package manager bun to pnpm #46
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
|
Deploying react-native-youtube-bridge-example with
|
| Latest commit: |
95253bf
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://090ce168.react-native-youtube-bridge-example.pages.dev |
| Branch Preview URL: | https://fix-migrate-pnpm.react-native-youtube-bridge-example.pages.dev |
|
Warning Rate limit exceeded@saseungmin has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 1 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis update migrates the project's tooling and workflows from the Bun package manager to pnpm. All continuous integration, release, and local scripts now use pnpm commands. Supporting configuration files were added or updated, and custom module resolution or path aliases in the example app were removed. Node version constraints and caching strategies were also adjusted. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant Node.js Setup
participant pnpm Setup
participant pnpm Cache
participant Project
GitHub Actions->>Node.js Setup: Setup Node.js (from .nvmrc)
Node.js Setup->>pnpm Setup: Setup pnpm (v10.11.0)
pnpm Setup->>pnpm Cache: Restore pnpm cache (using pnpm-lock.yaml)
pnpm Cache-->>pnpm Setup: Cache hit/miss
pnpm Setup->>Project: Install dependencies (pnpm install)
Project->>GitHub Actions: Run build/lint/test scripts (pnpm run ...)
Possibly related PRs
Poem
🪧 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 (
|
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: 4
🧹 Nitpick comments (7)
packages/react/package.json (1)
15-15:package.jsonis redundant in thefilesarray – consider trimming for cleanliness
package.jsonis always shipped by npm even when thefileswhitelist is present, so listing it explicitly is unnecessary noise. Dropping it keeps the manifest minimal and avoids future drift if the array is copy-pasted elsewhere.- "files": ["dist", "package.json"], + "files": ["dist"],packages/core/package.json (1)
15-15: Same redundancy infilesarrayAs in
packages/react,package.jsoncan be removed.- "files": ["dist", "package.json"], + "files": ["dist"],example/package.json (1)
10-11: Minor: make the build output explicit for easier artifact handling
expo exportdefaults toweb-build. Declaring the directory up-front avoids surprises and makes it simpler to upload artefacts in CI.-"web:build": "expo export --platform web --clear" +"web:build": "expo export --platform web --output-dir dist-web --clear"package.json (2)
8-8: Quote the glob in theclearscriptUnquoted
**/distmay be expanded by the shell before it reachesrimraf, especially on zsh/fish.-"clear": "rimraf --glob **/dist", +"clear": "rimraf --glob \"**/dist\"",
9-10: Reduce duplication with pnpm’s recursive modeHard-coding every package makes maintenance noisy. A single recursive run is shorter and automatically picks up new workspaces.
-"typecheck": "pnpm --filter @react-native-youtube-bridge/core typecheck && pnpm --filter @react-native-youtube-bridge/react typecheck && pnpm --filter @react-native-youtube-bridge/web typecheck && pnpm --filter react-native-youtube-bridge typecheck", -"build": "pnpm --filter @react-native-youtube-bridge/core build && pnpm --filter @react-native-youtube-bridge/react build && pnpm --filter @react-native-youtube-bridge/web build && pnpm --filter react-native-youtube-bridge build", +"typecheck": "pnpm -r run typecheck", +"build": "pnpm -r run build",.github/workflows/ci.yml (1)
28-30: Duplicate build—costly on CI timeCore and React packages are built again in the
build-libraryjob. Unless linting depends on the compiled artefacts, consider removing these two lines or running a single recursive build to save minutes..github/actions/setup/action.yml (1)
25-29: Guard against whitespace when exporting the store pathFor robustness, capture the store path in JSON mode and strip quotes:
-echo "STORE_PATH=$(pnpm store path --silent)" >> $GITHUB_ENV +echo "STORE_PATH=$(pnpm store path --silent --json | jq -r '.')" >> $GITHUB_ENV
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
bun.lockis excluded by!**/*.lockexample/bun.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
.github/actions/setup/action.yml(2 hunks).github/workflows/ci.yml(4 hunks).github/workflows/release.yml(1 hunks).gitignore(1 hunks).nvmrc(1 hunks)example/babel.config.js(0 hunks)example/package.json(2 hunks)example/tsconfig.json(1 hunks)lefthook.yml(1 hunks)package.json(2 hunks)packages/core/package.json(1 hunks)packages/react-native-youtube-bridge/package.json(1 hunks)packages/react/package.json(1 hunks)packages/web/package.json(1 hunks)pnpm-workspace.yaml(1 hunks)
💤 Files with no reviewable changes (1)
- example/babel.config.js
🧰 Additional context used
🪛 GitHub Actions: CI
packages/react-native-youtube-bridge/package.json
[warning] 1-1: Invalid package.json configuration: 'exports' resolution points to a non-existent file '/lib/module/index.js'. Consider fixing the package configuration.
package.json
[warning] 1-1: Invalid package.json configuration: 'exports' resolution points to a non-existent file '/lib/module/index.js'. Consider fixing the package configuration.
🔇 Additional comments (9)
.gitignore (1)
86-88: 👍 Good call adding the coverage directory to .gitignorePrevents large LCOV/HTML artefacts from creeping into the repo.
No further action required..nvmrc (1)
1-1: Broadening to the whole 22.x line—verify CI & toolchain compatibilityMoving from an exact patch (
v22.16.0) to just22gives flexibility but may introduce subtle breakages when Node 22 receives non-LTS updates. Make sure:
- CI runners install
22(not an older cache).- All dependencies (e.g.
react-native,pnpm@10) are already tested on 22.x.- Contributors are aware a non-LTS version is in use.
If stability is a concern, pin to the latest audited patch instead.
packages/react-native-youtube-bridge/package.json (1)
83-83: ```shell
#!/bin/bash
set -eecho "🔍 Directories in packages/react-native-youtube-bridge:"
find packages/react-native-youtube-bridge -maxdepth 2 -type d || echo "❌ no directories found"echo ""
echo "🔍 Listing files under packages/react-native-youtube-bridge/lib (if any):"
find packages/react-native-youtube-bridge/lib -type f || echo "❌ lib directory not present"echo ""
echo "🔍 'exports' field in package.json:"
grep -R -n '"exports"' -n packages/react-native-youtube-bridge/package.json || echo "❌ exports not found"echo ""
echo "🔍 Snippet around 'exports' (lines 60–120):"
sed -n '60,120p' packages/react-native-youtube-bridge/package.json</details> <details> <summary>packages/web/package.json (1)</summary> `11-11`: **Minor style change looks good** Array rewritten to single-line – no functional impact. </details> <details> <summary>lefthook.yml (1)</summary> `6-9`: **Hook commands correctly migrated to pnpm** Commands now call `pnpm run lint` / `typecheck`; glob typo fixed. Confirm that the scripts exist in the root `package.json`, otherwise the hooks will fail. </details> <details> <summary>example/tsconfig.json (1)</summary> `2-2`: I’ll try running the workspace’s TypeScript compiler against the example config: ```shell #!/bin/bash pnpm exec tsc --noEmit -p example/tsconfig.json.github/workflows/release.yml (1)
30-33: Confirm custom commands passed tochangesets/action
version: "pnpm changeset version"andpublish: "pnpm changeset publish"override defaults.
Double-check that these pnpm scripts exist (changeset version,changeset publish) or the action will exit with non-zero status.If you intend to call the binary directly, you can drop the quotes:
version: pnpm changeset version publish: pnpm changeset publishexample/package.json (1)
21-22: Verify that the workspace protocol actually resolves
"react-native-youtube-bridge": "workspace:*"will only install if
pnpm-workspace.yamlcontains theexamplepackage and- the
react-native-youtube-bridgepackage’sversionsatisfies the*selector.Otherwise
pnpmaborts with “No matching version found”. Double-check before merging..github/workflows/ci.yml (1)
68-68: Forward the argument after fixing the root scriptOnce the root
examplescript is corrected topnpm --filter example run, update the call here to pass the sub-script via--.-run: pnpm run example web:build +run: pnpm run example -- web:build
Deploying react-native-youtube-bridge with
|
| Latest commit: |
d0c74a0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3eac5b3f.react-native-youtube-bridge.pages.dev |
| Branch Preview URL: | https://fix-migrate-pnpm.react-native-youtube-bridge.pages.dev |
Summary by CodeRabbit