Update modern inspector targets to directly send CDP title and description (#46780)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46780

This is primarily a debugger server change to better-align the `title` and `description` fields (visible in via the CDP `/json/list` endpoint), reporting them directly from the device.

For React Native users, the net effect of this change is to improve/align the display name for each debugger target in the CLI multi-select menu (`j` to debug). This change may also be useful for discovery in non-Fusebox frontends such as VS Code (`vscode-expo` extension).

Changes:
- Rename `title` prop on `InspectorPageDescription`/`IInspector::addPage` to `description` (no call site changes).
- Add `deviceName` param to `InspectorPackagerConnection`.
- Move the page `description` to the `description` JSON field.
- Update `InspectorPackagerConnection::Impl::pages()` to return new `title` and `description` fields.
- Align `OpenDebuggerKeyboardHandler` to display `title` field.
- Deprecate the nonstandard `deviceName` field.

**Before**

```
[
    {
        "id": "3c9f24bedab0e73fca6a1b295030e7af9346a8c0-1",
        "title": "React Native Bridgeless [C++ connection]",
        "description": "com.facebook.RNTester",
         ...
    }
```

The `description` field was previously the closest thing we had to a target identifier. Today, this is not needed, since we have the stable `reactNative.logicalDeviceId` field.

**After**

```
[
    {
        "id": "3c9f24bedab0e73fca6a1b295030e7af9346a8c0-1",
        "title": "com.facebook.RNTester (iPhone 16 Pro)",
        "description": "React Native Bridgeless [C++ connection]",
         ...
    }
```

The `title` field is now more human readable and aligned with what we render in the window title of React Native DevTools. The `description` field describes the type of debugger target (the specific React Native implementation).

Changelog: [Internal]

Reviewed By: vzaidman

Differential Revision: D63329456

fbshipit-source-id: cfe98f77e31c729431005925cfc66e2780ef8c72
This commit is contained in:
Alex Hunt authored and Facebook GitHub Bot committed 2024-10-02 07:30:31 -07:00
1 parent 07c02d4427
commit c430083fa0
17 files changed
+91 -59

No files matched your search

@@ -18,7 +18,6 @@ type PageDescription = $ReadOnly<{
id: string,
title: string,
description: string,
deviceName: string,
...
}>;
@@ -53,9 +52,9 @@ export default class OpenDebuggerKeyboardHandler {
} catch (e) {
this.#log(
'error',
'Failed to open debugger for %s on %s debug targets: %s',
'Failed to open debugger for %s (%s): %s',
target.title,
target.description,
target.deviceName,
e.message,
);
this.#clearTerminalMenu();
@@ -108,8 +107,8 @@ export default class OpenDebuggerKeyboardHandler {
`Multiple debug targets available, please select:\n ${targets
.slice(0, 9)
.map(
({description, deviceName}, i) =>
`${chalk.white.inverse(` ${i + 1} `)} - "${description}" on "${deviceName}"`,
({title}, i) =>
`${chalk.white.inverse(` ${i + 1} `)} - "${title}"`,
)
.join('\n ')}`,
);
@@ -160,7 +160,7 @@ export default class InspectorProxy implements InspectorProxyQueries {
return {
id: `${deviceId}-${page.id}`,
title: page.title,
description: page.app,
description: page.description ?? page.app,
type: 'node',
devtoolsFrontendUrl,
webSocketDebuggerUrl,
@@ -53,6 +53,8 @@ export type TargetCapabilityFlags = $ReadOnly<{
export type PageFromDevice = $ReadOnly<{
id: string,
title: string,
/** Sent from modern targets only */
description?: string,
/** @deprecated This is sent from legacy targets only */
vm?: string,
app: string,
@@ -119,6 +121,7 @@ export type PageDescription = $ReadOnly<{
webSocketDebuggerUrl: string,
// React Native specific fields
/** @deprecated Prefer `title` */
deviceName: string,
/** @deprecated This is sent from legacy targets only */
vm?: string,
@@ -22,6 +22,7 @@
#import "RCTCxxInspectorPackagerConnection.h"
#import "RCTCxxInspectorPackagerConnectionDelegate.h"
#import "RCTCxxInspectorWebSocketAdapter.h"
#import "RCTInspectorUtils.h"
using namespace facebook::react::jsinspector_modern;
@interface RCTCxxInspectorPackagerConnection () {
@@ -36,8 +37,10 @@ RCT_NOT_IMPLEMENTED(-(instancetype)init)
- (instancetype)initWithURL:(NSURL *)url
{
if (self = [super init]) {
auto metadata = [RCTInspectorUtils getHostMetadata];
_cxxImpl = std::make_unique<InspectorPackagerConnection>(
[url absoluteString].UTF8String,
metadata.deviceName.UTF8String,
[[NSBundle mainBundle] bundleIdentifier].UTF8String,
std::make_unique<RCTCxxInspectorPackagerConnectionDelegate>());
}
@@ -71,7 +71,7 @@ RCT_NOT_IMPLEMENTED(-(instancetype)init)
NSMutableArray<RCTInspectorPage *> *array = [NSMutableArray arrayWithCapacity:pages.size()];
for (size_t i = 0; i < pages.size(); i++) {
RCTInspectorPage *pageWrapper = [[RCTInspectorPage alloc] initWithId:pages[i].id
title:@(pages[i].title.c_str())
title:@(pages[i].description.c_str())
vm:@(pages[i].vm.c_str())];
[array addObject:pageWrapper];
}
@@ -28,12 +28,12 @@ import okhttp3.WebSocketListener;
@DoNotStrip private final HybridData mHybridData;
public CxxInspectorPackagerConnection(String url, String packageName) {
mHybridData = initHybrid(url, packageName, new DelegateImpl());
public CxxInspectorPackagerConnection(String url, String deviceName, String packageName) {
mHybridData = initHybrid(url, deviceName, packageName, new DelegateImpl());
}
private static native HybridData initHybrid(
String url, String packageName, DelegateImpl delegate);
String url, String deviceName, String packageName, DelegateImpl delegate);
public native void connect();
@@ -214,8 +214,12 @@ public class DevServerHelper {
@Override
protected Void doInBackground(Void... params) {
if (InspectorFlags.getFuseboxEnabled()) {
Map<String, String> metadata =
AndroidInfoHelpers.getInspectorHostMetadata(mApplicationContext);
mInspectorPackagerConnection =
new CxxInspectorPackagerConnection(getInspectorDeviceUrl(), mPackageName);
new CxxInspectorPackagerConnection(
getInspectorDeviceUrl(), metadata.get("deviceName"), mPackageName);
} else {
mInspectorPackagerConnection =
new InspectorPackagerConnection(getInspectorDeviceUrl(), mPackageName);
@@ -15,17 +15,19 @@ namespace facebook::react::jsinspector_modern {
JCxxInspectorPackagerConnection::JCxxInspectorPackagerConnection(
const std::string& url,
const std::string& deviceName,
const std::string& packageName,
alias_ref<JDelegateImpl::javaobject> delegate)
: cxxImpl_(url, packageName, delegate->wrapInUniquePtr()) {}
: cxxImpl_(url, deviceName, packageName, delegate->wrapInUniquePtr()) {}
local_ref<JCxxInspectorPackagerConnection::jhybriddata>
JCxxInspectorPackagerConnection::initHybrid(
alias_ref<jclass>,
const std::string& url,
const std::string& deviceName,
const std::string& packageName,
alias_ref<JDelegateImpl::javaobject> delegate) {
return makeCxxInstance(url, packageName, delegate);
return makeCxxInstance(url, deviceName, packageName, delegate);
}
void JCxxInspectorPackagerConnection::connect() {
@@ -44,12 +44,14 @@ class JCxxInspectorPackagerConnection
*/
JCxxInspectorPackagerConnection(
const std::string& url,
const std::string& deviceName,
const std::string& packageName,
jni::alias_ref<JDelegateImpl::javaobject> delegate);
static jni::local_ref<jhybriddata> initHybrid(
jni::alias_ref<jclass>,
const std::string& url,
const std::string& deviceName,
const std::string& packageName,
jni::alias_ref<JDelegateImpl::javaobject> delegate);
@@ -85,7 +85,7 @@ jni::local_ref<jni::JArrayClass<JPage::javaobject>> JInspector::getPages() {
inspector_->getPages();
auto array = jni::JArrayClass<JPage::javaobject>::newArray(pages.size());
for (size_t i = 0; i < pages.size(); i++) {
(*array)[i] = JPage::create(pages[i].id, pages[i].title, pages[i].vm);
(*array)[i] = JPage::create(pages[i].id, pages[i].description, pages[i].vm);
}
return array;
}
@@ -37,7 +37,7 @@ namespace {
class InspectorImpl : public IInspector {
public:
int addPage(
const std::string& title,
const std::string& description,
const std::string& vm,
ConnectFunc connectFunc,
InspectorTargetCapabilities capabilities) override;
@@ -66,7 +66,7 @@ class InspectorImpl : public IInspector {
private:
int id_;
std::string title_;
std::string description_;
std::string vm_;
ConnectFunc connectFunc_;
InspectorTargetCapabilities capabilities_;
@@ -79,12 +79,12 @@ class InspectorImpl : public IInspector {
InspectorImpl::Page::Page(
int id,
const std::string& title,
const std::string& description,
const std::string& vm,
ConnectFunc connectFunc,
InspectorTargetCapabilities capabilities)
: id_(id),
title_(title),
description_(description),
vm_(vm),
connectFunc_(std::move(connectFunc)),
capabilities_(std::move(capabilities)) {}
@@ -92,7 +92,7 @@ InspectorImpl::Page::Page(
InspectorImpl::Page::operator InspectorPageDescription() const {
return InspectorPageDescription{
.id = id_,
.title = title_,
.description = description_,
.vm = vm_,
.capabilities = capabilities_,
};
@@ -103,7 +103,7 @@ InspectorImpl::ConnectFunc InspectorImpl::Page::getConnectFunc() const {
}
int InspectorImpl::addPage(
const std::string& title,
const std::string& description,
const std::string& vm,
ConnectFunc connectFunc,
InspectorTargetCapabilities capabilities) {
@@ -114,7 +114,8 @@ int InspectorImpl::addPage(
int pageId = nextPageId_++;
assert(pages_.count(pageId) == 0 && "Unexpected duplicate page ID");
pages_.emplace(
pageId, Page{pageId, title, vm, std::move(connectFunc), capabilities});
pageId,
Page{pageId, description, vm, std::move(connectFunc), capabilities});
return pageId;
}
@@ -44,7 +44,7 @@ const folly::dynamic targetCapabilitiesToDynamic(
struct InspectorPageDescription {
const int id;
const std::string title;
const std::string description;
const std::string vm;
const InspectorTargetCapabilities capabilities;
};
@@ -100,7 +100,7 @@ class JSINSPECTOR_EXPORT IInspector : public IDestructible {
* \returns the ID assigned to the new page.
*/
virtual int addPage(
const std::string& title,
const std::string& description,
const std::string& vm,
ConnectFunc connectFunc,
InspectorTargetCapabilities capabilities = {}) = 0;
@@ -28,21 +28,25 @@ static constexpr const char* INVALID = "<invalid>";
std::shared_ptr<InspectorPackagerConnection::Impl>
InspectorPackagerConnection::Impl::create(
std::string url,
std::string app,
std::string deviceName,
std::string appName,
std::unique_ptr<InspectorPackagerConnectionDelegate> delegate) {
// No make_shared because the constructor is private
std::shared_ptr<InspectorPackagerConnection::Impl> impl(
new InspectorPackagerConnection::Impl(url, app, std::move(delegate)));
new InspectorPackagerConnection::Impl(
url, deviceName, appName, std::move(delegate)));
getInspectorInstance().registerPageStatusListener(impl);
return impl;
}
InspectorPackagerConnection::Impl::Impl(
std::string url,
std::string app,
std::string deviceName,
std::string appName,
std::unique_ptr<InspectorPackagerConnectionDelegate> delegate)
: url_(std::move(url)),
app_(std::move(app)),
deviceName_(std::move(deviceName)),
appName_(std::move(appName)),
delegate_(std::move(delegate)) {}
void InspectorPackagerConnection::Impl::handleProxyMessage(
@@ -159,8 +163,9 @@ folly::dynamic InspectorPackagerConnection::Impl::pages() {
for (const auto& page : pages) {
folly::dynamic pageDescription = folly::dynamic::object;
pageDescription["id"] = std::to_string(page.id);
pageDescription["title"] = page.title + " [C++ connection]";
pageDescription["app"] = app_;
pageDescription["title"] = appName_ + " (" + deviceName_ + ")";
pageDescription["description"] = page.description + " [C++ connection]";
pageDescription["app"] = appName_;
pageDescription["capabilities"] =
targetCapabilitiesToDynamic(page.capabilities);
@@ -341,9 +346,10 @@ void InspectorPackagerConnection::Impl::RemoteConnection::onDisconnect() {
InspectorPackagerConnection::InspectorPackagerConnection(
std::string url,
std::string app,
std::string deviceName,
std::string appName,
std::unique_ptr<InspectorPackagerConnectionDelegate> delegate)
: impl_(Impl::create(url, app, std::move(delegate))) {}
: impl_(Impl::create(url, deviceName, appName, std::move(delegate))) {}
bool InspectorPackagerConnection::isConnected() const {
return impl_->isConnected();
@@ -33,13 +33,15 @@ class InspectorPackagerConnection {
* Creates a new connection instance. Connections start in the disconnected
* state; connect() should be called to establish a connection.
* \param url The WebSocket URL where the inspector-proxy server is listening.
* \param app The name of the application being debugged.
* \param deviceName The host device name.
* \param appName The application name.
* \param delegate An interface to platform-specific methods for creating a
* WebSocket, scheduling async work, etc.
*/
InspectorPackagerConnection(
std::string url,
std::string app,
std::string deviceName,
std::string appName,
std::unique_ptr<InspectorPackagerConnectionDelegate> delegate);
bool isConnected() const;
void connect();
@@ -31,7 +31,8 @@ class InspectorPackagerConnection::Impl
*/
static std::shared_ptr<Impl> create(
std::string url,
std::string app,
std::string deviceName,
std::string appName,
std::unique_ptr<InspectorPackagerConnectionDelegate> delegate);
// InspectorPackagerConnection's public API
@@ -61,7 +62,8 @@ class InspectorPackagerConnection::Impl
Impl(
std::string url,
std::string app,
std::string deviceName,
std::string appName,
std::unique_ptr<InspectorPackagerConnectionDelegate> delegate);
Impl(const Impl&) = delete;
Impl& operator=(const Impl&) = delete;
@@ -91,7 +93,8 @@ class InspectorPackagerConnection::Impl
virtual void onPageRemoved(int pageId) override;
const std::string url_;
const std::string app_;
const std::string deviceName_;
const std::string appName_;
const std::unique_ptr<InspectorPackagerConnectionDelegate> delegate_;
std::unordered_map<std::string, Session> inspectorSessions_;
@@ -39,6 +39,7 @@ class InspectorPackagerConnectionTestBase : public testing::Test {
InspectorPackagerConnectionTestBase()
: packagerConnection_(InspectorPackagerConnection{
"ws://mock-host:12345",
"my-device",
"my-app",
packagerConnectionDelegates_.make_unique(asyncExecutor_)}) {
ON_CALL(*packagerConnectionDelegate(), connectWebSocket(_, _))
@@ -200,14 +201,14 @@ TEST_F(InspectorPackagerConnectionTest, TestGetPages) {
})");
auto pageId1 = getInspectorInstance().addPage(
"mock-title-1",
"mock-description-1",
"mock-vm",
localConnections_
.lazily_make_unique<std::unique_ptr<IRemoteConnection>>(),
{.nativePageReloads = true});
auto pageId2 = getInspectorInstance().addPage(
"mock-title-2",
"mock-description-2",
"mock-vm",
localConnections_
.lazily_make_unique<std::unique_ptr<IRemoteConnection>>(),
@@ -222,17 +223,23 @@ TEST_F(InspectorPackagerConnectionTest, TestGetPages) {
"/payload",
ElementsAreArray(
{AllOf(
AtJsonPtr("/app", Eq("my-app")),
AtJsonPtr("/title", Eq("mock-title-1 [C++ connection]")),
AtJsonPtr("/id", Eq(std::to_string(pageId1))),
AtJsonPtr("/title", Eq("my-app (my-device)")),
AtJsonPtr(
"/description",
Eq("mock-description-1 [C++ connection]")),
AtJsonPtr("/app", Eq("my-app")),
AtJsonPtr("/capabilities/nativePageReloads", Eq(true)),
AtJsonPtr(
"/capabilities/nativeSourceCodeFetching",
Eq(false))),
AllOf(
AtJsonPtr("/app", Eq("my-app")),
AtJsonPtr("/title", Eq("mock-title-2 [C++ connection]")),
AtJsonPtr("/id", Eq(std::to_string(pageId2))),
AtJsonPtr("/title", Eq("my-app (my-device)")),
AtJsonPtr(
"/description",
Eq("mock-description-2 [C++ connection]")),
AtJsonPtr("/app", Eq("my-app")),
AtJsonPtr("/capabilities/nativePageReloads", Eq(true)),
AtJsonPtr(
"/capabilities/nativeSourceCodeFetching",
@@ -265,7 +272,7 @@ TEST_F(InspectorPackagerConnectionTest, TestSendReceiveEvents) {
packagerConnection_->connect();
auto pageId = getInspectorInstance().addPage(
"mock-title",
"mock-description",
"mock-vm",
localConnections_
.lazily_make_unique<std::unique_ptr<IRemoteConnection>>());
@@ -351,7 +358,7 @@ TEST_F(InspectorPackagerConnectionTest, TestSendReceiveEventsToMultiplePages) {
const int kNumPages = 2;
for (int i = 0; i < kNumPages; ++i) {
pageIds.push_back(getInspectorInstance().addPage(
"mock-title",
"mock-description",
"mock-vm",
localConnections_
.lazily_make_unique<std::unique_ptr<IRemoteConnection>>()));
@@ -429,7 +436,7 @@ TEST_F(InspectorPackagerConnectionTest, TestSendEventToAllConnections) {
packagerConnection_->connect();
auto pageId = getInspectorInstance().addPage(
"mock-title",
"mock-description",
"mock-vm",
localConnections_
.lazily_make_unique<std::unique_ptr<IRemoteConnection>>());
@@ -470,7 +477,7 @@ TEST_F(InspectorPackagerConnectionTest, TestConnectThenDisconnect) {
packagerConnection_->connect();
auto pageId = getInspectorInstance().addPage(
"mock-title",
"mock-description",
"mock-vm",
localConnections_
.lazily_make_unique<std::unique_ptr<IRemoteConnection>>());
@@ -505,7 +512,7 @@ TEST_F(InspectorPackagerConnectionTest, TestConnectThenCloseSocket) {
packagerConnection_->connect();
auto pageId = getInspectorInstance().addPage(
"mock-title",
"mock-description",
"mock-vm",
localConnections_
.lazily_make_unique<std::unique_ptr<IRemoteConnection>>());
@@ -533,7 +540,7 @@ TEST_F(InspectorPackagerConnectionTest, TestConnectThenSocketFailure) {
packagerConnection_->connect();
auto pageId = getInspectorInstance().addPage(
"mock-title",
"mock-description",
"mock-vm",
localConnections_
.lazily_make_unique<std::unique_ptr<IRemoteConnection>>());
@@ -563,7 +570,7 @@ TEST_F(
packagerConnection_->connect();
auto pageId = getInspectorInstance().addPage(
"mock-title",
"mock-description",
"mock-vm",
localConnections_
.lazily_make_unique<std::unique_ptr<IRemoteConnection>>());
@@ -609,7 +616,7 @@ TEST_F(
packagerConnection_->connect();
auto pageId = getInspectorInstance().addPage(
"mock-title",
"mock-description",
"mock-vm",
localConnections_
.lazily_make_unique<std::unique_ptr<IRemoteConnection>>());
@@ -644,7 +651,7 @@ TEST_F(InspectorPackagerConnectionTest, TestMultipleDisconnect) {
packagerConnection_->connect();
auto pageId = getInspectorInstance().addPage(
"mock-title",
"mock-description",
"mock-vm",
localConnections_
.lazily_make_unique<std::unique_ptr<IRemoteConnection>>());
@@ -690,7 +697,7 @@ TEST_F(InspectorPackagerConnectionTest, TestDisconnectThenSendEvent) {
packagerConnection_->connect();
auto pageId = getInspectorInstance().addPage(
"mock-title",
"mock-description",
"mock-vm",
localConnections_
.lazily_make_unique<std::unique_ptr<IRemoteConnection>>());
@@ -916,7 +923,7 @@ TEST_F(
packagerConnection_.reset();
auto pageId = getInspectorInstance().addPage(
"mock-title",
"mock-description",
"mock-vm",
localConnections_
.lazily_make_unique<std::unique_ptr<IRemoteConnection>>());
@@ -970,7 +977,7 @@ TEST_F(InspectorPackagerConnectionTest, TestDestroyConnectionOnPageRemoved) {
ASSERT_TRUE(webSockets_[0]);
auto pageId = getInspectorInstance().addPage(
"mock-title",
"mock-description",
"mock-vm",
localConnections_
.lazily_make_unique<std::unique_ptr<IRemoteConnection>>());
@@ -1000,7 +1007,7 @@ TEST_F(
packagerConnection_->connect();
auto pageId = getInspectorInstance().addPage(
"mock-title",
"mock-description",
"mock-vm",
localConnections_
.lazily_make_unique<std::unique_ptr<IRemoteConnection>>());
@@ -1040,7 +1047,7 @@ TEST_F(
packagerConnection_->connect();
auto pageId = getInspectorInstance().addPage(
"mock-title",
"mock-description",
"mock-vm",
localConnections_
.lazily_make_unique<std::unique_ptr<IRemoteConnection>>());
@@ -1122,7 +1129,7 @@ TEST_F(
packagerConnection_->connect();
auto pageId = getInspectorInstance().addPage(
"mock-title",
"mock-description",
"mock-vm",
localConnections_
.lazily_make_unique<std::unique_ptr<IRemoteConnection>>());
@@ -1228,7 +1235,7 @@ TEST_F(InspectorPackagerConnectionTest, TestRejectedPageConnection) {
} mockNextConnectionBehavior;
auto pageId = getInspectorInstance().addPage(
"mock-title",
"mock-description",
"mock-vm",
[&mockNextConnectionBehavior,
this](auto remoteConnection) -> std::unique_ptr<ILocalConnection> {
@@ -91,7 +91,7 @@ void ReactInstanceIntegrationTest::SetUp() {
// Under modern CDP, the React host is responsible for adding itself as
// the root target on startup.
pageId_ = inspector.addPage(
"mock-title",
"mock-description",
"mock-vm",
[hostTargetIfModernCDP](std::unique_ptr<IRemoteConnection> remote)
-> std::unique_ptr<ILocalConnection> {