From 685cc09b3b090a6a616b7967b2548bde536b487f Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Thu, 20 Aug 2020 13:31:17 -0700 Subject: [PATCH] DeviceLogs plugin to Sandy Summary: Converted the DeviceLogs plugin to sandy. Kept logic and UI the same (so same batching, localstorage mechanisms etc). But used sandy api's for log subscribing, state, and separating the logical part of the component from the UI. Note that some mechanisms work slightly different, like deeplinking and scrollToBottom handling, to reflect the fact that plugins are now long lived Reviewed By: jknoxville Differential Revision: D22845466 fbshipit-source-id: 7c98b2ddd9121dc730768ee1bece7e71bb5bec16 --- desktop/app/src/PluginContainer.tsx | 11 +- desktop/app/src/devices/BaseDevice.tsx | 1 + desktop/app/src/dispatcher/notifications.tsx | 8 +- desktop/app/src/reducers/connections.tsx | 38 +- .../src/ui/components/table/ManagedTable.tsx | 1 + .../src/plugin/DevicePlugin.tsx | 2 + desktop/flipper-plugin/src/state/atom.tsx | 6 +- desktop/plugins/logs/index.tsx | 456 +++++++++--------- desktop/plugins/logs/package.json | 3 + 9 files changed, 267 insertions(+), 259 deletions(-) diff --git a/desktop/app/src/PluginContainer.tsx b/desktop/app/src/PluginContainer.tsx index d075288c4..a4de12660 100644 --- a/desktop/app/src/PluginContainer.tsx +++ b/desktop/app/src/PluginContainer.tsx @@ -343,12 +343,11 @@ class PluginContainer extends PureComponent { let pluginElement: null | React.ReactElement; if (isSandyPlugin(activePlugin)) { // Make sure we throw away the container for different pluginKey! - pluginElement = ( - - ); + const instance = target.sandyPluginStates.get(activePlugin.id); + if (!instance) { + return null; + } + pluginElement = ; } else { const props: PluginProps & { key: string; diff --git a/desktop/app/src/devices/BaseDevice.tsx b/desktop/app/src/devices/BaseDevice.tsx index 73e4703c1..418996a9c 100644 --- a/desktop/app/src/devices/BaseDevice.tsx +++ b/desktop/app/src/devices/BaseDevice.tsx @@ -120,6 +120,7 @@ export default class BaseDevice { this._notifyLogListeners(entry); } + // TODO: remove getLogs T70688226 getLogs(startDate: Date | null = null) { return startDate != null ? this.logEntries.filter((log) => { diff --git a/desktop/app/src/dispatcher/notifications.tsx b/desktop/app/src/dispatcher/notifications.tsx index 4a55a485a..2fcc8485a 100644 --- a/desktop/app/src/dispatcher/notifications.tsx +++ b/desktop/app/src/dispatcher/notifications.tsx @@ -12,7 +12,7 @@ import {Logger} from '../fb-interfaces/Logger'; import {PluginNotification} from '../reducers/notifications'; import {PluginDefinition, isSandyPlugin} from '../plugin'; import isHeadless from '../utils/isHeadless'; -import {setStaticView, setDeeplinkPayload} from '../reducers/connections'; +import {setStaticView} from '../reducers/connections'; import {ipcRenderer, IpcRendererEvent} from 'electron'; import { setActiveNotifications, @@ -48,9 +48,11 @@ export default (store: Store, logger: Logger) => { ) => { if (eventName === 'click' || (eventName === 'action' && arg === 0)) { store.dispatch( - setDeeplinkPayload(pluginNotification.notification.action ?? null), + setStaticView( + NotificationScreen, + pluginNotification.notification.action ?? null, + ), ); - store.dispatch(setStaticView(NotificationScreen)); } else if (eventName === 'action') { if (arg === 1 && pluginNotification.notification.category) { // Hide similar (category) diff --git a/desktop/app/src/reducers/connections.tsx b/desktop/app/src/reducers/connections.tsx index 91b110b3c..0526ec8fb 100644 --- a/desktop/app/src/reducers/connections.tsx +++ b/desktop/app/src/reducers/connections.tsx @@ -128,6 +128,7 @@ export type Action = | { type: 'SET_STATIC_VIEW'; payload: StaticView; + deepLinkPayload: unknown; } | { type: 'DISMISS_ERROR'; @@ -145,10 +146,6 @@ export type Action = type: 'SELECT_CLIENT'; payload: string; } - | { - type: 'SET_DEEPLINK_PAYLOAD'; - payload: null | string; - } | RegisterPluginAction; const DEFAULT_PLUGIN = 'DeviceLogs'; @@ -173,12 +170,13 @@ const INITAL_STATE: State = { export default (state: State = INITAL_STATE, action: Actions): State => { switch (action.type) { case 'SET_STATIC_VIEW': { - const {payload} = action; + const {payload, deepLinkPayload} = action; const {selectedPlugin} = state; return { ...state, staticView: payload, selectedPlugin: payload != null ? null : selectedPlugin, + deepLinkPayload: deepLinkPayload ?? null, }; } @@ -252,13 +250,17 @@ export default (state: State = INITAL_STATE, action: Actions): State => { if (typeof deepLinkPayload === 'string') { const deepLinkParams = new URLSearchParams(deepLinkPayload); const deviceParam = deepLinkParams.get('device'); - 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 (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 (!selectDevice) { @@ -388,9 +390,6 @@ export default (state: State = INITAL_STATE, action: Actions): State => { errors, }; } - case 'SET_DEEPLINK_PAYLOAD': { - return {...state, deepLinkPayload: action.payload}; - } case 'REGISTER_PLUGINS': { // plugins are registered after creating the base devices, so update them const plugins = action.payload; @@ -435,13 +434,17 @@ export const selectDevice = (payload: BaseDevice): Action => ({ payload, }); -export const setStaticView = (payload: StaticView): Action => { +export const setStaticView = ( + payload: StaticView, + deepLinkPayload?: unknown, +): Action => { if (!payload) { throw new Error('Cannot set empty static view'); } return { type: 'SET_STATIC_VIEW', payload, + deepLinkPayload, }; }; @@ -479,11 +482,6 @@ export const selectClient = (clientId: string): Action => ({ payload: clientId, }); -export const setDeeplinkPayload = (payload: string | null): Action => ({ - type: 'SET_DEEPLINK_PAYLOAD', - payload, -}); - export function getAvailableClients( device: null | undefined | BaseDevice, clients: Client[], diff --git a/desktop/app/src/ui/components/table/ManagedTable.tsx b/desktop/app/src/ui/components/table/ManagedTable.tsx index ccd245f52..92137fe83 100644 --- a/desktop/app/src/ui/components/table/ManagedTable.tsx +++ b/desktop/app/src/ui/components/table/ManagedTable.tsx @@ -276,6 +276,7 @@ export class ManagedTable extends React.Component< prevState: ManagedTableState, ) { if ( + this.props.stickyBottom !== false && this.props.rows.length !== prevProps.rows.length && this.state.shouldScrollToBottom && this.state.highlightedRows.size < 2 diff --git a/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx b/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx index 911a1ca73..603259f64 100644 --- a/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx +++ b/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx @@ -33,6 +33,7 @@ export type LogLevel = | 'fatal'; export interface Device { + readonly realDevice: any; // TODO: temporarily, clean up T70688226 readonly isArchived: boolean; readonly os: string; readonly deviceType: DeviceType; @@ -77,6 +78,7 @@ export class SandyDevicePluginInstance extends BasePluginInstance { ) { super(flipperLib, definition, initialStates); const device: Device = { + 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, diff --git a/desktop/flipper-plugin/src/state/atom.tsx b/desktop/flipper-plugin/src/state/atom.tsx index 5a9cb2ab7..c62b81809 100644 --- a/desktop/flipper-plugin/src/state/atom.tsx +++ b/desktop/flipper-plugin/src/state/atom.tsx @@ -7,14 +7,14 @@ * @format */ -import {produce} from 'immer'; +import {produce, Draft} from 'immer'; import {useState, useEffect} from 'react'; import {getCurrentPluginInstance} from '../plugin/PluginBase'; export type Atom = { get(): T; set(newValue: T): void; - update(recipe: (draft: T) => void): void; + update(recipe: (draft: Draft) => void): void; }; class AtomValue implements Atom { @@ -36,7 +36,7 @@ class AtomValue implements Atom { } } - update(recipe: (draft: T) => void) { + update(recipe: (draft: Draft) => void) { this.set(produce(this.value, recipe)); } diff --git a/desktop/plugins/logs/index.tsx b/desktop/plugins/logs/index.tsx index 591ea3f25..6b99ac81e 100644 --- a/desktop/plugins/logs/index.tsx +++ b/desktop/plugins/logs/index.tsx @@ -7,14 +7,15 @@ * @format */ +import {TableBodyRow, TableRowSortOrder} from 'flipper'; import { - TableBodyRow, - TableRowSortOrder, - Props as PluginProps, - BaseAction, + Device, + DevicePluginClient, DeviceLogEntry, - produce, -} from 'flipper'; + createState, + usePlugin, + useValue, +} from 'flipper-plugin'; import {Counter} from './LogWatcher'; import { @@ -24,17 +25,13 @@ import { ContextMenu, FlexColumn, DetailSidebar, - FlipperDevicePlugin, SearchableTable, styled, - Device, - createPaste, textContent, - KeyboardActions, MenuTemplate, } from 'flipper'; import LogWatcher from './LogWatcher'; -import React from 'react'; +import React, {useCallback, createRef, MutableRefObject} from 'react'; import {Icon, LogCount, HiddenScrollText} from './logComponents'; import {pad, getLineCount} from './logUtils'; @@ -50,16 +47,6 @@ type BaseState = { readonly entries: Entries; }; -type AdditionalState = { - readonly highlightedRows: ReadonlySet; - readonly counters: ReadonlyArray; - readonly timeDirection: 'up' | 'down'; -}; - -type State = BaseState & AdditionalState; - -type PersistedState = {}; - const COLUMN_SIZE = { type: 40, time: 120, @@ -95,7 +82,7 @@ const COLUMNS = { }, } as const; -const INITIAL_COLUMN_ORDER = [ +const COLUMN_ORDER = [ { key: 'type', visible: true, @@ -325,38 +312,81 @@ export function processEntry( }; } -export default class LogTable extends FlipperDevicePlugin< - State, - BaseAction, - PersistedState -> { - static keyboardActions: KeyboardActions = [ - 'clear', - 'goToBottom', - 'createPaste', - ]; +export function supportsDevice(device: Device) { + return ( + device.os === 'Android' || + device.os === 'Metro' || + (device.os === 'iOS' && device.deviceType !== 'physical') + ); +} - batchTimer: NodeJS.Timeout | undefined; +export function devicePlugin(client: DevicePluginClient) { + let counter = 0; + let batch: Array<{ + readonly row: TableBodyRow; + readonly entry: DeviceLogEntry; + }> = []; + let queued: boolean = false; + let batchTimer: NodeJS.Timeout | undefined; + const tableRef: MutableRefObject = createRef(); - static supportsDevice(device: Device) { - return ( - device.os === 'Android' || - device.os === 'Metro' || - (device.os === 'iOS' && device.deviceType !== 'physical') - ); - } + // TODO T70688226: this can be removed once plugin stores logs, + // rather than the device. - onKeyboardAction = (action: string) => { - if (action === 'clear') { - this.clearLogs(); - } else if (action === 'goToBottom') { - this.goToBottom(); - } else if (action === 'createPaste') { - this.createPaste(); + const initialState = addEntriesToState( + client.device.realDevice + .getLogs() + .map((log: DeviceLogEntry) => processEntry(log, '' + counter++)), + ); + + const rows = createState>(initialState.rows); + const entries = createState([]); + const highlightedRows = createState>(new Set()); + const counters = createState>(restoreSavedCounters()); + const timeDirection = createState<'up' | 'down'>('up'); + const isDeeplinked = createState(false); + + client.onDeepLink((payload: unknown) => { + if (typeof payload === 'string') { + highlightedRows.set(calculateHighlightedRows(payload, rows.get())); + isDeeplinked.set(true); } - }; + }); - restoreSavedCounters = (): Array => { + client.onDeactivate(() => { + isDeeplinked.set(false); + tableRef.current = null; + }); + + client.onDestroy(() => { + if (batchTimer) { + clearTimeout(batchTimer); + } + }); + + client.addMenuEntry( + { + action: 'clear', + handler: clearLogs, + }, + { + action: 'createPaste', + handler: createPaste, + }, + { + action: 'goToBottom', + handler: goToBottom, + }, + ); + + client.device.onLogEntry((entry: DeviceLogEntry) => { + const processedEntry = processEntry(entry, '' + counter++); + incrementCounterIfNeeded(processedEntry.entry); + scheduleEntryForBatch(processedEntry); + }); + + // TODO: make local storage abstraction T69990351 + function restoreSavedCounters(): Counter[] { const savedCounters = window.localStorage.getItem(LOG_WATCHER_LOCAL_STORAGE_KEY) || '[]'; return JSON.parse(savedCounters).map((counter: Counter) => ({ @@ -364,12 +394,12 @@ export default class LogTable extends FlipperDevicePlugin< expression: new RegExp(counter.label, 'gi'), count: 0, })); - }; + } - calculateHighlightedRows = ( + function calculateHighlightedRows( deepLinkPayload: unknown, rows: ReadonlyArray, - ): Set => { + ): Set { const highlightedRows = new Set(); if (typeof deepLinkPayload !== 'string') { return highlightedRows; @@ -398,50 +428,15 @@ export default class LogTable extends FlipperDevicePlugin< } } return highlightedRows; - }; - - tableRef: ManagedTableClass | undefined; - logListener: Symbol | undefined; - - batch: Array<{ - readonly row: TableBodyRow; - readonly entry: DeviceLogEntry; - }> = []; - queued: boolean = false; - counter: number = 0; - - constructor(props: PluginProps) { - super(props); - - const initialState = addEntriesToState( - this.device - .getLogs() - .map((log) => processEntry(log, String(this.counter++))), - this.state, - ); - this.state = { - ...initialState, - highlightedRows: this.calculateHighlightedRows( - props.deepLinkPayload, - initialState.rows, - ), - counters: this.restoreSavedCounters(), - timeDirection: 'up', - }; - - this.logListener = this.device.addLogListener((entry: DeviceLogEntry) => { - const processedEntry = processEntry(entry, String(this.counter++)); - this.incrementCounterIfNeeded(processedEntry.entry); - this.scheduleEntryForBatch(processedEntry); - }); } - incrementCounterIfNeeded = (entry: DeviceLogEntry) => { + function incrementCounterIfNeeded(entry: DeviceLogEntry) { let counterUpdated = false; - const counters = this.state.counters.map((counter) => { + const newCounters = counters.get().map((counter) => { if (entry.message.match(counter.expression)) { counterUpdated = true; if (counter.notify) { + // TODO: use new notifications system T69990351 new Notification(`${counter.label}`, { body: 'The watched log message appeared', }); @@ -455,161 +450,168 @@ export default class LogTable extends FlipperDevicePlugin< } }); if (counterUpdated) { - this.setState({counters}); - } - }; - - scheduleEntryForBatch = (item: { - row: TableBodyRow; - entry: DeviceLogEntry; - }) => { - // batch up logs to be processed every 250ms, if we have lots of log - // messages coming in, then calling an setState 200+ times is actually - // pretty expensive - this.batch.push(item); - - if (!this.queued) { - this.queued = true; - - this.batchTimer = setTimeout(() => { - const thisBatch = this.batch; - this.batch = []; - this.queued = false; - this.setState((state) => - addEntriesToState(thisBatch, state, state.timeDirection), - ); - }, 100); - } - }; - - componentWillUnmount() { - if (this.batchTimer) { - clearTimeout(this.batchTimer); - } - - if (this.logListener) { - this.device.removeLogListener(this.logListener); + counters.set(newCounters); } } - clearLogs = () => { - this.device.clearLogs().catch((e) => { + function scheduleEntryForBatch(item: { + row: TableBodyRow; + entry: DeviceLogEntry; + }) { + // batch up logs to be processed every 250ms, if we have lots of log + // messages coming in, then calling an setState 200+ times is actually + // pretty expensive + batch.push(item); + + if (!queued) { + queued = true; + + batchTimer = setTimeout(() => { + const thisBatch = batch; + batch = []; + queued = false; + const newState = addEntriesToState( + thisBatch, + { + rows: rows.get(), + entries: entries.get(), + }, + timeDirection.get(), + ); + rows.set(newState.rows); + entries.set(newState.entries); + }, 100); + } + } + + function clearLogs() { + // TODO T70688226: implement this when the store is local + client.device.realDevice.clearLogs().catch((e: any) => { console.error('Failed to clear logs: ', e); }); - this.setState({ - entries: [], - rows: [], - highlightedRows: new Set(), - counters: this.state.counters.map((counter) => ({ - ...counter, - count: 0, - })), + entries.set([]); + rows.set([]); + highlightedRows.set(new Set()); + counters.update((counters) => { + for (const counter of counters) { + counter.count = 0; + } }); - }; + } - createPaste = () => { + function createPaste() { let paste = ''; const mapFn = (row: TableBodyRow) => Object.keys(COLUMNS) .map((key) => textContent(row.columns[key].value)) .join('\t'); - if (this.state.highlightedRows.size > 0) { + if (highlightedRows.get().size > 0) { // create paste from selection - paste = this.state.rows - .filter((row) => this.state.highlightedRows.has(row.key)) + paste = rows + .get() + .filter((row) => highlightedRows.get().has(row.key)) .map(mapFn) .join('\n'); } else { // create paste with all rows - paste = this.state.rows.map(mapFn).join('\n'); + paste = rows.get().map(mapFn).join('\n'); } - createPaste(paste); - }; - - setTableRef = (ref: ManagedTableClass) => { - this.tableRef = ref; - }; - - goToBottom = () => { - if (this.tableRef != null) { - this.tableRef.scrollToBottom(); - } - }; - - onRowHighlighted = (highlightedRows: Array) => { - this.setState({ - ...this.state, - highlightedRows: new Set(highlightedRows), - }); - }; - - renderSidebar = () => { - return ( - - this.setState({counters}, () => - window.localStorage.setItem( - LOG_WATCHER_LOCAL_STORAGE_KEY, - JSON.stringify(this.state.counters), - ), - ) - } - /> - ); - }; - - static ContextMenu = styled(ContextMenu)({ - flex: 1, - }); - - buildContextMenuItems: () => MenuTemplate = () => [ - { - type: 'separator', - }, - { - label: 'Clear all', - click: this.clearLogs, - }, - ]; - - render() { - return ( - - Clear Logs} - allowRegexSearch={true} - // If the logs is opened through deeplink, then don't scroll as the row is highlighted - stickyBottom={ - !(this.props.deepLinkPayload && this.state.highlightedRows.size > 0) - } - initialSortOrder={{key: 'time', direction: 'up'}} - onSort={(order: TableRowSortOrder) => - this.setState( - produce((prevState) => { - prevState.rows.reverse(); - prevState.timeDirection = order.direction; - }), - ) - } - /> - {this.renderSidebar()} - - ); + client.createPaste(paste); } + + function goToBottom() { + tableRef.current?.scrollToBottom(); + } + + return { + rows, + highlightedRows, + counters, + isDeeplinked, + tableRef, + onRowHighlighted(selectedRows: Array) { + highlightedRows.set(new Set(selectedRows)); + }, + clearLogs, + onSort(order: TableRowSortOrder) { + rows.set(rows.get().slice().reverse()); + timeDirection.set(order.direction); + }, + updateCounters(newCounters: readonly Counter[]) { + counters.set(newCounters); + // TODO: make local storage abstraction T69989583 + window.localStorage.setItem( + LOG_WATCHER_LOCAL_STORAGE_KEY, + JSON.stringify(newCounters), + ); + }, + }; +} + +const DeviceLogsContextMenu = styled(ContextMenu)({ + flex: 1, +}); + +export function Component() { + const plugin = usePlugin(devicePlugin); + const rows = useValue(plugin.rows); + const highlightedRows = useValue(plugin.highlightedRows); + const isDeeplinked = useValue(plugin.isDeeplinked); + + const buildContextMenuItems = useCallback( + (): MenuTemplate => [ + { + type: 'separator', + }, + { + label: 'Clear all', + click: plugin.clearLogs, + }, + ], + [plugin.clearLogs], + ); + + return ( + + Clear Logs} + allowRegexSearch={true} + // If the logs is opened through deeplink, then don't scroll as the row is highlighted + stickyBottom={!(isDeeplinked && highlightedRows.size > 0)} + initialSortOrder={{key: 'time', direction: 'up'}} + onSort={plugin.onSort} + /> + + + + + ); +} + +function Sidebar() { + const plugin = usePlugin(devicePlugin); + const counters = useValue(plugin.counters); + return ( + { + plugin.updateCounters(counters); + }} + /> + ); } diff --git a/desktop/plugins/logs/package.json b/desktop/plugins/logs/package.json index e7e643842..771ca2e07 100644 --- a/desktop/plugins/logs/package.json +++ b/desktop/plugins/logs/package.json @@ -10,6 +10,9 @@ "flipper-plugin" ], "dependencies": {}, + "peerDependencies": { + "flipper-plugin": "0.51.0" + }, "title": "Logs", "icon": "arrow-right", "bugs": {