Skip to content

Conversation

byCedric
Copy link
Member

@byCedric byCedric commented Nov 29, 2023

Why

This PR re-implements the debugging tools, temporarily removed to merge the React Native 0.73.0-rc.x upgrade.

In this re-implementation, we are also trying to move over to the new React Native JS Inspector, the one shipped with React Native 0.73.x.

image

TODO

There are still pending/follow-up tasks to complete the full debugging transition.

  • (Optional) try to get rid of the last proxy workarounds, besides using different Device class
  • Fix @react-native/dev-middleware not responding to /open-debugger (request times out, but works)
  • Enable network inspector inside Chrome DevTools from @react-native/dev-middleware
  • Use file tree instead of snippet workspaces, under sources tab in Chrome DevTools from @react-native/dev-middleware
  • Update vscode-expo-tools to provide ?userAgent=, since we don't get any user-agent header from the extension.

How

  • Refactored existing InspectorProxy, InspectorDevice, and all handles into new debugging group under src/start/server/metro/debugging.
  • Integrated @react-native/dev-middleware into separate createDebugMiddleware function
  • Extend InspectorProxy instead of wrapping it
  • Cleaned up no-longer-necessary workarounds and overwrites in InspectorProxy and InspectorDevice

Test Plan

Prepare the current template as npm tarball

  • $ cd .../expo/expo
  • $ cd ./templates/expo-template-bare-minimum/
  • $ npm pack

Prepare a new canary project

  • $ bun create expo ./test-debug --template blank
  • $ bun expo install expo@canary
  • $ expod install --fix (expod → run @expo/cli from source)
  • $ expod prebuild --template ./<tarball-from-above>.tgz
  • $ expod run:ios
  • Open the two debuggers:
    • Press J in terminal → Current Expo Chrome DevTools implementation
    • $ curl -X POST "http://localhost:8081/open-debugger" → Experimental Chrome DevTools
    • $ curl -X POST "http://localhost:8081/open-debugger?appId=<bundleIdentifier>" → Experimental Chrome DevTools
    • $ curl -X POST "http://localhost:8081/open-debugger?device=<deviceIdentifier>" → Experimental Chrome DevTools (see device ID for android)

Checklist

@byCedric byCedric requested a review from EvanBacon as a code owner November 29, 2023 19:06
@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Nov 29, 2023
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Nov 29, 2023
@@ -1,6 +0,0 @@
import { unstable_Device } from '@react-native/dev-middleware';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Copy link
Member Author

@byCedric byCedric Nov 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

murder-it

(That is, the custom type declarations)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generated TS types in the React Native repo! 😉🚀

@byCedric byCedric force-pushed the @bycedric/cli/fix-dev-middleware branch from 1ea6862 to 3312f06 Compare December 1, 2023 12:15
@byCedric byCedric merged commit ade54e6 into main Dec 1, 2023
@byCedric byCedric deleted the @bycedric/cli/fix-dev-middleware branch December 1, 2023 16:55
byCedric added a commit that referenced this pull request Dec 4, 2023
…t Native JS Inspector (#25671)

# Why

Follow-up of #25649

# How

This PR changes two things

1. When pressing `j` in terminal, allow users to select a device using a
select UI. This is copied from the multiple dev tools, and uses the
(known) device name. This replaces the "open all app debuggers" to
confuse users less.
2. When using `EXPO_USE_UNSTABLE_DEBUGGER=true`, the React Native JS
Inspector is used instead of the "classic" Chrome DevTools from SDK
<=49.

It also cleans up the older middleware that handled `/inspector`. This
is now handled by the CDP-spec-compliant `/json/list`. We could also
move everything under `_expo/inspector/...` if this is a problem.

# Test Plan

- `$ bun create expo ./test-debugger --template blank`
- `$ bun expo install expo@canary`
- `$ bun expo install --fix`
- `$ bun expo run:ios`
  - Or `$ EXPO_USE_UNSTABLE_DEBUGGER=true bun expo run:ios`

# 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).

---------

Co-authored-by: Evan Bacon <bacon@expo.io>
Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
onizam95 pushed a commit to onizam95/expo-av-drm that referenced this pull request Jan 15, 2024
…25649)

# Why

This PR re-implements the debugging tools, temporarily removed to merge
the React Native `0.73.0-rc.x` upgrade.

In this re-implementation, we are also trying to move over to the new
React Native JS Inspector, the one shipped with React Native 0.73.x.


![image](https://github.com/expo/expo/assets/1203991/d747a6cc-9c89-4279-8d3a-496fbb5a7cce)

# TODO

There are still pending/follow-up tasks to complete the full debugging
transition.

- [ ] (Optional) try to get rid of the last proxy workarounds, besides
using different `Device` class
- [ ] Fix `@react-native/dev-middleware` not responding to
`/open-debugger` (request times out, but works)
- [ ] Enable network inspector inside Chrome DevTools from
`@react-native/dev-middleware`
- [ ] Use file tree instead of snippet workspaces, under `sources` tab
in Chrome DevTools from `@react-native/dev-middleware`
- [ ] Update `vscode-expo-tools` to provide `?userAgent=`, since we
don't get any `user-agent` header from the extension.

# How

- Refactored existing `InspectorProxy`, `InspectorDevice`, and all
handles into new `debugging` group under
`src/start/server/metro/debugging`.
- Integrated `@react-native/dev-middleware` into separate
`createDebugMiddleware` function
- Extend `InspectorProxy` instead of wrapping it
- Cleaned up no-longer-necessary workarounds and overwrites in
`InspectorProxy` and `InspectorDevice`

# Test Plan

Prepare the current template as npm tarball

- `$ cd .../expo/expo`
- `$ cd ./templates/expo-template-bare-minimum/`
- `$ npm pack`

Prepare a new canary project

- `$ bun create expo ./test-debug --template blank`
- `$ bun expo install expo@canary`
- `$ expod install --fix` (_`expod` → run `@expo/cli` from source_)
- `$ expod prebuild --template ./<tarball-from-above>.tgz`
- `$ expod run:ios`
- Open the _two_ debuggers:
- **Press J in terminal** → Current Expo Chrome DevTools implementation
- `$ curl -X POST "http://localhost:8081/open-debugger"` → Experimental
Chrome DevTools
- `$ curl -X POST
"http://localhost:8081/open-debugger?appId=<bundleIdentifier>"` →
Experimental Chrome DevTools
- `$ curl -X POST
"http://localhost:8081/open-debugger?device=<deviceIdentifier>"` →
Experimental Chrome DevTools (see [device ID for
android](https://github.com/facebook/react-native/blob/c0375b8dadaafc0e0c11bbe676a61c44ac9fab1d/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java#L317C20-L317C40))

# 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).

---------

Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
onizam95 pushed a commit to onizam95/expo-av-drm that referenced this pull request Jan 15, 2024
…t Native JS Inspector (expo#25671)

# Why

Follow-up of expo#25649

# How

This PR changes two things

1. When pressing `j` in terminal, allow users to select a device using a
select UI. This is copied from the multiple dev tools, and uses the
(known) device name. This replaces the "open all app debuggers" to
confuse users less.
2. When using `EXPO_USE_UNSTABLE_DEBUGGER=true`, the React Native JS
Inspector is used instead of the "classic" Chrome DevTools from SDK
<=49.

It also cleans up the older middleware that handled `/inspector`. This
is now handled by the CDP-spec-compliant `/json/list`. We could also
move everything under `_expo/inspector/...` if this is a problem.

# Test Plan

- `$ bun create expo ./test-debugger --template blank`
- `$ bun expo install expo@canary`
- `$ bun expo install --fix`
- `$ bun expo run:ios`
  - Or `$ EXPO_USE_UNSTABLE_DEBUGGER=true bun expo run:ios`

# 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).

---------

Co-authored-by: Evan Bacon <bacon@expo.io>
Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: fingerprint changed bot: passed checks ExpoBot has nothing to complain about
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants