Skip to content

Commit

Permalink
Enable launching debugger by device ID (#41080)
Browse files Browse the repository at this point in the history
Summary:

Building on byCedric's approach in facebook/metro#991, adds support for passing a `device=...` argument to `/open-debugger` for more precise targeting.

Changelog: [Internal]

---

## Note on what "device" means in this context

In `dev-middleware` / `inspector-proxy`, "device" is something of a misnomer. It refers to a *logical device* containing one or more *pages*. In React Native, each app process forms its own logical device in which individual VMs register themselves as pages. An instance of `inspector-proxy` connects one or more *debuggers* (frontends) to one or more logical devices (one frontend to one page on one device).

The intent of the logical device ID is to help with target discovery and especially *re*discovery - to reduce the number of times users need to explicitly close and restart the debugger frontend (e.g. after an app crash).

If provided, the logical device ID:
1. SHOULD be stable for the current combination of physical device (or emulator instance) and app.
2. SHOULD be stable across installs/launches of the same app on the same device (or emulator instance), though it MAY be user-resettable (so as to not require any special privacy permissions).
3. MUST be unique across different apps on the same physical device (or emulator).
4. MUST be unique across physical devices (or emulators).
5. MUST be unique for each concurrent *instance* of the same app on the same physical device (or emulator).

NOTE: The uniqueness requirements are stronger (MUST) than the stability requirements (SHOULD). In particular, on platforms that allow multiple instances of the same app to run concurrently, requirements 1 and/or 2 MAY be violated in order to meet requirement 5. This will be relevant, for example, on desktop platforms.

In an upcoming diff, we will pass device IDs meeting these criteria from both iOS and Android.

Reviewed By: huntie, blakef

Differential Revision: D49954920
  • Loading branch information
motiz88 authored and facebook-github-bot committed Oct 20, 2023
1 parent c47bef6 commit 9a6191f
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 7 deletions.
3 changes: 3 additions & 0 deletions packages/dev-middleware/src/inspector-proxy/InspectorProxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ export default class InspectorProxy implements InspectorProxyQueries {
webSocketDebuggerUrl,
vm: page.vm,
deviceName: device.getName(),
reactNative: {
logicalDeviceId: deviceId,
},
};
}

Expand Down
4 changes: 4 additions & 0 deletions packages/dev-middleware/src/inspector-proxy/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ export type PageDescription = {
devtoolsFrontendUrl: string,
type: string,
webSocketDebuggerUrl: string,
// Metadata specific to React Native
reactNative: {
logicalDeviceId: string,
},
...
};
export type JsonPagesListResponse = Array<PageDescription>;
Expand Down
24 changes: 18 additions & 6 deletions packages/dev-middleware/src/middleware/openDebuggerMiddleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export default function openDebuggerMiddleware({
(experiments.enableOpenDebuggerRedirect && req.method === 'GET')
) {
const {query} = url.parse(req.url, true);
const {appId} = query;
const {appId, device}: {appId?: string, device?: string, ...} = query;

const targets = inspectorProxy.getPageDescriptions().filter(
// Only use targets with better reloading support
Expand All @@ -69,12 +69,19 @@ export default function openDebuggerMiddleware({
const launchType: 'launch' | 'redirect' =
req.method === 'POST' ? 'launch' : 'redirect';

if (typeof appId === 'string') {
if (typeof appId === 'string' || typeof device === 'string') {
logger?.info(
(launchType === 'launch' ? 'Launching' : 'Redirecting to') +
' JS debugger (experimental)...',
);
target = targets.find(_target => _target.description === appId);
if (typeof device === 'string') {
target = targets.find(
_target => _target.reactNative.logicalDeviceId === device,
);
}
if (!target && typeof appId === 'string') {
target = targets.find(_target => _target.description === appId);
}
} else {
logger?.info(
(launchType === 'launch' ? 'Launching' : 'Redirecting to') +
Expand All @@ -101,9 +108,13 @@ export default function openDebuggerMiddleware({
try {
switch (launchType) {
case 'launch':
await debuggerInstances.get(appId)?.kill();
const frontendInstanceId =
device != null
? 'device:' + device
: 'app:' + (appId ?? '<null>');
await debuggerInstances.get(frontendInstanceId)?.kill();
debuggerInstances.set(
appId,
frontendInstanceId,
await browserLauncher.launchDebuggerAppWindow(
getDevToolsFrontendUrl(
target.webSocketDebuggerUrl,
Expand All @@ -130,7 +141,8 @@ export default function openDebuggerMiddleware({
type: 'launch_debugger_frontend',
launchType,
status: 'success',
appId,
appId: appId ?? null,
deviceId: device ?? null,
});
return;
} catch (e) {
Expand Down
2 changes: 1 addition & 1 deletion packages/dev-middleware/src/types/EventReporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export type ReportableEvent =
type: 'launch_debugger_frontend',
launchType: 'launch' | 'redirect',
...
| SuccessResult<{appId: string}>
| SuccessResult<{appId: string | null, deviceId: string | null}>
| ErrorResult<mixed>
| CodedErrorResult<'NO_APPS_FOUND'>,
}
Expand Down

0 comments on commit 9a6191f

Please sign in to comment.