From 9ba56f6f5d0601b29c68390600ce0ab2152904ae Mon Sep 17 00:00:00 2001 From: Alex Hunt Date: Tue, 6 Feb 2024 04:09:53 -0800 Subject: [PATCH] Remove matching of Modern/Legacy page types (#42885) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/42885 ## Context We're introducing the concept of **capability flags** to provide granular control of behaviours in the Inspector Proxy, to replace the recently added `type: 'Legacy' | 'Modern'` target switch. A capability flag disables a specific feature/hack in the Inspector Proxy layer by indicating that the target supports one or more modern CDP features. ## This diff Following D53355413, we're now able to remove the previous `type: 'Legacy' | 'Modern'` page concept, implemented in this diff. Changelog: [Internal] Reviewed By: robhogan Differential Revision: D53358480 fbshipit-source-id: 62e53a1bd60760291ada3479121dfca9e1f6edbc --- .../InspectorProxyCdpRewritingHacks-test.js | 13 ++++--------- .../__tests__/InspectorProxyHttpApi-test.js | 2 -- .../InspectorProxyReactNativeReloads-test.js | 18 +++++++----------- .../src/inspector-proxy/Device.js | 9 +++------ .../src/inspector-proxy/InspectorProxy.js | 1 - .../src/inspector-proxy/types.js | 2 -- .../src/middleware/openDebuggerMiddleware.js | 3 +-- 7 files changed, 15 insertions(+), 33 deletions(-) diff --git a/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js b/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js index 08327991c3ccb6..51545ecd9be702 100644 --- a/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js +++ b/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js @@ -475,19 +475,14 @@ describe.each(['HTTP', 'HTTPS'])( ); }); - describe.each([ - ['for modern targets', {}], - [ - "when target has 'nativeSourceMapFetching' capability flag", - {nativeSourceMapFetching: true}, - ], - ])('disabled %s', (_, capabilities: TargetCapabilityFlags) => { + describe("disabled when target has 'nativeSourceCodeFetching' capability flag", () => { const pageDescription = { app: 'bar-app', id: 'page1', title: 'bar-title', - type: 'Modern', - capabilities, + capabilities: { + nativeSourceCodeFetching: true, + }, vm: 'bar-vm', }; diff --git a/packages/dev-middleware/src/__tests__/InspectorProxyHttpApi-test.js b/packages/dev-middleware/src/__tests__/InspectorProxyHttpApi-test.js index b0860ab458efab..e601f219f06324 100644 --- a/packages/dev-middleware/src/__tests__/InspectorProxyHttpApi-test.js +++ b/packages/dev-middleware/src/__tests__/InspectorProxyHttpApi-test.js @@ -193,7 +193,6 @@ describe('inspector proxy HTTP API', () => { reactNative: { capabilities: {}, logicalDeviceId: 'device1', - type: 'Legacy', }, title: 'bar-title', type: 'node', @@ -209,7 +208,6 @@ describe('inspector proxy HTTP API', () => { reactNative: { capabilities: {}, logicalDeviceId: 'device2', - type: 'Legacy', }, title: 'bar-title', type: 'node', diff --git a/packages/dev-middleware/src/__tests__/InspectorProxyReactNativeReloads-test.js b/packages/dev-middleware/src/__tests__/InspectorProxyReactNativeReloads-test.js index e32a6d71bb8dfc..a89ddbd08ed558 100644 --- a/packages/dev-middleware/src/__tests__/InspectorProxyReactNativeReloads-test.js +++ b/packages/dev-middleware/src/__tests__/InspectorProxyReactNativeReloads-test.js @@ -397,13 +397,7 @@ describe('inspector proxy React Native reloads', () => { } }); - test.each([ - ['for modern targets', {}], - [ - "when target has 'nativePageReloads' capability flag", - {nativePageReloads: true}, - ], - ])('disabled %s', async (_, capabilities) => { + test("disabled when target has 'nativePageReloads' capability flag", async () => { let device1; try { /*** @@ -420,8 +414,9 @@ describe('inspector proxy React Native reloads', () => { // NOTE: 'React' is a magic string used to detect React Native pages // in legacy mode. title: 'React Native (mock)', - type: 'Modern', - capabilities, + capabilities: { + nativePageReloads: true, + }, vm: 'vm', }, ]); @@ -456,8 +451,9 @@ describe('inspector proxy React Native reloads', () => { id: 'originalPage-updated', // NOTE: 'React' is a magic string used to detect React Native pages. title: 'React Native (mock)', - type: 'Modern', - capabilities, + capabilities: { + nativePageReloads: true, + }, vm: 'vm', }, ]); diff --git a/packages/dev-middleware/src/inspector-proxy/Device.js b/packages/dev-middleware/src/inspector-proxy/Device.js index 50bd3f70c4f3bd..26befc0b1185c4 100644 --- a/packages/dev-middleware/src/inspector-proxy/Device.js +++ b/packages/dev-middleware/src/inspector-proxy/Device.js @@ -167,7 +167,6 @@ export default class Device { title: 'React Native Experimental (Improved Chrome Reloads)', vm: "don't use", app: this.#app, - type: 'Legacy', capabilities: {}, }; return [...this.#pages.values(), reactNativeReloadablePage]; @@ -306,11 +305,10 @@ export default class Device { } /** - * Returns `true` if a page reports `type: 'Modern'` or supports the given - * target capability flag. + * Returns `true` if a page supports the given target capability flag. */ #pageHasCapability(page: Page, flag: $Keys): boolean { - return page.type === 'Modern' || page.capabilities[flag] === true; + return page.capabilities[flag] === true; } // Handles messages received from device: @@ -323,11 +321,10 @@ export default class Device { #handleMessageFromDevice(message: MessageFromDevice) { if (message.event === 'getPages') { this.#pages = new Map( - message.payload.map(({type, capabilities, ...page}) => [ + message.payload.map(({capabilities, ...page}) => [ page.id, { ...page, - type: type ?? 'Legacy', capabilities: capabilities ?? {}, }, ]), diff --git a/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js b/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js index 2a581c871dd278..066fa44cc3d536 100644 --- a/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js +++ b/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js @@ -152,7 +152,6 @@ export default class InspectorProxy implements InspectorProxyQueries { deviceName: device.getName(), reactNative: { logicalDeviceId: deviceId, - type: nullthrows(page.type), capabilities: nullthrows(page.capabilities), }, }; diff --git a/packages/dev-middleware/src/inspector-proxy/types.js b/packages/dev-middleware/src/inspector-proxy/types.js index 7ff48f1a83d526..f4994a812c4830 100644 --- a/packages/dev-middleware/src/inspector-proxy/types.js +++ b/packages/dev-middleware/src/inspector-proxy/types.js @@ -41,7 +41,6 @@ export type PageFromDevice = $ReadOnly<{ title: string, vm: string, app: string, - type?: 'Legacy' | 'Modern', capabilities?: TargetCapabilityFlags, }>; @@ -106,7 +105,6 @@ export type PageDescription = $ReadOnly<{ // Metadata specific to React Native reactNative: $ReadOnly<{ logicalDeviceId: string, - type: $NonMaybeType, capabilities: Page['capabilities'], }>, }>; diff --git a/packages/dev-middleware/src/middleware/openDebuggerMiddleware.js b/packages/dev-middleware/src/middleware/openDebuggerMiddleware.js index ea052c23d42170..1f3b4d74358521 100644 --- a/packages/dev-middleware/src/middleware/openDebuggerMiddleware.js +++ b/packages/dev-middleware/src/middleware/openDebuggerMiddleware.js @@ -63,8 +63,7 @@ export default function openDebuggerMiddleware({ // Only use targets with better reloading support app => app.title === 'React Native Experimental (Improved Chrome Reloads)' || - app.reactNative.capabilities?.nativePageReloads === true || - app.reactNative.type === 'Modern', + app.reactNative.capabilities?.nativePageReloads === true, ); let target;