-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
[dev-menu] Integrate javascript inspector #13041
Conversation
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.
The review previously left here is no longer valid, jump to the latest one 👉 #13041 (review)
# Why After #13041, we will have inspector support. Sometimes, the inspector is not available, for example, app uses JSC runtime. In such case, we should better to add JS runtime information to UI for trouleshooting. # How Hermes has `global.HermesInternal` property existed in JS. Check the property and add the DevMenuAppInfo. # Test Plan ### BareExpo + JSC 1. Show dev menu ### BareExpo + Hermes 1. Change enableHermes: true in apps/bare-expo/android/app/build.gradle 2. Show dev menu
64b25d4
to
e5b8f71
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.
The review previously left here is no longer valid, jump to the latest one 👉 #13041 (review)
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.
The review previously left here is no longer valid, jump to the latest one 👉 #13041 (review)
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.
Hi there! 👋 I'm a bot whose goal is to ensure your contributions meet our guidelines.
Looks like I have nothing to complain about 👏 Keep up the good work! 💪
Generated by ExpoBot 🤖 against 56a19a7
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 🚀
Thanks for the review and approval 🤗 |
Why
To support hermes inspector in dev-menu
How
Add a menu item
in expo/expo-cli#3495, we added
/inspector
endpoint to dev-server which supports querying or opening inspector.and in this pr, we added a menu item to integrate the function.
js runtime w/ inspector support, the item is enabled. e.g. Hermes.
![](https://user-images.githubusercontent.com/46429/120075967-0b4c0d80-c0d6-11eb-8f2b-3305c750f397.png)
js runtime w/o inspector support, the item is disabled. e.g. JSC.
![](https://user-images.githubusercontent.com/46429/120075989-24ed5500-c0d6-11eb-932b-513071d837f6.png)
dev-menu and dev-launcher fixes
there is a workaround and see code comment for details.
Test Plan
BareExpo + JSC
/path/to/expo-cli/packages/expo-cli/bin/expo.js start --dev-client
BareExpo + Hermes
/path/to/expo-cli/packages/expo-cli/bin/expo.js start --dev-client
BareSandbox + Hermes
/path/to/expo-cli/packages/expo-cli/bin/expo.js start --dev-client
apps/bare-sandbox/android/app/build.gradle
to makeenableHermes: true