[NOJIRA] [FIX] [V2] Session Replay build issues#1276
[NOJIRA] [FIX] [V2] Session Replay build issues#1276cdn34dd wants to merge 4 commits intofeature/v2from
Conversation
There was a problem hiding this comment.
Pull request overview
Addresses build stability issues for the Session Replay package by updating iOS CocoaPods configuration for newer React Native versions and preventing Metro/Node processes from hanging after bundling completes.
Changes:
- Refactors the Session Replay podspec to remove Folly-specific flags and restructure
install_modules_dependencies/ xcconfig handling for newer RN. - Updates the Metro asset bundler to manage a single chokidar watcher instance and close it after bundling-related reporter events to allow Node to exit.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/react-native-session-replay/src/metro/index.ts | Replaces boolean “watching” guard with a chokidar watcher instance and adds logic to close it after Metro reporter events. |
| packages/react-native-session-replay/DatadogSDKReactNativeSessionReplay.podspec | Simplifies new-arch compiler flags and reorganizes pod_target_xcconfig + install_modules_dependencies handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| mergeSvgAssets(assetsDir); | ||
|
|
There was a problem hiding this comment.
The watcher is closed not only on bundle_build_done but also on transformer_load_done. transformer_load_done can fire early (before assets are added / before bundling finishes), which can stop the chokidar watcher prematurely and cause newly added SVGs to never be merged. Consider only closing the watcher on bundle_build_done (or otherwise gating the close to one-off bundle/CI builds).
| mergeSvgAssets(assetsDir); | |
| mergeSvgAssets(assetsDir); | |
| } | |
| if (event.type === 'bundle_build_done') { |
| // Close the file watcher after bundling completes | ||
| // This prevents the chokidar watcher from keeping the | ||
| // Node process alive during release/CI builds | ||
| if (watcher) { | ||
| watcher.close(); | ||
| watcher = null; | ||
| } |
There was a problem hiding this comment.
There’s no automated coverage around the new lifecycle behavior (closing the chokidar watcher after a Metro reporter event). Since this affects build stability and can differ between dev server vs one-off bundling, adding a unit test that mocks chokidar and asserts when close() is (and is not) called would help prevent regressions.
What does this PR do?
This PR does two things:
Additional Notes
Tested on an specific Expo 54.0.33 RN 0.81.5 test app.
Review checklist (to be filled by reviewers)