Skip to content

Commit

Permalink
Remove nonstandard "vm" field from modern CDP targets (#44835)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #44835

As titled. The `vm` field is not part of the CDP spec and will not be used by the modern debugger frontend or proxy.

This change affects modern CDP targets only (using `InspectorPackagerConnection`). We aim to enable sharing of more detailed metadata over 1/ a new, dedicated CDP domain, and 2/ namespaced under the existing `reactNative` field (for the latter, strictly limited to metadata necessary for dev server functionality).

Changelog: [Internal]

(Note: `/json` endpoint behaviour is unchanged for legacy CDP targets)

Reviewed By: robhogan

Differential Revision: D58285587

fbshipit-source-id: dfef3a56b20486ba11891df9940f6c7bef59528e
  • Loading branch information
huntie authored and facebook-github-bot committed Jun 11, 2024
1 parent d0012b7 commit d72ac96
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,6 @@ describe('inspector proxy HTTP API', () => {
app: 'bar-app',
id: 'page1',
title: 'bar-title',
vm: 'bar-vm',
},
]);

Expand Down Expand Up @@ -209,7 +208,6 @@ describe('inspector proxy HTTP API', () => {
},
title: 'bar-title',
type: 'node',
vm: 'bar-vm',
webSocketDebuggerUrl: expect.any(String),
},
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ export default class InspectorProxy implements InspectorProxyQueries {
type: 'node',
devtoolsFrontendUrl,
webSocketDebuggerUrl,
vm: page.vm,
...(page.vm != null ? {vm: page.vm} : null),
deviceName: device.getName(),
reactNative: {
logicalDeviceId: deviceId,
Expand Down
11 changes: 8 additions & 3 deletions packages/dev-middleware/src/inspector-proxy/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,16 @@ export type TargetCapabilityFlags = $ReadOnly<{
export type PageFromDevice = $ReadOnly<{
id: string,
title: string,
vm: string,
/** @deprecated This is sent from legacy targets only */
vm?: string,
app: string,
capabilities?: TargetCapabilityFlags,
}>;

export type Page = Required<PageFromDevice>;
export type Page = $ReadOnly<{
...PageFromDevice,
capabilities: $NonMaybeType<PageFromDevice['capabilities']>,
}>;

// Chrome Debugger Protocol message/event passed between device and debugger.
export type WrappedEvent = $ReadOnly<{
Expand Down Expand Up @@ -116,7 +120,8 @@ export type PageDescription = $ReadOnly<{

// React Native specific fields
deviceName: string,
vm: string,
/** @deprecated This is sent from legacy targets only */
vm?: string,

// React Native specific metadata
reactNative: $ReadOnly<{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,6 @@ folly::dynamic InspectorPackagerConnection::Impl::pages() {
pageDescription["id"] = std::to_string(page.id);
pageDescription["title"] = page.title + " [C++ connection]";
pageDescription["app"] = app_;
pageDescription["vm"] = page.vm;
pageDescription["capabilities"] =
targetCapabilitiesToDynamic(page.capabilities);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,6 @@ TEST_F(InspectorPackagerConnectionTest, TestGetPages) {
ElementsAreArray({AllOf(
AtJsonPtr("/app", Eq("my-app")),
AtJsonPtr("/title", Eq("mock-title [C++ connection]")),
AtJsonPtr("/vm", Eq("mock-vm")),
AtJsonPtr("/id", Eq(std::to_string(pageId))),
AtJsonPtr("/capabilities/nativePageReloads", Eq(true)),
AtJsonPtr(
Expand Down

0 comments on commit d72ac96

Please sign in to comment.