-
Notifications
You must be signed in to change notification settings - Fork 5
feat: expose loadMetroConfig & updateManifest helpers
#90
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
…act-native-community/cli dependency
b475569 to
abfe822
Compare
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.
Pull Request Overview
This PR integrates Zephyr monitoring/observability platform with Metro Module Federation and React Native Components (RNC). The integration includes local development dependencies, wrapper commands for build processes, and configuration updates across multiple applications.
Key changes:
- Added local npm links for Zephyr packages using portal dependencies
- Introduced
zephyrCommandWrapperto wrap existing Metro Federation commands - Updated Metro configurations to use async
withZephyr()wrapper - Added new CLI commands and manifest update functionality
Reviewed Changes
Copilot reviewed 16 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Added portal resolutions for local Zephyr packages |
| packages/plugin-rnc-cli/index.js | Exposed loadMetroConfig command for CLI usage |
| packages/core/src/plugin/manifest.ts | Added updateManifest function for runtime manifest updates |
| packages/core/src/index.ts | Exported updateManifest for external usage |
| packages/core/src/commands/index.ts | Added loadMetroConfig command with debug logging |
| apps/*/react-native.config.js | Added Zephyr-wrapped bundle commands |
| apps/*/metro.config.js | Converted to async configuration with Zephyr integration |
| apps/*/package.json | Added Zephyr dependencies and renamed applications |
Comments suppressed due to low confidence (2)
87b321a to
64985a6
Compare
64985a6 to
961a994
Compare
jbroma
left a comment
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.
Took a look around this PR, would be nice to clear up the API needs before moving forward with this + PR needs some cleaning up
1adc6cd to
75ca051
Compare
jbroma
left a comment
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.
please remember to run pnpm lint in the root of the monorepo to fix formatting
jbroma
left a comment
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.
last thing and we're g2g 🎉
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.
lets undo removal of this file
loadMetroConfig & updateManifest helpers
|
@Thegrep01 I've adjusted the PR title and description so we can have this merged sooner 👍 |
jbroma
left a comment
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.
LGTM 🎉
Summary
loadMetroConfighelper function for integration with ZephyrupdateManifesthelper function for fixing the remote paths when using Zephyr