Add bundle analysis workflow #802
Conversation
WalkthroughThis change introduces a new automated workflow for analyzing React Native bundle sizes for Android and iOS. It adds platform-specific analysis scripts, a Node.js CLI utility to enforce bundle size thresholds, a corresponding test suite, and a GitHub Actions workflow to run these checks on pull requests and upload bundle reports as artifacts. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant Runner (macOS)
participant CLI Script
participant Bundle Visualizer
participant File System
GitHub Actions->>Runner (macOS): Trigger workflow (PR or manual)
Runner (macOS)->>Runner (macOS): Checkout repo, install deps
Runner (macOS)->>CLI Script: Run analyze:android:ci or analyze:ios:ci
CLI Script->>Bundle Visualizer: Execute with platform argument
Bundle Visualizer->>File System: Generate bundle file
CLI Script->>File System: Check bundle file exists, get size
CLI Script->>Runner (macOS): Log result, exit success/failure
Runner (macOS)->>GitHub Actions: Upload bundle report artifact
Estimated code review effort🎯 2 (Simple) | ⏱️ ~9 minutes Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✨ 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. 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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/bundle-analysis.yml (2)
18-57: Reduce duplication with a matrix & add concurrency guardBoth jobs are identical minus the platform. A matrix keeps the file concise, makes future tweaks one-liner changes, and shares caching between runs.
At the same time, heavy bundle analysis should cancel stale runs on the same branch to save CI minutes.-jobs: - analyze-android: - runs-on: macos-14 - steps: - - uses: actions/checkout@v4 - - name: Install Mobile Dependencies - uses: ./.github/actions/mobile-setup - with: - app_path: ${{ env.APP_PATH }} - node_version: ${{ env.NODE_VERSION }} - ruby_version: ${{ env.RUBY_VERSION }} - workspace: ${{ env.WORKSPACE }} - - name: Run Android analysis - run: yarn analyze:android - working-directory: ./app - - name: Upload Android bundle report - uses: actions/upload-artifact@v4 - with: - name: android-bundle-${{ github.sha }} - path: /tmp/react-native-bundle-visualizer/OpenPassport/output/explorer.html - - analyze-ios: - runs-on: macos-14 - steps: - - uses: actions/checkout@v4 - - name: Install Mobile Dependencies - uses: ./.github/actions/mobile-setup - with: - app_path: ${{ env.APP_PATH }} - node_version: ${{ env.NODE_VERSION }} - ruby_version: ${{ env.RUBY_VERSION }} - workspace: ${{ env.WORKSPACE }} - - name: Run iOS analysis - run: yarn analyze:ios - working-directory: ./app - - name: Upload iOS bundle report - uses: actions/upload-artifact@v4 - with: - name: ios-bundle-${{ github.sha }} - path: /tmp/react-native-bundle-visualizer/OpenPassport/output/explorer.html +jobs: + analyze: + runs-on: macos-14 + strategy: + matrix: + platform: [android, ios] + concurrency: + group: bundle-${{ github.ref }}-${{ matrix.platform }} + cancel-in-progress: true + steps: + - uses: actions/checkout@v4 + - name: Install Mobile Dependencies + uses: ./.github/actions/mobile-setup + with: + app_path: ${{ env.APP_PATH }} + node_version: ${{ env.NODE_VERSION }} + ruby_version: ${{ env.RUBY_VERSION }} + workspace: ${{ env.WORKSPACE }} + - name: Run ${{ matrix.platform }} analysis + run: yarn analyze:${{ matrix.platform }} + working-directory: ./app + - name: Upload ${{ matrix.platform }} bundle report + uses: actions/upload-artifact@v4 + with: + name: ${{ matrix.platform }}-bundle-${{ github.sha }} + path: /tmp/react-native-bundle-visualizer/OpenPassport/output/explorer.html
30-33: Harden against missing scriptsIf
yarn analyze:android/iosis absent or renamed, the workflow bails out late. Add--if-presentto fail softly and surface a clearer log, or enforce presence viayarn workspaces infobefore the run.- run: yarn analyze:${{ matrix.platform }} + run: yarn analyze:${{ matrix.platform }} --if-presentAlternatively, insert a script-existence assertion step first.
Also applies to: 51-52
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/bundle-analysis.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
.github/workflows/bundle-analysis.yml (1)
Learnt from: transphorm
PR: #795
File: app/android/app/build.gradle:157-158
Timestamp: 2025-07-29T01:08:28.530Z
Learning: For this React Native project, the team prefers build flexibility over fail-fast behavior for release builds in app/android/app/build.gradle. They intentionally allow fallback to debug signing for local development runs, relying on Google Play Console validation to catch any improperly signed releases during upload.
🔇 Additional comments (1)
.github/workflows/bundle-analysis.yml (1)
3-8: Verify Node version usage & remove unused JAVA_VERSIONI didn’t find any references to
JAVA_VERSIONorNODE_VERSIONin the workflow steps:•
.github/workflows/bundle-analysis.ymlenv block declares
NODE_VERSION: 18JAVA_VERSION: 17• Ripgrep checks show
- No
JAVA_VERSIONusages anywhere in the repo- No
actions/setup-nodeornode-versionsettings in this workflow- No interpolations of
${{ env.NODE_VERSION }}Please confirm how you’re selecting or installing Node in this job. If you do rely on
NODE_VERSION, update it to 20 for active-LTS. Otherwise, drop the entireNODE_VERSIONentry. In either case,JAVA_VERSIONcan be removed as dead weight.
transphorm
force-pushed
the
codex/add-github-workflows-for-yarn-commands
branch
from
905166e to
61db0f3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/scripts/bundle-analyze-ci.cjs (3)
13-13: Enhance number parsing robustness.Consider validating the parsed number to handle invalid environment variable values gracefully.
-const warning = Number(process.env.BUNDLE_WARNING_INCREASE || '0'); +const warning = parseInt(process.env.BUNDLE_WARNING_INCREASE || '0', 10) || 0;This ensures the warning threshold is always a valid integer, even if the environment variable contains invalid data.
15-33: Good defensive programming with room for improvement.The utility functions handle edge cases well. Consider these enhancements for better robustness:
function sanitize(str) { - return str ? str.replace(/[^\w]/g, '') : str; + return str ? str.replace(/[^\w-]/g, '') : str; }Allow hyphens in app names as they're common and safe for file paths.
- return sanitize(appJson.name || (appJson.expo && appJson.expo.name)); + return sanitize(appJson.name || appJson.expo?.name);Use optional chaining for safer property access (requires Node.js 14+).
41-43: Command execution is well-implemented.Good use of
stdio: 'inherit'for CI visibility. The command construction is safe given the platform validation.Consider adding platform validation for extra security:
+// Validate platform is one of expected values +if (!['android', 'ios'].includes(platform)) { + console.error('Platform must be either "android" or "ios"'); + process.exit(1); +} + execSync(`react-native-bundle-visualizer --platform ${platform} --dev`, {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/bundle-analysis.yml(1 hunks)app/package.json(1 hunks)app/scripts/bundle-analyze-ci.cjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/package.json
- .github/workflows/bundle-analysis.yml
🧰 Additional context used
🪛 GitHub Check: lint
app/scripts/bundle-analyze-ci.cjs
[warning] 49-49:
Unexpected console statement
[warning] 9-9:
Unexpected console statement
🔇 Additional comments (4)
app/scripts/bundle-analyze-ci.cjs (4)
1-5: LGTM! Clean module imports and proper shebang.The Node.js shebang and standard library imports are appropriate for this CI script's functionality.
7-11: Platform validation looks good.The argument validation is robust with clear error messaging. The
console.errorusage is appropriate for CLI scripts despite static analysis warnings.
35-39: Solid file handling and path construction.Good use of OS temp directory and defensive file existence checking. The path isolation by app name prevents conflicts between different projects.
45-53: Excellent bundle size monitoring implementation.The size comparison logic is thorough and provides valuable CI feedback. The warning message is informative with good visual indicators. Console usage is appropriate for CLI scripts despite static analysis flags.
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
app/scripts/bundle-analyze-ci.cjs (1)
23-37: Add error logging for configuration file issues.While the try-catch blocks handle errors gracefully, silent failures make debugging difficult in CI environments. Consider adding debug logging.
function getAppName() { try { const pkg = JSON.parse( fs.readFileSync(path.join(__dirname, '..', 'package.json'), 'utf8'), ); if (pkg.name) return sanitize(pkg.name); - } catch {} + } catch (error) { + console.warn('Warning: Could not read package.json:', error.message); + } try { const appJson = JSON.parse( fs.readFileSync(path.join(__dirname, '..', 'app.json'), 'utf8'), ); return sanitize(appJson.name || (appJson.expo && appJson.expo.name)); - } catch {} + } catch (error) { + console.warn('Warning: Could not read app.json:', error.message); + } return 'UnknownApp'; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/mobile-bundle-analysis.yml(1 hunks)app/scripts/bundle-analyze-ci.cjs(1 hunks)app/scripts/tests/bundle-analyze-ci.test.cjs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/mobile-bundle-analysis.yml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: transphorm
PR: selfxyz/self#795
File: app/android/app/build.gradle:157-158
Timestamp: 2025-07-29T01:08:28.530Z
Learning: For this React Native project, the team prefers build flexibility over fail-fast behavior for release builds in app/android/app/build.gradle. They intentionally allow fallback to debug signing for local development runs, relying on Google Play Console validation to catch any improperly signed releases during upload.
Learnt from: transphorm
PR: selfxyz/self#636
File: app/android/app/build.gradle:76-76
Timestamp: 2025-06-30T15:51:11.193Z
Learning: For React Native 0.80, the correct JSC version is `io.github.react-native-community:jsc-android:2026004.+`. This version was specifically built with Android NDK r23 to align with React Native 0.80's NDK requirements and prevent compatibility issues. The `2026004` version pattern is the official version for RN 0.80, not an outdated pin.
🪛 GitHub Check: lint
app/scripts/bundle-analyze-ci.cjs
[warning] 69-69:
Unexpected console statement
[warning] 63-63:
Unexpected console statement
[warning] 62-62:
Unexpected console statement
[warning] 61-61:
Unexpected console statement
[warning] 60-60:
Unexpected console statement
[warning] 57-57:
Unexpected console statement
[warning] 51-51:
Unexpected console statement
[warning] 50-50:
Unexpected console statement
[warning] 46-46:
'platform' is already declared in the upper scope on line 7 column 7
[warning] 9-9:
Unexpected console statement
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: analyze-android
- GitHub Check: build
🔇 Additional comments (6)
app/scripts/bundle-analyze-ci.cjs (1)
9-9: Console statements are appropriate for CLI tools.The static analysis warnings about console statements can be ignored here since this is a CLI tool where console output is expected and necessary for user feedback.
Also applies to: 50-64, 69-69
app/scripts/tests/bundle-analyze-ci.test.cjs (5)
11-15: Good coverage for missing platform argument.The test correctly validates that the script exits with an error when no platform is provided. This is essential for CLI argument validation.
45-49: Excellent error handling validation.The test properly validates that the script handles missing bundle files gracefully with appropriate error messages and exit codes.
51-58: Good user experience validation.Testing for helpful error messages ensures developers understand how to resolve threshold issues. This promotes maintainability.
60-65: Comprehensive function validation.The test ensures the DRY principle is followed with proper return values for the checkBundleSize function. This validates the refactoring from previous implementations.
17-21: Verify platform‐validation in the CLIThe test in app/scripts/tests/bundle-analyze-ci.test.cjs (lines 17–21) assumes an invalid platform argument will cause the script to exit with an error. However, we haven’t been able to locate any code in the CLI that explicitly validates or rejects unrecognized platform strings. Please confirm:
- That the
scriptPathin the test points to the correct entrypoint for your bundle-analyze-ci tool.- Whether the implementation maintains a whitelist of valid platforms (e.g.
['ios', 'android', 'web']) and throws or callsprocess.exit(1)on anything else.- If no such validation exists, either add a guard in the CLI (and tests for each supported platform) or adjust the test to reflect the real behavior.
Summary
Testing
yarn lintyarn buildyarn workspace @selfxyz/contracts build(fails: Hardhat config error)yarn typesyarn workspace @selfxyz/common testyarn workspace @selfxyz/circuits test(fails: circom missing)yarn workspace @selfxyz/mobile-app testhttps://chatgpt.com/codex/tasks/task_b_68886317be64832da01d88b60982e9da
Summary by CodeRabbit