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.

Sign up for GitHub

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

Jump to bottom

Improve inspector proxy device identifier #985

Closed
byCedric opened this issue May 16, 2023 · 1 comment
Closed

Improve inspector proxy device identifier #985

byCedric opened this issue May 16, 2023 · 1 comment

Comments

@byCedric
Copy link
Contributor

byCedric commented May 16, 2023 •
edited
Loading

Do you want to request a feature or report a bug?

Feature

What is the current behavior?

This is an idea I shared with @huntie during the App.js Conf, but something I would love other opinions on.

Metro currently registers each device using an incremental ID generator inside the inspector proxy, when using the inspector through CDP. This is a simple approach, but it has some downsides.

For example, if an app fully restarts while keeping the inspector open, the same device is registered under a different ID. This forces users to restart the inspector, or it won't be connected to the right websocket address.

Instead of using incremental IDs, we might be able to leverage a more "stable identifier", such as the app installation ID (or even a generated UUID on the device). With that, the URL of the websocket address shouldn't change much. If an app gracefully restarts, the websocket disconnection should free up the previous registered device ID.

With this, the inspector connection should be a bit more stable. But I wonder what other people think.
@byCedric
Copy link
Contributor Author

Very early POC of having stable device identifiers:

stable-device-identifiers.mp4

It should definitely improve the whole experience.

This was referenced May 27, 2023
Closed
Merged
byCedric added a commit to expo/expo that referenced this issue Jun 16, 2023
@byCedric
feature(cli): allow client-side device ids to reuse debugger sessions…
f5fa30f 
… when restarting app (#22742)

# Why

After using the debugger for some time, I noticed a really annoying
behavior when the app crashes or is restarted manually. See
facebook/metro#985 for more info.

# How

This adds support for client-side unique device identifiers, such as
"installation ids" or "device ids". Similarly to future support within
Metro: facebook/metro#991

# Test Plan

See updated tests, and the test plan at
facebook/metro#991

> - Create a new project, and enable building from source on both
Android & iOS
> - **android**: edit [this
file](https://github.com/facebook/react-native/blob/main/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java#L282-L289)
in `node_modules/react-native` and add a hardcoded
`&device=testingandroid` query param.
> - **ios**: edit [this
file](https://github.com/facebook/react-native/blob/main/packages/react-native/React/DevSupport/RCTInspectorDevServerHelper.mm#L43-L53)
in `node_modules/react-naive` and add a hardcoded `&device=testingios`
query param.
> - Connect the debugger to the running app
> - Force close the app, which should cause a "reconnect" warning in the
debugger
> - Open the app again, and press "reconnect" in the debugger
> - _Due to the stable identifiers, the URL won't change and the above
scenario should work fine_

# 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).
@sirinartk sirinartk mentioned this issue Jun 21, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow client-side device identifiers in inspector proxy byCedric/facebook-metro
1 participant
@byCedric