Skip to content
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

Add sendWrappedEventToPackager to InspectorPackagerConnection #46902

Open
wants to merge 2 commits into
base: 0.76-stable
Choose a base branch
from

Conversation

Kudo
Copy link
Contributor

@Kudo Kudo commented Oct 8, 2024

Summary:

For Expo network inspector, we need a way to send CDP events from native to metro-inspector-proxy. Since the new c++ implemented InspectorPackagerConnection, it's difficult to have a way to send the CDP events. This PR tries to expose a new sendWrappedEventToPackager API. I think when react-native core adding the network inspector functionality, the new API should also be necessary.

Also expose IInspectorPackagerConnection as a public interface so that the callsites could integrate it. Not 100% sure how ReactAndroid.api being generated using OSS scripts, I just manually changed it.

Changelog:

[GENERAL] [ADDED] - Add sendWrappedEventToPackager to InspectorPackagerConnection

Test Plan:

  • Test build passed.
  • OSS doesn't have gtest infra, but I tried to update the InspectorPackagerConnectionTest, hopefully it passed in Meta's internal test.
  • Test expo-dev-client integration

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Contributor A React Native contributor. p: Expo Partner: Expo Partner labels Oct 8, 2024
@Kudo Kudo force-pushed the @kudo/sendWrappedEventToPackager branch from 3e61a94 to 64496f2 Compare October 9, 2024 10:31
@Kudo Kudo marked this pull request as ready for review October 9, 2024 12:02
@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Oct 9, 2024
@brentvatne brentvatne requested a review from huntie October 9, 2024 16:23
@hezi hezi self-assigned this Oct 9, 2024
byCedric added a commit to expo/expo that referenced this pull request Oct 14, 2024
# Why

These changes came after initial testing with Fusebox & RNDT on RN 0.76.
This does NOT change the default debugger to be RNDT yet, pending
facebook/react-native#46902.

# How

- Tweaked the "open js inspector" code to support Fusebox/RNDT better

# Test Plan

- `bun create expo@<canary> ./test-rndt`
- `cd ./test-rndt`
- `EXPO_USE_UNSTABLE_DEBUGGER=true bun expo start`
- Press `j` to open RNDT

# Checklist

<!--
Please check the appropriate items below if they apply to your diff.
This is required for changes to Expo modules.
-->

- [ ] Documentation is up to date to reflect these changes (eg:
https://docs.expo.dev and README.md).
- [ ] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
- [ ] This diff will work correctly for `npx expo prebuild` & EAS Build
(eg: updated a module plugin).
@Kudo Kudo force-pushed the @kudo/sendWrappedEventToPackager branch from e6dcd56 to 88a9f8b Compare October 16, 2024 16:45
@Kudo Kudo changed the base branch from main to 0.76-stable October 16, 2024 16:45
@Kudo
Copy link
Contributor Author

Kudo commented Oct 16, 2024

@huntie rebase the pr upon 0.76-stable is smooth without conflicts, so i just set the pr's base to 0.76-stable without re-create a new pr. please help to take a look again. thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Contributor A React Native contributor. p: Expo Partner: Expo Partner Pick Request Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants