diff --git a/desktop/app/src/__tests__/disconnect.node.tsx b/desktop/app/src/__tests__/disconnect.node.tsx new file mode 100644 index 000000000..b3587beab --- /dev/null +++ b/desktop/app/src/__tests__/disconnect.node.tsx @@ -0,0 +1,108 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + */ + +import {default as BaseDevice} from '../devices/BaseDevice'; +import {createMockFlipperWithPlugin} from '../test-utils/createMockFlipperWithPlugin'; +import { + TestUtils, + _SandyPluginDefinition, + createState, + DevicePluginClient, +} from 'flipper-plugin'; + +test('Devices can disconnect', async () => { + const deviceplugin = new _SandyPluginDefinition( + TestUtils.createMockPluginDetails(), + { + devicePlugin(client: DevicePluginClient) { + const destroy = jest.fn(); + client.onDestroy(destroy); + const counter = createState(0); + return { + counter, + destroy, + }; + }, + supportsDevice() { + return true; + }, + Component() { + return null; + }, + }, + ); + const {device} = await createMockFlipperWithPlugin(deviceplugin); + + device.sandyPluginStates.get(deviceplugin.id)!.instanceApi.counter.set(1); + + expect(device.isArchived).toBe(false); + + device.markDisconnected(); + + expect(device.isArchived).toBe(true); + const instance = device.sandyPluginStates.get(deviceplugin.id)!; + expect(instance).toBeTruthy(); + expect(instance.instanceApi.counter.get(1)).toBe(1); // state preserved + expect(instance.instanceApi.destroy).toBeCalledTimes(0); + + device.destroy(); + expect(device.isArchived).toBe(true); + expect(instance.instanceApi.destroy).toBeCalledTimes(1); + + expect(device.sandyPluginStates.get(deviceplugin.id)).toBeUndefined(); +}); + +test('New device with same serial removes & cleans the old one', async () => { + const deviceplugin = new _SandyPluginDefinition( + TestUtils.createMockPluginDetails(), + { + devicePlugin(client: DevicePluginClient) { + const destroy = jest.fn(); + client.onDestroy(destroy); + return { + destroy, + }; + }, + supportsDevice() { + return true; + }, + Component() { + return null; + }, + }, + ); + const {device, store} = await createMockFlipperWithPlugin(deviceplugin); + + const instance = device.sandyPluginStates.get(deviceplugin.id)!; + + expect(device.isArchived).toBe(false); + expect(instance.instanceApi.destroy).toBeCalledTimes(0); + expect(store.getState().connections.devices).toEqual([device]); + + // submit a new device with same serial + const device2 = new BaseDevice( + device.serial, + 'physical', + 'MockAndroidDevice', + 'Android', + ); + store.dispatch({ + type: 'REGISTER_DEVICE', + payload: device2, + }); + device2.loadDevicePlugins(store.getState().plugins.devicePlugins); + + expect(device.isArchived).toBe(true); + expect(instance.instanceApi.destroy).toBeCalledTimes(1); + expect( + device2.sandyPluginStates.get(deviceplugin.id)!.instanceApi.destroy, + ).toBeCalledTimes(0); + expect(store.getState().connections.devices.length).toBe(1); + expect(store.getState().connections.devices[0]).toBe(device2); +}); diff --git a/desktop/app/src/devices/AndroidDevice.tsx b/desktop/app/src/devices/AndroidDevice.tsx index 8be72ddf3..227eb913d 100644 --- a/desktop/app/src/devices/AndroidDevice.tsx +++ b/desktop/app/src/devices/AndroidDevice.tsx @@ -89,16 +89,6 @@ export default class AndroidDevice extends BaseDevice { return this.executeShell(['logcat', '-c']); } - archive(): ArchivedDevice { - return new ArchivedDevice({ - serial: this.serial, - deviceType: this.deviceType, - title: this.title, - os: this.os, - screenshotHandle: null, - }); - } - navigateToLocation(location: string) { const shellCommand = `am start ${encodeURI(location)}`; this.adb.shell(this.serial, shellCommand); diff --git a/desktop/app/src/devices/ArchivedDevice.tsx b/desktop/app/src/devices/ArchivedDevice.tsx index 82b8b5a06..ef1e13fba 100644 --- a/desktop/app/src/devices/ArchivedDevice.tsx +++ b/desktop/app/src/devices/ArchivedDevice.tsx @@ -23,13 +23,13 @@ export default class ArchivedDevice extends BaseDevice { supportRequestDetails?: SupportFormRequestDetailsState; }) { super(options.serial, options.deviceType, options.title, options.os); + this.archivedState.set(true); this.source = options.source || ''; this.supportRequestDetails = options.supportRequestDetails; this.archivedScreenshotHandle = options.screenshotHandle; } archivedScreenshotHandle: string | null; - isArchived = true; displayTitle(): string { return `${this.title} ${this.source ? '(Imported)' : '(Offline)'}`; diff --git a/desktop/app/src/devices/BaseDevice.tsx b/desktop/app/src/devices/BaseDevice.tsx index 0d14ddfda..4695c462f 100644 --- a/desktop/app/src/devices/BaseDevice.tsx +++ b/desktop/app/src/devices/BaseDevice.tsx @@ -15,6 +15,7 @@ import { DeviceType, DeviceLogListener, Idler, + createState, } from 'flipper-plugin'; import type {DevicePluginDefinition, DevicePluginMap} from '../plugin'; import {getFlipperLibImplementation} from '../utils/flipperLibImplementation'; @@ -70,7 +71,12 @@ export default class BaseDevice { icon: string | null | undefined; logListeners: Map = new Map(); - isArchived: boolean = false; + + archivedState = createState(false); + get isArchived() { + return this.archivedState.get(); + } + // if imported, stores the original source location source = ''; @@ -87,7 +93,7 @@ export default class BaseDevice { } displayTitle(): string { - return this.title; + return this.isArchived ? `${this.title} (Offline)` : this.title; } async exportState( @@ -154,10 +160,6 @@ export default class BaseDevice { throw new Error('unimplemented'); } - archive(): any | null | undefined { - return null; - } - screenshot(): Promise { return Promise.reject( new Error('No screenshot support for current device'), @@ -218,4 +220,16 @@ export default class BaseDevice { } this.devicePlugins.splice(this.devicePlugins.indexOf(pluginId), 1); } + + markDisconnected() { + this.archivedState.set(true); + } + + destroy() { + this.markDisconnected(); + this.sandyPluginStates.forEach((instance) => { + instance.destroy(); + }); + this.sandyPluginStates.clear(); + } } diff --git a/desktop/app/src/dispatcher/androidDevice.tsx b/desktop/app/src/dispatcher/androidDevice.tsx index d61a2ff3e..834b0f4d9 100644 --- a/desktop/app/src/dispatcher/androidDevice.tsx +++ b/desktop/app/src/dispatcher/androidDevice.tsx @@ -262,28 +262,11 @@ export default (store: Store, logger: Logger) => { }), ); - const archivedDevices = deviceIds - .map((id) => { - const device = store - .getState() - .connections.devices.find((device) => device.serial === id); - if (device && !device.isArchived) { - return device.archive(); - } - }) - .filter(Boolean); - - store.dispatch({ - type: 'UNREGISTER_DEVICES', - payload: new Set(deviceIds), - }); - - archivedDevices.forEach((device: BaseDevice) => { - device.loadDevicePlugins(store.getState().plugins.devicePlugins); - store.dispatch({ - type: 'REGISTER_DEVICE', - payload: device, - }); + deviceIds.forEach((id) => { + const device = store + .getState() + .connections.devices.find((device) => device.serial === id); + device?.markDisconnected(); }); } diff --git a/desktop/app/src/dispatcher/iOSDevice.tsx b/desktop/app/src/dispatcher/iOSDevice.tsx index 1dc35a27e..972d337ac 100644 --- a/desktop/app/src/dispatcher/iOSDevice.tsx +++ b/desktop/app/src/dispatcher/iOSDevice.tsx @@ -148,15 +148,12 @@ function processDevices( } } - if (currentDeviceIDs.size > 0) { - currentDeviceIDs.forEach((id) => - logger.track('usage', 'unregister-device', {os: 'iOS', serial: id}), - ); - store.dispatch({ - type: 'UNREGISTER_DEVICES', - payload: currentDeviceIDs, - }); - } + currentDeviceIDs.forEach((id) => { + const device = store + .getState() + .connections.devices.find((device) => device.serial === id); + device?.markDisconnected(); + }); } function getDeviceSetPath() { diff --git a/desktop/app/src/dispatcher/metroDevice.tsx b/desktop/app/src/dispatcher/metroDevice.tsx index b19ce9cdd..c77d5644a 100644 --- a/desktop/app/src/dispatcher/metroDevice.tsx +++ b/desktop/app/src/dispatcher/metroDevice.tsx @@ -79,26 +79,10 @@ async function unregisterDevices(store: Store, logger: Logger) { serial: METRO_URL, }); - let archivedDevice: ArchivedDevice | undefined = undefined; - const device = store - .getState() - .connections.devices.find((device) => device.serial === METRO_URL); - if (device && !device.isArchived) { - archivedDevice = device.archive(); - } - store.dispatch({ type: 'UNREGISTER_DEVICES', payload: new Set([METRO_URL]), }); - - if (archivedDevice) { - archivedDevice.loadDevicePlugins(store.getState().plugins.devicePlugins); - store.dispatch({ - type: 'REGISTER_DEVICE', - payload: archivedDevice, - }); - } } export default (store: Store, logger: Logger) => { diff --git a/desktop/app/src/reducers/connections.tsx b/desktop/app/src/reducers/connections.tsx index 626654f3a..88f180d5f 100644 --- a/desktop/app/src/reducers/connections.tsx +++ b/desktop/app/src/reducers/connections.tsx @@ -197,6 +197,7 @@ export default (state: State = INITAL_STATE, action: Actions): State => { console.debug( `Got a new device instance for already existing serial ${payload.serial}`, ); + state.devices[existing].destroy(); newDevices[existing] = payload; } else { newDevices.push(payload); diff --git a/desktop/app/src/sandy-chrome/appinspect/AppInspect.tsx b/desktop/app/src/sandy-chrome/appinspect/AppInspect.tsx index 268d893f5..e46cc2903 100644 --- a/desktop/app/src/sandy-chrome/appinspect/AppInspect.tsx +++ b/desktop/app/src/sandy-chrome/appinspect/AppInspect.tsx @@ -11,7 +11,7 @@ import React from 'react'; import {Alert} from 'antd'; import {LeftSidebar, SidebarTitle, InfoIcon} from '../LeftSidebar'; import {Layout, Link, styled} from '../../ui'; -import {theme} from 'flipper-plugin'; +import {theme, useValue} from 'flipper-plugin'; import {AppSelector} from './AppSelector'; import {useStore} from '../../utils/useStore'; import {PluginList} from './PluginList'; @@ -52,7 +52,7 @@ export function AppInspect() { metroDevice, connections.userPreferredDevice, ]); - const isArchived = !!activeDevice?.isArchived; + const isArchived = useValue(activeDevice?.archivedState, false); return ( diff --git a/desktop/app/src/sandy-chrome/appinspect/AppSelector.tsx b/desktop/app/src/sandy-chrome/appinspect/AppSelector.tsx index a2dedc5c9..ff5085992 100644 --- a/desktop/app/src/sandy-chrome/appinspect/AppSelector.tsx +++ b/desktop/app/src/sandy-chrome/appinspect/AppSelector.tsx @@ -17,7 +17,7 @@ import { RocketOutlined, } from '@ant-design/icons'; import {Glyph, Layout, styled} from '../../ui'; -import {theme, useTrackedCallback} from 'flipper-plugin'; +import {theme, useTrackedCallback, useValue} from 'flipper-plugin'; import {batch} from 'react-redux'; import {useDispatch, useStore} from '../../utils/useStore'; import { @@ -56,6 +56,7 @@ export function AppSelector() { uninitializedClients, selectedApp, } = useStore((state) => state.connections); + useValue(selectedDevice?.archivedState, false); // subscribe to future archived state changes const onSelectDevice = useTrackedCallback( 'select-device', diff --git a/desktop/app/src/utils/__tests__/exportData.node.tsx b/desktop/app/src/utils/__tests__/exportData.node.tsx index bc0c3d793..9bf0c6ed2 100644 --- a/desktop/app/src/utils/__tests__/exportData.node.tsx +++ b/desktop/app/src/utils/__tests__/exportData.node.tsx @@ -21,7 +21,7 @@ import {FlipperPlugin, FlipperDevicePlugin} from '../../plugin'; import {Notification} from '../../plugin'; import {default as Client, ClientExport} from '../../Client'; import {State as PluginsState} from '../../reducers/plugins'; -import {renderMockFlipperWithPlugin} from '../../test-utils/createMockFlipperWithPlugin'; +import {createMockFlipperWithPlugin} from '../../test-utils/createMockFlipperWithPlugin'; import { TestUtils, _SandyPluginDefinition, @@ -1116,7 +1116,7 @@ const sandyTestPlugin = new _SandyPluginDefinition( ); test('Sandy plugins are exported properly', async () => { - const {client, sendMessage, store} = await renderMockFlipperWithPlugin( + const {client, sendMessage, store} = await createMockFlipperWithPlugin( sandyTestPlugin, ); @@ -1145,7 +1145,7 @@ test('Sandy plugins are exported properly', async () => { }); test('Sandy plugins with custom export are exported properly', async () => { - const {client, sendMessage, store} = await renderMockFlipperWithPlugin( + const {client, sendMessage, store} = await createMockFlipperWithPlugin( sandyTestPlugin, ); @@ -1218,7 +1218,7 @@ test('Sandy plugins are imported properly', async () => { }, }; - const {client, store} = await renderMockFlipperWithPlugin(sandyTestPlugin); + const {client, store} = await createMockFlipperWithPlugin(sandyTestPlugin); await importDataToStore('unittest.json', JSON.stringify(data), store); @@ -1312,7 +1312,7 @@ test('Sandy device plugins are exported / imported properly', async () => { }, }; - const {device, store} = await renderMockFlipperWithPlugin( + const {device, store} = await createMockFlipperWithPlugin( sandyDeviceTestPlugin, ); @@ -1350,7 +1350,7 @@ test('Sandy device plugins are exported / imported properly', async () => { }); test('Sandy device plugins with custom export are export properly', async () => { - const {device, store} = await renderMockFlipperWithPlugin( + const {device, store} = await createMockFlipperWithPlugin( sandyDeviceTestPlugin, ); @@ -1384,7 +1384,7 @@ test('Sandy plugin with custom import', async () => { }, ); - const {store} = await renderMockFlipperWithPlugin(plugin); + const {store} = await createMockFlipperWithPlugin(plugin); const data = { clients: [ @@ -1484,7 +1484,7 @@ test('Sandy device plugin with custom import', async () => { }, }; - const {store} = await renderMockFlipperWithPlugin(plugin); + const {store} = await createMockFlipperWithPlugin(plugin); await importDataToStore('unittest.json', JSON.stringify(data), store); @@ -1503,7 +1503,7 @@ test('Sandy device plugin with custom import', async () => { }); test('Sandy plugins with complex data are imported / exported correctly', async () => { - const deviceplugin = new _SandyPluginDefinition( + const plugin = new _SandyPluginDefinition( TestUtils.createMockPluginDetails(), { plugin() { @@ -1522,7 +1522,7 @@ test('Sandy plugins with complex data are imported / exported correctly', async }, ); - const {store} = await renderMockFlipperWithPlugin(deviceplugin); + const {store} = await createMockFlipperWithPlugin(plugin); const data = await exportStore(store); expect(Object.values(data.exportStoreData.pluginStates2)).toMatchObject([ @@ -1551,7 +1551,7 @@ test('Sandy plugins with complex data are imported / exported correctly', async await importDataToStore('unittest.json', data.serializedString, store); const api = store .getState() - .connections.clients[1].sandyPluginStates.get(deviceplugin.id)?.instanceApi; + .connections.clients[1].sandyPluginStates.get(plugin.id)?.instanceApi; expect(api.m.get()).toMatchInlineSnapshot(` Map { "a" => 1, @@ -1590,7 +1590,7 @@ test('Sandy device plugins with complex data are imported / exported correctly' }, ); - const {store} = await renderMockFlipperWithPlugin(deviceplugin); + const {store} = await createMockFlipperWithPlugin(deviceplugin); const data = await exportStore(store); expect(data.exportStoreData.device?.pluginStates).toMatchObject({ diff --git a/desktop/flipper-plugin/src/plugin/PluginBase.tsx b/desktop/flipper-plugin/src/plugin/PluginBase.tsx index e18741ff6..60bc4494c 100644 --- a/desktop/flipper-plugin/src/plugin/PluginBase.tsx +++ b/desktop/flipper-plugin/src/plugin/PluginBase.tsx @@ -132,7 +132,9 @@ export abstract class BasePluginInstance { realDevice, // TODO: temporarily, clean up T70688226 // N.B. we model OS as string, not as enum, to make custom device types possible in the future os: realDevice.os, - isArchived: realDevice.isArchived, + get isArchived() { + return realDevice.isArchived; + }, deviceType: realDevice.deviceType, onLogEntry(cb) { diff --git a/desktop/flipper-plugin/src/state/atom.tsx b/desktop/flipper-plugin/src/state/atom.tsx index 7c6d86525..f66b17cfc 100644 --- a/desktop/flipper-plugin/src/state/atom.tsx +++ b/desktop/flipper-plugin/src/state/atom.tsx @@ -84,9 +84,16 @@ export function createState( return atom; } -export function useValue(atom: Atom): T { - const [localValue, setLocalValue] = useState(atom.get()); +export function useValue(atom: Atom): T; +export function useValue(atom: Atom | undefined, defaultValue: T): T; +export function useValue(atom: Atom | undefined, defaultValue?: T): T { + const [localValue, setLocalValue] = useState( + atom ? atom.get() : defaultValue!, + ); useEffect(() => { + if (!atom) { + return; + } // atom might have changed between mounting and effect setup // in that case, this will cause a re-render, otherwise not setLocalValue(atom.get());