Skip to content

Commit

Permalink
Remove matching of Modern/Legacy page types (#42885)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #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
  • Loading branch information
huntie authored and facebook-github-bot committed Feb 6, 2024
1 parent d75fc99 commit 9ba56f6
Show file tree
Hide file tree
Showing 7 changed files with 15 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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',
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,6 @@ describe('inspector proxy HTTP API', () => {
reactNative: {
capabilities: {},
logicalDeviceId: 'device1',
type: 'Legacy',
},
title: 'bar-title',
type: 'node',
Expand All @@ -209,7 +208,6 @@ describe('inspector proxy HTTP API', () => {
reactNative: {
capabilities: {},
logicalDeviceId: 'device2',
type: 'Legacy',
},
title: 'bar-title',
type: 'node',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
/***
Expand All @@ -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',
},
]);
Expand Down Expand Up @@ -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',
},
]);
Expand Down
9 changes: 3 additions & 6 deletions packages/dev-middleware/src/inspector-proxy/Device.js
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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<TargetCapabilityFlags>): boolean {
return page.type === 'Modern' || page.capabilities[flag] === true;
return page.capabilities[flag] === true;
}

// Handles messages received from device:
Expand All @@ -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 ?? {},
},
]),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ export default class InspectorProxy implements InspectorProxyQueries {
deviceName: device.getName(),
reactNative: {
logicalDeviceId: deviceId,
type: nullthrows(page.type),
capabilities: nullthrows(page.capabilities),
},
};
Expand Down
2 changes: 0 additions & 2 deletions packages/dev-middleware/src/inspector-proxy/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ export type PageFromDevice = $ReadOnly<{
title: string,
vm: string,
app: string,
type?: 'Legacy' | 'Modern',
capabilities?: TargetCapabilityFlags,
}>;

Expand Down Expand Up @@ -106,7 +105,6 @@ export type PageDescription = $ReadOnly<{
// Metadata specific to React Native
reactNative: $ReadOnly<{
logicalDeviceId: string,
type: $NonMaybeType<Page['type']>,
capabilities: Page['capabilities'],
}>,
}>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 9ba56f6

Please sign in to comment.