Fix inconsistent handling of app id and name

Summary:
Changelog: Improved plugin / device / app selection handing.

During some refactorings I discovered that the `connetions.selectedApp` field contained sometimes an application id, and sometimes just the name. This caused inconsistent behavior especially in unit tests.

I've cleaned that up, and renamed it to `selectedAppId` where applicable, to make the distinction more clear.

And, in contrast, userPreferredApp now always has a name, not an id.

During refactoring our existing selection update logic was quite in the way, which was overcomplicated still, since during the sandy chrome migration, the reducers needed to be able to handle both the old UI, and the new application selection UI. That logic has been simplified now, and a lot of tests were added.

As a further simplification the preferredApp/Device/Plugin are now only read and used when updating selection, but not when running selectors.

Reviewed By: timur-valiev

Differential Revision: D31305180

fbshipit-source-id: 2dbd9f9c33950227cc63aa29cc4a98bdd0db8e7a
This commit is contained in:
Michel Weststrate
2021-10-04 07:26:11 -07:00
committed by Facebook GitHub Bot
parent ba89daf12c
commit c9a34d3cc2
30 changed files with 497 additions and 280 deletions

View File

@@ -52,11 +52,7 @@ type StateFromProps = {
};
type DispatchFromProps = {
selectPlugin: (payload: {
selectedPlugin: string | null;
selectedApp: string | null;
deepLinkPayload: unknown;
}) => any;
selectPlugin: typeof selectPlugin;
updatePluginBlocklist: (blocklist: Array<string>) => any;
updateCategoryBlocklist: (blocklist: Array<string>) => any;
};
@@ -410,11 +406,7 @@ type ItemProps = {
onClear?: () => any;
isSelected?: boolean;
inactive?: boolean;
selectPlugin?: (payload: {
selectedPlugin: string | null;
selectedApp: string | null;
deepLinkPayload: unknown;
}) => any;
selectPlugin?: typeof selectPlugin;
logger?: Logger;
plugin: PluginDefinition | null | undefined;
};
@@ -477,7 +469,7 @@ class NotificationItem extends Component<
if (this.props.selectPlugin && notification.action) {
this.props.selectPlugin({
selectedPlugin: pluginId,
selectedApp: client,
selectedAppId: client,
deepLinkPayload: notification.action,
});
}

View File

@@ -101,11 +101,7 @@ type StateFromProps = {
};
type DispatchFromProps = {
selectPlugin: (payload: {
selectedPlugin: string | null;
selectedApp?: string | null;
deepLinkPayload: unknown;
}) => any;
selectPlugin: typeof selectPlugin;
setStaticView: (payload: StaticView) => void;
enablePlugin: typeof switchPlugin;
loadPlugin: typeof loadPlugin;

View File

@@ -249,6 +249,7 @@ test('PluginContainer can render Sandy plugins', async () => {
act(() => {
store.dispatch(
selectPlugin({
selectedAppId: client.id,
selectedPlugin: 'Logs',
deepLinkPayload: null,
}),
@@ -279,6 +280,7 @@ test('PluginContainer can render Sandy plugins', async () => {
act(() => {
store.dispatch(
selectPlugin({
selectedAppId: client.id,
selectedPlugin: definition.id,
deepLinkPayload: null,
}),
@@ -405,6 +407,8 @@ test('PluginContainer triggers correct lifecycles for background plugin', async
act(() => {
store.dispatch(
selectPlugin({
selectedAppId: client.id,
selectedPlugin: 'Logs',
deepLinkPayload: null,
}),
@@ -422,6 +426,8 @@ test('PluginContainer triggers correct lifecycles for background plugin', async
act(() => {
store.dispatch(
selectPlugin({
selectedAppId: client.id,
selectedPlugin: definition.id,
deepLinkPayload: null,
}),
@@ -454,6 +460,8 @@ test('PluginContainer triggers correct lifecycles for background plugin', async
act(() => {
store.dispatch(
selectPlugin({
selectedAppId: client.id,
selectedPlugin: 'Logs',
deepLinkPayload: null,
}),
@@ -487,6 +495,7 @@ test('PluginContainer triggers correct lifecycles for background plugin', async
act(() => {
store.dispatch(
selectPlugin({
selectedAppId: client.id,
selectedPlugin: definition.id,
deepLinkPayload: null,
}),
@@ -566,7 +575,7 @@ test('PluginContainer + Sandy plugin supports deeplink', async () => {
selectPlugin({
selectedPlugin: definition.id,
deepLinkPayload: 'universe!',
selectedApp: client.query.app,
selectedAppId: client.id,
}),
);
});
@@ -607,7 +616,7 @@ test('PluginContainer + Sandy plugin supports deeplink', async () => {
selectPlugin({
selectedPlugin: definition.id,
deepLinkPayload: 'universe!',
selectedApp: client.query.app,
selectedAppId: client.id,
}),
);
});
@@ -630,7 +639,7 @@ test('PluginContainer + Sandy plugin supports deeplink', async () => {
selectPlugin({
selectedPlugin: definition.id,
deepLinkPayload: 'london!',
selectedApp: client.query.app,
selectedAppId: client.id,
}),
);
});
@@ -643,7 +652,7 @@ test('PluginContainer + Sandy plugin supports deeplink', async () => {
selectPlugin({
selectedPlugin: 'Logs',
deepLinkPayload: 'london!',
selectedApp: client.query.app,
selectedAppId: client.id,
}),
);
});
@@ -652,7 +661,7 @@ test('PluginContainer + Sandy plugin supports deeplink', async () => {
selectPlugin({
selectedPlugin: definition.id,
deepLinkPayload: 'london!',
selectedApp: client.query.app,
selectedAppId: client.id,
}),
);
});
@@ -783,6 +792,7 @@ test('PluginContainer can render Sandy device plugins', async () => {
act(() => {
store.dispatch(
selectPlugin({
selectedDevice: device,
selectedPlugin: 'Logs',
deepLinkPayload: null,
}),
@@ -801,6 +811,7 @@ test('PluginContainer can render Sandy device plugins', async () => {
act(() => {
store.dispatch(
selectPlugin({
selectedDevice: device,
selectedPlugin: definition.id,
deepLinkPayload: null,
}),
@@ -836,7 +847,9 @@ test('PluginContainer + Sandy device plugin supports deeplink', async () => {
},
},
);
const {renderer, act, store} = await renderMockFlipperWithPlugin(definition);
const {renderer, act, store, device} = await renderMockFlipperWithPlugin(
definition,
);
const theUniverse = {
thisIs: 'theUniverse',
@@ -877,9 +890,10 @@ test('PluginContainer + Sandy device plugin supports deeplink', async () => {
act(() => {
store.dispatch(
selectPlugin({
selectedDevice: device,
selectedPlugin: definition.id,
deepLinkPayload: theUniverse,
selectedApp: null,
selectedAppId: null,
}),
);
});
@@ -918,9 +932,10 @@ test('PluginContainer + Sandy device plugin supports deeplink', async () => {
act(() => {
store.dispatch(
selectPlugin({
selectedDevice: device,
selectedPlugin: definition.id,
deepLinkPayload: theUniverse,
selectedApp: null,
selectedAppId: null,
}),
);
});
@@ -941,9 +956,10 @@ test('PluginContainer + Sandy device plugin supports deeplink', async () => {
act(() => {
store.dispatch(
selectPlugin({
selectedDevice: device,
selectedPlugin: definition.id,
deepLinkPayload: 'london!',
selectedApp: null,
selectedAppId: null,
}),
);
});
@@ -954,18 +970,20 @@ test('PluginContainer + Sandy device plugin supports deeplink', async () => {
act(() => {
store.dispatch(
selectPlugin({
selectedDevice: device,
selectedPlugin: 'Logs',
deepLinkPayload: 'london!',
selectedApp: null,
selectedAppId: null,
}),
);
});
act(() => {
store.dispatch(
selectPlugin({
selectedDevice: device,
selectedPlugin: definition.id,
deepLinkPayload: 'london!',
selectedApp: null,
selectedAppId: null,
}),
);
});
@@ -1203,6 +1221,7 @@ test('PluginContainer can render Sandy plugins for archived devices', async () =
act(() => {
store.dispatch(
selectPlugin({
selectedAppId: client.id,
selectedPlugin: 'Logs',
deepLinkPayload: null,
}),
@@ -1225,6 +1244,8 @@ test('PluginContainer can render Sandy plugins for archived devices', async () =
act(() => {
store.dispatch(
selectPlugin({
selectedAppId: client.id,
selectedPlugin: definition.id,
deepLinkPayload: null,
}),
@@ -1315,6 +1336,8 @@ test('PluginContainer triggers correct lifecycles for background plugin', async
act(() => {
store.dispatch(
selectPlugin({
selectedAppId: client.id,
selectedPlugin: 'Logs',
deepLinkPayload: null,
}),
@@ -1331,6 +1354,8 @@ test('PluginContainer triggers correct lifecycles for background plugin', async
act(() => {
store.dispatch(
selectPlugin({
selectedAppId: client.id,
selectedPlugin: definition.id,
deepLinkPayload: null,
}),
@@ -1346,6 +1371,7 @@ test('PluginContainer triggers correct lifecycles for background plugin', async
act(() => {
store.dispatch(
selectPlugin({
selectedAppId: client.id,
selectedPlugin: 'Logs',
deepLinkPayload: null,
}),
@@ -1355,6 +1381,7 @@ test('PluginContainer triggers correct lifecycles for background plugin', async
act(() => {
store.dispatch(
selectPlugin({
selectedAppId: client.id,
selectedPlugin: definition.id,
deepLinkPayload: null,
}),

View File

@@ -36,7 +36,7 @@ Object {
],
},
"flipperServer": undefined,
"selectedApp": "TestApp#Android#MockAndroidDevice#serial",
"selectedAppId": "TestApp#Android#MockAndroidDevice#serial",
"selectedAppPluginListRevision": 0,
"selectedDevice": Object {
"deviceType": "physical",
@@ -47,7 +47,7 @@ Object {
"selectedPlugin": "TestPlugin",
"staticView": null,
"uninitializedClients": Array [],
"userPreferredApp": "TestApp#Android#MockAndroidDevice#serial",
"userPreferredApp": "TestApp",
"userPreferredDevice": "MockAndroidDevice",
"userPreferredPlugin": "TestPlugin",
}

View File

@@ -112,11 +112,32 @@ export async function handleDeeplink(
match[1]
}&client=${match[0]}&payload=${encodeURIComponent(match[2])}' instead.`,
);
const deepLinkPayload = match[2];
const deepLinkParams = new URLSearchParams(deepLinkPayload);
const deviceParam = deepLinkParams.get('device');
// if there is a device Param, find a matching device
const selectedDevice = deviceParam
? store
.getState()
.connections.devices.find((v) => v.title === deviceParam)
: undefined;
// if a client is specified, find it, withing the device if applicable
const selectedClient = store
.getState()
.connections.clients.find(
(c) =>
c.query.app === match[0] &&
(selectedDevice == null || c.device === selectedDevice),
);
store.dispatch(
selectPlugin({
selectedApp: match[0],
selectedAppId: selectedClient?.id,
selectedDevice: selectedClient ? selectedClient.device : selectedDevice,
selectedPlugin: match[1],
deepLinkPayload: match[2],
deepLinkPayload,
}),
);
return;

View File

@@ -139,11 +139,15 @@ test('triggering a deeplink without applicable device can wait for a device', as
supportedDevices: [{os: 'iOS'}],
},
);
const {renderer, store, logger, createDevice} =
const {renderer, store, logger, createDevice, device} =
await renderMockFlipperWithPlugin(definition);
store.dispatch(
selectPlugin({selectedPlugin: 'nonexisting', deepLinkPayload: null}),
selectPlugin({
selectedPlugin: 'nonexisting',
deepLinkPayload: null,
selectedDevice: device,
}),
);
expect(renderer.baseElement).toMatchInlineSnapshot(`
<body>
@@ -219,7 +223,11 @@ test('triggering a deeplink without applicable client can wait for a device', as
await renderMockFlipperWithPlugin(definition);
store.dispatch(
selectPlugin({selectedPlugin: 'nonexisting', deepLinkPayload: null}),
selectPlugin({
selectedPlugin: 'nonexisting',
deepLinkPayload: null,
selectedDevice: device,
}),
);
expect(renderer.baseElement).toMatchInlineSnapshot(`
<body>

View File

@@ -173,11 +173,6 @@ export async function handleClientConnected(store: Store, client: Client) {
payload: client,
});
const device = client.device;
if (device) {
store.dispatch(selectDevice(device));
store.dispatch(selectClient(client.id));
}
store.dispatch(selectClient(client.id));
client.emit('plugins-change');
}

View File

@@ -121,7 +121,7 @@ export async function handleOpenPluginDeeplink(store: Store, query: string) {
store.dispatch(
selectPlugin({
selectedPlugin: params.pluginId,
selectedApp: null,
selectedAppId: null,
selectedDevice: device,
deepLinkPayload: params.payload,
}),
@@ -130,7 +130,7 @@ export async function handleOpenPluginDeeplink(store: Store, query: string) {
store.dispatch(
selectPlugin({
selectedPlugin: params.pluginId,
selectedApp: client!.query.app,
selectedAppId: client!.id,
selectedDevice: device,
deepLinkPayload: params.payload,
}),

View File

@@ -166,12 +166,12 @@ function updatePlugin(store: Store, payload: UpdatePluginActionPayload) {
}
}
function getSelectedAppId(store: Store) {
function getSelectedAppName(store: Store) {
const {connections} = store.getState();
const selectedApp = connections.selectedApp
? deconstructClientId(connections.selectedApp).app
const selectedAppId = connections.selectedAppId
? deconstructClientId(connections.selectedAppId).app
: undefined;
return selectedApp;
return selectedAppId;
}
function switchPlugin(
@@ -190,7 +190,7 @@ function switchClientPlugin(
plugin: PluginDefinition,
selectedApp: string | undefined,
) {
selectedApp = selectedApp ?? getSelectedAppId(store);
selectedApp = selectedApp ?? getSelectedAppName(store);
if (!selectedApp) {
return;
}
@@ -242,7 +242,7 @@ function updateClientPlugin(
) {
const clients = store.getState().connections.clients;
if (enable) {
const selectedApp = getSelectedAppId(store);
const selectedApp = getSelectedAppName(store);
if (selectedApp) {
store.dispatch(setPluginEnabled(plugin.id, selectedApp));
}

View File

@@ -108,12 +108,12 @@ export default (store: Store, logger: Logger) => {
timeSinceLastStartup,
});
// create fresh exit data
const {selectedDevice, selectedApp, selectedPlugin} =
const {selectedDevice, selectedAppId, selectedPlugin} =
store.getState().connections;
persistExitData(
{
selectedDevice,
selectedApp,
selectedAppId,
selectedPlugin,
},
false,
@@ -158,11 +158,11 @@ export default (store: Store, logger: Logger) => {
);
return;
}
const {selectedDevice, selectedPlugin, selectedApp, clients} =
const {selectedDevice, selectedPlugin, selectedAppId, clients} =
state.connections;
persistExitData(
{selectedDevice, selectedPlugin, selectedApp},
{selectedDevice, selectedPlugin, selectedAppId},
args[0] === 'exit',
);
@@ -224,8 +224,8 @@ export default (store: Store, logger: Logger) => {
let app: string | null = null;
let sdkVersion: number | null = null;
if (selectedApp) {
const client = clients.find((c: Client) => c.id === selectedApp);
if (selectedAppId) {
const client = clients.find((c: Client) => c.id === selectedAppId);
if (client) {
app = client.query.app;
sdkVersion = client.query.sdk_version || 0;
@@ -357,7 +357,7 @@ export function persistExitData(
state: {
selectedDevice: BaseDevice | null;
selectedPlugin: string | null;
selectedApp: string | null;
selectedAppId: string | null;
},
cleanExit: boolean,
) {
@@ -370,7 +370,9 @@ export function persistExitData(
deviceType: state.selectedDevice ? state.selectedDevice.deviceType : '',
deviceTitle: state.selectedDevice ? state.selectedDevice.title : '',
plugin: state.selectedPlugin || '',
app: state.selectedApp ? deconstructClientId(state.selectedApp).app : '',
app: state.selectedAppId
? deconstructClientId(state.selectedAppId).app
: '',
cleanExit,
pid: remote.process.pid,
};

View File

@@ -72,7 +72,7 @@ export type Props<T> = {
deepLinkPayload: unknown;
selectPlugin: (pluginID: string, deepLinkPayload: unknown) => void;
isArchivedDevice: boolean;
selectedApp: string | null;
selectedApp: string | null; // name
setStaticView: (payload: StaticView) => void;
settingsState: Settings;
};

View File

@@ -7,7 +7,7 @@
* @format
*/
import reducer from '../connections';
import reducer, {selectClient, selectDevice} from '../connections';
import {State, selectPlugin} from '../connections';
import {
_SandyPluginDefinition,
@@ -16,7 +16,14 @@ import {
MockedConsole,
} from 'flipper-plugin';
import {TestDevice} from '../../test-utils/TestDevice';
import {createMockFlipperWithPlugin} from '../../test-utils/createMockFlipperWithPlugin';
import {
createMockFlipperWithPlugin,
MockFlipperResult,
} from '../../test-utils/createMockFlipperWithPlugin';
import {Store} from '..';
import {getActiveClient, getActiveDevice} from '../../selectors/connections';
import BaseDevice from '../../devices/BaseDevice';
import Client from '../../Client';
let mockedConsole: MockedConsole;
beforeEach(() => {
@@ -81,9 +88,23 @@ test('register, remove, re-register a metro device works correctly', () => {
});
test('selectPlugin sets deepLinkPayload correctly', () => {
const state = reducer(
const device1 = new TestDevice(
'http://localhost:8081',
'emulator',
'React Native',
'Metro',
);
let state = reducer(undefined, {
type: 'REGISTER_DEVICE',
payload: device1,
});
state = reducer(
undefined,
selectPlugin({selectedPlugin: 'myPlugin', deepLinkPayload: 'myPayload'}),
selectPlugin({
selectedPlugin: 'myPlugin',
deepLinkPayload: 'myPayload',
selectedDevice: device1,
}),
);
expect(state.deepLinkPayload).toBe('myPayload');
});
@@ -172,3 +193,221 @@ test('can handle device plugins that throw at start', async () => {
`);
expect(device2.sandyPluginStates.size).toBe(0);
});
describe('selection changes', () => {
const TestPlugin1 = new _SandyPluginDefinition(
TestUtils.createMockPluginDetails(),
{
Component() {
return null;
},
plugin() {
return {};
},
},
);
const TestPlugin2 = new _SandyPluginDefinition(
TestUtils.createMockPluginDetails(),
{
Component() {
return null;
},
plugin() {
return {};
},
},
);
const DevicePlugin1 = new _SandyPluginDefinition(
TestUtils.createMockPluginDetails({pluginType: 'device'}),
{
Component() {
return null;
},
devicePlugin() {
return {};
},
},
);
let device1: BaseDevice;
let device2: BaseDevice;
let metroDevice: BaseDevice;
let d1app1: Client;
let d1app2: Client;
let d2app1: Client;
let d2app2: Client;
let store: Store;
let mockFlipper: MockFlipperResult;
beforeEach(async () => {
mockFlipper = await createMockFlipperWithPlugin(TestPlugin1, {
additionalPlugins: [TestPlugin2, DevicePlugin1],
supportedPlugins: [TestPlugin1.id, TestPlugin2.id, DevicePlugin1.id],
});
device1 = mockFlipper.device;
device2 = mockFlipper.createDevice({});
metroDevice = mockFlipper.createDevice({
os: 'Metro',
serial: 'http://localhost:8081',
});
d1app1 = mockFlipper.client;
d1app2 = await mockFlipper.createClient(device1, 'd1app2');
d2app1 = await mockFlipper.createClient(device2, 'd2app1');
d2app2 = await mockFlipper.createClient(device2, 'd2app2');
store = mockFlipper.store;
});
test('basic/ device selection change', async () => {
// after registering d1app2, this will have become the selection
expect(store.getState().connections).toMatchObject({
selectedDevice: device1,
selectedPlugin: TestPlugin1.id,
selectedAppId: d1app2.id,
// no preferences changes, no explicit selection was made
userPreferredDevice: device1.title,
userPreferredPlugin: TestPlugin1.id,
userPreferredApp: d1app1.query.app,
});
expect(getActiveClient(store.getState())).toBe(d1app2);
expect(getActiveDevice(store.getState())).toBe(device1);
// select plugin 2 on d2app2
store.dispatch(
selectPlugin({
selectedPlugin: TestPlugin2.id,
selectedAppId: d2app2.id,
}),
);
expect(store.getState().connections).toMatchObject({
selectedDevice: device2,
selectedPlugin: TestPlugin2.id,
selectedAppId: d2app2.id,
userPreferredDevice: device2.title,
userPreferredPlugin: TestPlugin2.id,
userPreferredApp: d2app2.query.app,
});
// disconnect device1, and then register a new device should select it
device1.disconnect();
const device3 = await mockFlipper.createDevice({});
expect(store.getState().connections).toMatchObject({
selectedDevice: device3,
selectedPlugin: TestPlugin2.id,
selectedAppId: null,
// prefs not updated
userPreferredDevice: device2.title,
userPreferredPlugin: TestPlugin2.id,
userPreferredApp: d2app2.query.app,
});
store.dispatch(selectDevice(device1));
expect(store.getState().connections).toMatchObject({
selectedDevice: device1,
selectedPlugin: TestPlugin2.id,
selectedAppId: null,
userPreferredDevice: device1.title,
// other prefs not updated
userPreferredPlugin: TestPlugin2.id,
userPreferredApp: d2app2.query.app,
});
// used by plugin list, to keep main device / app selection correct
expect(getActiveClient(store.getState())).toBe(null);
expect(getActiveDevice(store.getState())).toBe(device1);
});
test('select a metro device', async () => {
store.dispatch(
selectPlugin({
selectedPlugin: DevicePlugin1.id,
selectedDevice: metroDevice,
selectedAppId: d2app1.id, // this app will determine the active device
}),
);
const state = store.getState();
expect(state.connections).toMatchObject({
selectedDevice: metroDevice,
selectedPlugin: DevicePlugin1.id,
selectedAppId: d2app1.id,
userPreferredDevice: metroDevice.title,
// other prefs not updated
userPreferredPlugin: DevicePlugin1.id,
userPreferredApp: d2app1.query.app,
});
// used by plugin list, to keep main device / app selection correct
expect(getActiveClient(state)).toBe(d2app1);
expect(getActiveDevice(state)).toBe(device2);
});
test('introducing new client does not select it', async () => {
await mockFlipper.createClient(device2, 'd2app3');
expect(store.getState().connections).toMatchObject({
selectedDevice: device1,
selectedPlugin: TestPlugin1.id,
selectedAppId: d1app2.id,
// other prefs not updated
userPreferredDevice: device1.title,
userPreferredPlugin: TestPlugin1.id,
userPreferredApp: d1app1.query.app,
});
});
test('introducing new client does select it if preferred', async () => {
// pure testing evil
const client3 = await mockFlipper.createClient(
device2,
store.getState().connections.userPreferredApp!,
);
expect(store.getState().connections).toMatchObject({
selectedDevice: device2,
selectedPlugin: TestPlugin1.id,
selectedAppId: client3.id,
// other prefs not updated
userPreferredDevice: device1.title,
userPreferredPlugin: TestPlugin1.id,
userPreferredApp: d1app1.query.app,
});
});
test('introducing new client does select it if old is offline', async () => {
d1app2.disconnect();
const client3 = await mockFlipper.createClient(device2, 'd2app3');
expect(store.getState().connections).toMatchObject({
selectedDevice: device2,
selectedPlugin: TestPlugin1.id,
selectedAppId: client3.id,
// other prefs not updated
userPreferredDevice: device1.title,
userPreferredPlugin: TestPlugin1.id,
userPreferredApp: d1app1.query.app,
});
});
test('select client', () => {
store.dispatch(selectClient(d2app2.id));
expect(store.getState().connections).toMatchObject({
selectedDevice: device2,
selectedPlugin: TestPlugin1.id,
selectedAppId: d2app2.id,
userPreferredDevice: device2.title,
userPreferredPlugin: TestPlugin1.id,
userPreferredApp: d2app2.query.app,
});
});
test('select device', () => {
store.dispatch(selectDevice(metroDevice));
expect(store.getState().connections).toMatchObject({
selectedDevice: metroDevice,
selectedPlugin: TestPlugin1.id,
selectedAppId: null,
userPreferredDevice: metroDevice.title,
// other prefs not updated
userPreferredPlugin: TestPlugin1.id,
userPreferredApp: d1app1.query.app,
});
});
});

View File

@@ -56,7 +56,7 @@ function selectTestPlugin(store: Store) {
store.dispatch(
selectPlugin({
selectedPlugin: TestPlugin.id,
selectedApp: null,
selectedAppId: null,
deepLinkPayload: null,
selectedDevice: store.getState().connections.selectedDevice!,
}),

View File

@@ -64,10 +64,10 @@ type StateV2 = {
devices: Array<BaseDevice>;
selectedDevice: null | BaseDevice;
selectedPlugin: null | string;
selectedApp: null | string;
selectedAppId: null | string; // Full quantified identifier of the app
userPreferredDevice: null | string;
userPreferredPlugin: null | string;
userPreferredApp: null | string;
userPreferredApp: null | string; // The name of the preferred app, e.g. Facebook
enabledPlugins: {[client: string]: string[]};
enabledDevicePlugins: Set<string>;
clients: Array<Client>;
@@ -100,17 +100,13 @@ export type Action =
| {
type: 'SELECT_PLUGIN';
payload: {
selectedPlugin: null | string;
selectedApp?: null | string;
deepLinkPayload: unknown;
selectedDevice?: null | BaseDevice;
selectedPlugin: string;
selectedAppId?: null | string; // not set for device plugins
deepLinkPayload?: unknown;
selectedDevice?: BaseDevice | null;
time: number;
};
}
| {
type: 'SELECT_USER_PREFERRED_PLUGIN';
payload: string;
}
| {
type: 'NEW_CLIENT';
payload: Client;
@@ -119,10 +115,6 @@ export type Action =
type: 'CLIENT_REMOVED';
payload: string;
}
| {
type: 'PREFER_DEVICE';
payload: string;
}
| {
type: 'START_CLIENT_SETUP';
payload: UninitializedClient;
@@ -160,7 +152,7 @@ export type Action =
}
| {
type: 'SELECT_CLIENT';
payload: string | null;
payload: string; // App ID
}
| {
type: 'APP_PLUGIN_LIST_CHANGED';
@@ -176,7 +168,7 @@ const DEFAULT_DEVICE_BLACKLIST: DeviceOS[] = ['MacOS', 'Metro', 'Windows'];
const INITAL_STATE: State = {
devices: [],
selectedDevice: null,
selectedApp: null,
selectedAppId: null,
selectedPlugin: DEFAULT_PLUGIN,
userPreferredDevice: null,
userPreferredPlugin: null,
@@ -208,32 +200,31 @@ export default (state: State = INITAL_STATE, action: Actions): State => {
case 'SET_STATIC_VIEW': {
const {payload, deepLinkPayload} = action;
const {selectedPlugin} = state;
return {
...state,
staticView: payload,
selectedPlugin: payload != null ? null : selectedPlugin,
deepLinkPayload: deepLinkPayload ?? null,
};
}
case 'RESET_SUPPORT_FORM_V2_STATE': {
return updateSelection({
return {
...state,
staticView: null,
});
};
}
case 'SELECT_DEVICE': {
const {payload} = action;
return updateSelection({
return {
...state,
staticView: null,
selectedDevice: payload,
selectedAppId: null,
userPreferredDevice: payload
? payload.title
: state.userPreferredDevice,
});
};
}
case 'REGISTER_DEVICE': {
@@ -253,65 +244,66 @@ export default (state: State = INITAL_STATE, action: Actions): State => {
newDevices.push(payload);
}
return updateSelection({
const selectNewDevice =
!state.selectedDevice ||
!state.selectedDevice.isConnected ||
state.userPreferredDevice === payload.title;
let selectedAppId = state.selectedAppId;
if (selectNewDevice) {
// need to select a different app
selectedAppId =
state.clients.find(
(c) =>
c.device === payload && c.query.app === state.userPreferredApp,
)?.id ?? null;
// nothing found, try first app if any
if (!selectedAppId) {
selectedAppId =
state.clients.find((c) => c.device === payload)?.id ?? null;
}
}
return {
...state,
devices: newDevices,
});
selectedDevice: selectNewDevice ? payload : state.selectedDevice,
selectedAppId,
};
}
case 'SELECT_PLUGIN': {
const {payload} = action;
const {selectedPlugin, selectedApp, deepLinkPayload} = payload;
let selectedDevice = payload.selectedDevice;
if (typeof deepLinkPayload === 'string') {
const deepLinkParams = new URLSearchParams(deepLinkPayload);
const deviceParam = deepLinkParams.get('device');
if (deviceParam) {
const deviceMatch = state.devices.find(
(v) => v.title === deviceParam,
);
if (deviceMatch) {
selectedDevice = deviceMatch;
} else {
console.warn(
`Could not find matching device "${deviceParam}" requested through deep-link.`,
);
}
}
}
if (!selectedDevice && selectedPlugin) {
const selectedClient = state.clients.find((c) =>
c.supportsPlugin(selectedPlugin),
);
selectedDevice = state.devices.find(
(v) => v.serial === selectedClient?.query.device_id,
);
}
if (!selectedDevice) {
console.warn('Trying to select a plugin before a device was selected!');
}
const {selectedPlugin, selectedAppId, deepLinkPayload} = action.payload;
if (selectedPlugin) {
performance.mark(`activePlugin-${selectedPlugin}`);
}
return updateSelection({
const client = state.clients.find((c) => c.id === selectedAppId);
const device = action.payload.selectedDevice ?? client?.device;
if (!device) {
console.warn(
'No valid device / client provided when calling SELECT_PLUGIN',
);
return state;
}
return {
...state,
staticView: null,
selectedApp: selectedApp || null,
selectedDevice: device,
userPreferredDevice: canBeDefaultDevice(device)
? device.title
: state.userPreferredDevice,
selectedAppId: selectedAppId ?? null,
userPreferredApp:
state.clients.find((c) => c.id === selectedAppId)?.query.app ??
state.userPreferredApp,
selectedPlugin,
userPreferredPlugin: selectedPlugin || state.userPreferredPlugin,
selectedDevice: selectedDevice!,
userPreferredDevice:
selectedDevice && canBeDefaultDevice(selectedDevice)
? selectedDevice.title
: state.userPreferredDevice,
userPreferredPlugin: selectedPlugin,
deepLinkPayload: deepLinkPayload,
});
}
case 'SELECT_USER_PREFERRED_PLUGIN': {
const {payload} = action;
return {...state, userPreferredPlugin: payload};
};
}
case 'NEW_CLIENT': {
@@ -328,8 +320,18 @@ export default (state: State = INITAL_STATE, action: Actions): State => {
});
newClients.push(payload);
return updateSelection({
// select new client if nothing select, this one is preferred, or the old one is offline
const selectNewClient =
!state.selectedAppId ||
state.userPreferredApp === payload.query.app ||
state.clients
.find((c) => c.id === state.selectedAppId)
?.connected.get() === false;
return {
...state,
selectedAppId: selectNewClient ? payload.id : state.selectedAppId,
selectedDevice: selectNewClient ? payload.device : state.selectedDevice,
clients: newClients,
uninitializedClients: state.uninitializedClients.filter((c) => {
return (
@@ -337,16 +339,31 @@ export default (state: State = INITAL_STATE, action: Actions): State => {
c.appName !== payload.query.app
);
}),
});
};
}
case 'SELECT_CLIENT': {
const {payload} = action;
return updateSelection({
const client = state.clients.find((c) => c.id === payload);
if (!client) {
return state;
}
return {
...state,
selectedApp: payload,
userPreferredApp: payload || state.userPreferredApp,
});
selectedAppId: payload,
selectedDevice: client.device,
userPreferredDevice: client.device.title,
userPreferredApp: client.query.app,
selectedPlugin:
state.selectedPlugin && client.supportsPlugin(state.selectedPlugin)
? state.selectedPlugin
: state.userPreferredPlugin &&
client.supportsPlugin(state.userPreferredPlugin)
? state.userPreferredPlugin
: null,
};
}
case 'CLIENT_REMOVED': {
@@ -355,16 +372,14 @@ export default (state: State = INITAL_STATE, action: Actions): State => {
const newClients = state.clients.filter(
(client) => client.id !== payload,
);
return updateSelection({
return {
...state,
selectedAppId:
state.selectedAppId === payload ? null : state.selectedAppId,
clients: newClients,
});
};
}
case 'PREFER_DEVICE': {
const {payload: userPreferredDevice} = action;
return {...state, userPreferredDevice};
}
case 'START_CLIENT_SETUP': {
const {payload} = action;
return {
@@ -457,23 +472,18 @@ export const setStaticView = (
};
};
export const preferDevice = (payload: string): Action => ({
type: 'PREFER_DEVICE',
payload,
});
export const selectPlugin = (payload: {
selectedPlugin: null | string;
selectedApp?: null | string;
selectedDevice?: BaseDevice | null;
deepLinkPayload: unknown;
selectedPlugin: string;
selectedAppId?: null | string;
selectedDevice?: null | BaseDevice;
deepLinkPayload?: unknown;
time?: number;
}): Action => ({
type: 'SELECT_PLUGIN',
payload: {...payload, time: payload.time ?? Date.now()},
});
export const selectClient = (clientId: string | null): Action => ({
export const selectClient = (clientId: string): Action => ({
type: 'SELECT_CLIENT',
payload: clientId,
});
@@ -532,20 +542,11 @@ export function getAvailableClients(
.sort((a, b) => (a.query.app || '').localeCompare(b.query.app));
}
function getBestAvailableClient(
device: BaseDevice | null | undefined,
export function getClientByAppName(
clients: Client[],
preferredClient: string | null,
): Client | null {
const availableClients = getAvailableClients(device, clients);
if (availableClients.length === 0) {
return null;
}
return (
getClientById(availableClients, preferredClient) ||
availableClients[0] ||
null
);
appName: string | null | undefined,
): Client | undefined {
return clients.find((client) => client.query.app === appName);
}
export function getClientById(
@@ -559,67 +560,10 @@ export function canBeDefaultDevice(device: BaseDevice) {
return !DEFAULT_DEVICE_BLACKLIST.includes(device.os);
}
/**
* This function, given the current state, tries to build to build the best
* selection possible, preselection device if there is non, plugins based on preferences, etc
* @param state
*/
function updateSelection(state: Readonly<State>): State {
if (state.staticView && state.staticView !== WelcomeScreenStaticView) {
return state;
}
const updates: Partial<State> = {
staticView: null,
};
// Find the selected device if it still exists
let device: BaseDevice | null =
state.selectedDevice && state.devices.includes(state.selectedDevice)
? state.selectedDevice
: null;
if (!device) {
device =
state.devices.find(
(device) => device.title === state.userPreferredDevice,
) ||
state.devices.find((device) => canBeDefaultDevice(device)) ||
null;
}
updates.selectedDevice = device;
if (!device) {
updates.staticView = WelcomeScreenStaticView;
}
// Select client based on device
const client = getBestAvailableClient(
device,
state.clients,
state.selectedApp || state.userPreferredApp,
);
updates.selectedApp = client ? client.id : null;
if (
// Try the preferred plugin first
state.userPreferredPlugin &&
state.userPreferredPlugin !== state.selectedPlugin
) {
updates.selectedPlugin = state.userPreferredPlugin;
} else if (
!state.selectedPlugin &&
state.enabledDevicePlugins.has(DEFAULT_PLUGIN)
) {
// currently selected plugin is not available in this state,
// fall back to the default
updates.selectedPlugin = DEFAULT_PLUGIN;
}
return {...state, ...updates};
}
export function getSelectedPluginKey(state: State): string | undefined {
return state.selectedPlugin
? getPluginKey(
state.selectedApp,
state.selectedAppId,
state.selectedDevice,
state.selectedPlugin,
)

View File

@@ -17,13 +17,13 @@ export const GLOBAL_NOTIFICATION_PLUGIN_ID = 'Flipper';
export type PluginNotification = {
notification: Notification;
pluginId: string;
client: null | string;
client: null | string; // id
};
export type PluginNotificationReference = {
notificationId: string;
pluginId: string;
client: null | string;
client: null | string; // id
};
export type State = {

View File

@@ -116,9 +116,9 @@ export class Group {
store.dispatch(
setStaticView(require('../fb-stubs/SupportRequestFormV2').default),
);
const selectedApp = store.getState().connections.selectedApp;
const selectedApp = store.getState().connections.selectedAppId;
const selectedClient = store.getState().connections.clients.find((o) => {
return o.id === store.getState().connections.selectedApp;
return o.id === store.getState().connections.selectedAppId;
});
let errorMessage: string | undefined = undefined;
if (selectedApp) {

View File

@@ -50,8 +50,13 @@ function getOsIcon(os?: DeviceOS) {
export function AppSelector() {
const dispatch = useDispatch();
const {devices, selectedDevice, clients, uninitializedClients, selectedApp} =
useStore((state) => state.connections);
const {
devices,
selectedDevice,
clients,
uninitializedClients,
selectedAppId,
} = useStore((state) => state.connections);
useValue(selectedDevice?.connected, false); // subscribe to future archived state changes
const onSelectDevice = useTrackedCallback(
@@ -59,16 +64,14 @@ export function AppSelector() {
(device: BaseDevice) => {
batch(() => {
dispatch(selectDevice(device));
dispatch(selectClient(null));
});
},
[],
);
const onSelectApp = useTrackedCallback(
'select-app',
(device: BaseDevice, client: Client) => {
(_device: BaseDevice, client: Client) => {
batch(() => {
dispatch(selectDevice(device));
dispatch(selectClient(client.id));
});
},
@@ -82,13 +85,13 @@ export function AppSelector() {
onSelectDevice,
onSelectApp,
);
const client = clients.find((client) => client.id === selectedApp);
const client = clients.find((client) => client.id === selectedAppId);
return (
<>
{entries.length ? (
<Radio.Group
value={selectedApp}
value={selectedAppId}
size="small"
style={{
display: 'flex',
@@ -97,7 +100,7 @@ export function AppSelector() {
<Dropdown
trigger={['click']}
overlay={
<Menu selectedKeys={selectedApp ? [selectedApp] : []}>
<Menu selectedKeys={selectedAppId ? [selectedAppId] : []}>
{entries}
</Menu>
}>

View File

@@ -140,9 +140,9 @@ const StyledAutoComplete = styled(AutoComplete)({
const NAVIGATION_PLUGIN_ID = 'Navigation';
function navPluginStateSelector(state: State) {
const {selectedApp, clients} = state.connections;
if (!selectedApp) return undefined;
const client = clients.find((client) => client.id === selectedApp);
const {selectedAppId, clients} = state.connections;
if (!selectedAppId) return undefined;
const client = clients.find((client) => client.id === selectedAppId);
if (!client) return undefined;
return client.sandyPluginStates.get(NAVIGATION_PLUGIN_ID)?.instanceApi as
| undefined

View File

@@ -99,26 +99,26 @@ export const PluginList = memo(function PluginList({
dispatch(
selectPlugin({
selectedPlugin: pluginId,
selectedApp: connections.selectedApp,
selectedAppId: connections.selectedAppId,
deepLinkPayload: null,
selectedDevice: activeDevice,
}),
);
},
[dispatch, activeDevice, connections.selectedApp],
[dispatch, activeDevice, connections.selectedAppId],
);
const handleMetroPluginClick = useCallback(
(pluginId) => {
dispatch(
selectPlugin({
selectedPlugin: pluginId,
selectedApp: connections.selectedApp,
selectedAppId: connections.selectedAppId,
deepLinkPayload: null,
selectedDevice: metroDevice,
}),
);
},
[dispatch, metroDevice, connections.selectedApp],
[dispatch, metroDevice, connections.selectedAppId],
);
const handleEnablePlugin = useCallback(
(id: string) => {

View File

@@ -12,7 +12,6 @@ import {
MockFlipperResult,
} from '../../../test-utils/createMockFlipperWithPlugin';
import {FlipperPlugin} from '../../../plugin';
import MetroDevice from '../../../server/devices/metro/MetroDevice';
import BaseDevice from '../../../devices/BaseDevice';
import {_SandyPluginDefinition} from 'flipper-plugin';
import {TestUtils} from 'flipper-plugin';
@@ -106,7 +105,7 @@ describe('basic getActiveDevice with metro present', () => {
selectedPlugin: 'DeviceLogs',
userPreferredDevice: 'MockAndroidDevice',
userPreferredPlugin: 'DeviceLogs',
userPreferredApp: 'TestApp#Android#MockAndroidDevice#serial',
userPreferredApp: 'TestApp',
});
expect(getActiveClient(state)).toBe(flipper.client);
});
@@ -116,19 +115,19 @@ describe('basic getActiveDevice with metro present', () => {
flipper.store.dispatch(
selectPlugin({
selectedPlugin: logsPlugin.id,
selectedApp: null,
selectedAppId: flipper.client.id,
selectedDevice: metro,
deepLinkPayload: null,
}),
);
expect(flipper.store.getState().connections).toMatchObject({
devices: [testDevice, metro],
selectedApp: null,
selectedAppId: 'TestApp#Android#MockAndroidDevice#serial',
selectedDevice: metro,
selectedPlugin: 'DeviceLogs',
userPreferredDevice: 'MockAndroidDevice', // Not metro!
userPreferredPlugin: 'DeviceLogs',
userPreferredApp: 'TestApp#Android#MockAndroidDevice#serial',
userPreferredApp: 'TestApp',
});
const state = flipper.store.getState();
// find best device is still metro

View File

@@ -310,7 +310,7 @@ export function openNotification(store: Store, noti: PluginNotificationOrig) {
store.dispatch(
selectPlugin({
selectedPlugin: noti.pluginId,
selectedApp: noti.client,
selectedAppId: client.id,
selectedDevice: client.device,
deepLinkPayload: noti.notification.action,
}),

View File

@@ -16,17 +16,14 @@ import {
import createSelector from './createSelector';
const getSelectedPluginId = (state: State) => state.connections.selectedPlugin;
const getSelectedApp = (state: State) =>
state.connections.selectedApp || state.connections.userPreferredApp;
const getSelectedAppId = (state: State) => state.connections.selectedAppId;
const getSelectedDevice = (state: State) => state.connections.selectedDevice;
const getUserPreferredDevice = (state: State) =>
state.connections.userPreferredDevice;
const getClients = (state: State) => state.connections.clients;
const getDevices = (state: State) => state.connections.devices;
const getPluginDownloads = (state: State) => state.pluginDownloads;
export const getActiveClient = createSelector(
getSelectedApp,
getSelectedAppId,
getClients,
(selectedApp, clients) => {
return clients.find((c) => c.id === selectedApp) || null;
@@ -42,11 +39,9 @@ export const getMetroDevice = createSelector(getDevices, (devices) => {
export const getActiveDevice = createSelector(
getSelectedDevice,
getUserPreferredDevice,
getDevices,
getActiveClient,
getMetroDevice,
(selectedDevice, userPreferredDevice, devices, client, metroDevice) => {
(selectedDevice, client, metroDevice) => {
// if not Metro device, use the selected device as metro device
if (selectedDevice !== metroDevice) {
return selectedDevice;
@@ -55,13 +50,6 @@ export const getActiveDevice = createSelector(
if (client) {
return client.device;
}
// if no active app, use the preferred device
if (userPreferredDevice) {
return (
devices.find((device) => device.title === userPreferredDevice) ??
selectedDevice
);
}
return selectedDevice;
},
);

View File

@@ -152,6 +152,7 @@ export default class MetroDevice extends ServerDevice {
deviceType: 'emulator',
title: 'React Native',
os: 'Metro',
icon: 'mobile',
});
if (ws) {
this.ws = ws;

View File

@@ -191,7 +191,7 @@ export async function createMockFlipperWithPlugin(
store.dispatch(
selectPlugin({
selectedPlugin: id,
selectedApp: theClient.query.app,
selectedAppId: theClient.id,
deepLinkPayload,
selectedDevice: theDevice,
}),
@@ -268,13 +268,13 @@ export async function renderMockFlipperWithPlugin(
isDevicePluginDefinition(pluginClazz)
? {
selectedPlugin: pluginClazz.id,
selectedApp: null,
selectedAppId: null,
deepLinkPayload: null,
selectedDevice: store.getState().connections.selectedDevice!,
}
: {
selectedPlugin: pluginClazz.id,
selectedApp: client.query.app,
selectedAppId: client.id,
deepLinkPayload: null,
selectedDevice: store.getState().connections.selectedDevice!,
},

View File

@@ -1137,7 +1137,7 @@ test('Sandy plugins are exported properly', async () => {
store.dispatch(
selectPlugin({
selectedPlugin: 'DeviceLogs',
selectedApp: null,
selectedAppId: null,
selectedDevice: device,
deepLinkPayload: null,
}),
@@ -1195,7 +1195,7 @@ test('Non sandy plugins are exported properly if they are still queued', async (
store.dispatch(
selectPlugin({
selectedPlugin: 'DeviceLogs',
selectedApp: null,
selectedAppId: null,
selectedDevice: device,
deepLinkPayload: null,
}),
@@ -1229,7 +1229,7 @@ test('Sandy plugins with custom export are exported properly', async () => {
store.dispatch(
selectPlugin({
selectedPlugin: 'DeviceLogs',
selectedApp: client.id,
selectedAppId: client.id,
deepLinkPayload: null,
}),
);

View File

@@ -72,7 +72,7 @@ describe('info', () => {
store.dispatch(
selectPlugin({
selectedPlugin: inspectorPlugin.id,
selectedApp: client.query.app,
selectedAppId: client.id,
selectedDevice: device,
deepLinkPayload: null,
}),

View File

@@ -77,7 +77,7 @@ function selectDeviceLogs(store: Store) {
store.dispatch(
selectPlugin({
selectedPlugin: 'DeviceLogs',
selectedApp: null,
selectedAppId: null,
deepLinkPayload: null,
selectedDevice: store.getState().connections.selectedDevice!,
}),
@@ -88,7 +88,7 @@ function selectTestPlugin(store: Store, client: Client) {
store.dispatch(
selectPlugin({
selectedPlugin: TestPlugin.id,
selectedApp: client.query.app,
selectedAppId: client.id,
deepLinkPayload: null,
selectedDevice: store.getState().connections.selectedDevice!,
}),
@@ -408,7 +408,7 @@ test('queue - messages that arrive during processing will be queued', async () =
store.dispatch(
selectPlugin({
selectedPlugin: TestPlugin.id,
selectedApp: client.id,
selectedAppId: client.id,
deepLinkPayload: null,
selectedDevice: device,
}),

View File

@@ -418,7 +418,7 @@ async function getStoreExport(
fetchMetaDataErrors: {[plugin: string]: Error} | null;
}> {
let state = store.getState();
const {clients, selectedApp, selectedDevice} = state.connections;
const {clients, selectedAppId, selectedDevice} = state.connections;
const pluginsToProcess = determinePluginsToProcess(
clients,
selectedDevice,
@@ -433,7 +433,7 @@ async function getStoreExport(
const fetchMetaDataMarker = `${EXPORT_FLIPPER_TRACE_EVENT}:fetch-meta-data`;
performance.mark(fetchMetaDataMarker);
const client = clients.find((client) => client.id === selectedApp);
const client = clients.find((client) => client.id === selectedAppId);
const pluginStates2 = pluginsToProcess
? await exportSandyPluginStates(pluginsToProcess, idler, statusUpdate)
@@ -487,9 +487,9 @@ export async function exportStore(
exportData.supportRequestDetails = {
...state.supportForm?.supportFormV2,
appName:
state.connections.selectedApp == null
state.connections.selectedAppId == null
? ''
: deconstructClientId(state.connections.selectedApp).app,
: deconstructClientId(state.connections.selectedAppId).app,
};
}

View File

@@ -41,7 +41,7 @@ export function initializeFlipperLibImplementation(
payload: {
selectedPlugin: pluginId,
selectedDevice: device as BaseDevice,
selectedApp: client ? client.id : null,
selectedAppId: client ? client.id : null,
deepLinkPayload: deeplink,
time: Date.now(),
},

View File

@@ -129,14 +129,16 @@ export function stringifyInfo(info: Info): string {
export function getSelectionInfo({
plugins: {clientPlugins, devicePlugins, loadedPlugins},
connections: {
selectedApp,
selectedAppId,
selectedPlugin,
enabledDevicePlugins,
enabledPlugins,
selectedDevice,
},
}: State): SelectionInfo {
const clientIdParts = selectedApp ? deconstructClientId(selectedApp) : null;
const clientIdParts = selectedAppId
? deconstructClientId(selectedAppId)
: null;
const loadedPlugin = selectedPlugin
? loadedPlugins.get(selectedPlugin)
: null;