From bc647972e1c20691cae719c10a63ba15a9276047 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Mon, 7 Jun 2021 08:08:53 -0700 Subject: [PATCH] Type improvements Summary: some type simplifications, that makes it easier to reuse data sources and helps type inference Reviewed By: passy Differential Revision: D28413380 fbshipit-source-id: 261a8b981bf18a00edc3075926bd668322e1c37d --- .../src/data-source/DataSource.tsx | 68 ++++++++++++------- .../data-source/DataSourceRendererStatic.tsx | 2 +- .../data-source/DataSourceRendererVirtual.tsx | 2 +- .../__tests__/datasource-basics.node.tsx | 7 +- .../__tests__/datasource-perf.node.tsx | 5 +- .../flipper-plugin/src/data-source/index.tsx | 2 +- .../src/data-source/package.json | 2 +- .../src/state/createDataSource.tsx | 39 ++++------- desktop/flipper-plugin/src/ui/DataList.tsx | 6 +- .../src/ui/data-table/DataTable.tsx | 15 ++-- .../src/ui/data-table/DataTableManager.tsx | 6 +- desktop/plugins/public/network/types.tsx | 2 +- 12 files changed, 80 insertions(+), 76 deletions(-) diff --git a/desktop/flipper-plugin/src/data-source/DataSource.tsx b/desktop/flipper-plugin/src/data-source/DataSource.tsx index 53cc21765..182a86a0a 100644 --- a/desktop/flipper-plugin/src/data-source/DataSource.tsx +++ b/desktop/flipper-plugin/src/data-source/DataSource.tsx @@ -20,12 +20,6 @@ const defaultLimit = 100 * 1000; // rather than search and remove the affected individual items const shiftRebuildTreshold = 0.05; -export type ExtractKeyType = T[KEY] extends string - ? string - : T[KEY] extends number - ? number - : never; - type AppendEvent = { type: 'append'; entry: Entry; @@ -83,19 +77,45 @@ type OutputChange = newCount: number; }; -export class DataSource< - T = any, - KEY extends keyof T = any, - KEY_TYPE extends string | number | never = ExtractKeyType, -> { +export type DataSourceOptions = { + /** + * If a key is set, the given field of the records is assumed to be unique, + * and it's value can be used to perform lookups and upserts. + */ + key?: K; + /** + * The maximum amount of records that this DataSource will store. + * If the limit is exceeded, the oldest records will automatically be dropped to make place for the new ones + */ + limit?: number; +}; + +export function createDataSource( + initialSet: readonly T[], + options?: DataSourceOptions, +): DataSource; +export function createDataSource(initialSet?: readonly T[]): DataSource; +export function createDataSource( + initialSet: readonly T[] = [], + options?: DataSourceOptions, +): DataSource { + const ds = new DataSource(options?.key); + if (options?.limit !== undefined) { + ds.limit = options.limit; + } + initialSet.forEach((value) => ds.append(value)); + return ds as any; +} + +export class DataSource { private nextId = 0; private _records: Entry[] = []; - private _recordsById: Map = new Map(); + private _recordsById: Map = new Map(); /** * @readonly */ - public keyAttribute: undefined | keyof T; - private idToIndex: Map = new Map(); + public keyAttribute: keyof T | undefined; + private idToIndex: Map = new Map(); // if we shift the window, we increase shiftOffset to correct idToIndex results, rather than remapping all values private shiftOffset = 0; @@ -113,7 +133,7 @@ export class DataSource< */ public readonly view: DataSourceView; - constructor(keyAttribute: KEY | undefined) { + constructor(keyAttribute: keyof T | undefined) { this.keyAttribute = keyAttribute; this.view = new DataSourceView(this); } @@ -134,22 +154,22 @@ export class DataSource< return unwrap(this._records[index]); } - public has(key: KEY_TYPE) { + public has(key: string) { this.assertKeySet(); return this._recordsById.has(key); } - public getById(key: KEY_TYPE) { + public getById(key: string): T | undefined { this.assertKeySet(); return this._recordsById.get(key); } - public keys(): IterableIterator { + public keys(): IterableIterator { this.assertKeySet(); return this._recordsById.keys(); } - public entries(): IterableIterator<[KEY_TYPE, T]> { + public entries(): IterableIterator<[string, T]> { this.assertKeySet(); return this._recordsById.entries(); } @@ -178,7 +198,7 @@ export class DataSource< * Returns the index of a specific key in the *records* set. * Returns -1 if the record wansn't found */ - public getIndexOfKey(key: KEY_TYPE): number { + public getIndexOfKey(key: string): number { this.assertKeySet(); const stored = this.idToIndex.get(key); return stored === undefined ? -1 : stored + this.shiftOffset; @@ -302,7 +322,7 @@ export class DataSource< * * Warning: this operation can be O(n) if a key is set */ - public deleteByKey(keyValue: KEY_TYPE): boolean { + public deleteByKey(keyValue: string): boolean { this.assertKeySet(); const index = this.getIndexOfKey(keyValue); if (index === -1) { @@ -382,7 +402,7 @@ export class DataSource< } } - private getKey(value: T): KEY_TYPE; + private getKey(value: T): string; private getKey(value: any): any { this.assertKeySet(); const key = value[this.keyAttribute!]; @@ -392,7 +412,7 @@ export class DataSource< throw new Error(`Invalid key value: '${key}'`); } - private storeIndexOfKey(key: KEY_TYPE, index: number) { + private storeIndexOfKey(key: string, index: number) { // de-normalize the index, so that on later look ups its corrected again this.idToIndex.set(key, index - this.shiftOffset); } @@ -448,7 +468,7 @@ class DataSourceView { */ private _output: Entry[] = []; - constructor(datasource: DataSource) { + constructor(datasource: DataSource) { this.datasource = datasource; } diff --git a/desktop/flipper-plugin/src/data-source/DataSourceRendererStatic.tsx b/desktop/flipper-plugin/src/data-source/DataSourceRendererStatic.tsx index a96f9e4a7..25359feb9 100644 --- a/desktop/flipper-plugin/src/data-source/DataSourceRendererStatic.tsx +++ b/desktop/flipper-plugin/src/data-source/DataSourceRendererStatic.tsx @@ -16,7 +16,7 @@ type DataSourceProps = { /** * The data source to render */ - dataSource: DataSource; + dataSource: DataSource; /** * additional context that will be passed verbatim to the itemRenderer, so that it can be easily memoized */ diff --git a/desktop/flipper-plugin/src/data-source/DataSourceRendererVirtual.tsx b/desktop/flipper-plugin/src/data-source/DataSourceRendererVirtual.tsx index c83072c92..50707166f 100644 --- a/desktop/flipper-plugin/src/data-source/DataSourceRendererVirtual.tsx +++ b/desktop/flipper-plugin/src/data-source/DataSourceRendererVirtual.tsx @@ -39,7 +39,7 @@ type DataSourceProps = { /** * The data source to render */ - dataSource: DataSource; + dataSource: DataSource; /** * Automatically scroll if the user is near the end? */ diff --git a/desktop/flipper-plugin/src/data-source/__tests__/datasource-basics.node.tsx b/desktop/flipper-plugin/src/data-source/__tests__/datasource-basics.node.tsx index 6ac652cd1..7f6e4f9fb 100644 --- a/desktop/flipper-plugin/src/data-source/__tests__/datasource-basics.node.tsx +++ b/desktop/flipper-plugin/src/data-source/__tests__/datasource-basics.node.tsx @@ -7,10 +7,7 @@ * @format */ -// ok for now, should be factored if this becomes a stand-alone lib -// eslint-disable-next-line -import {createDataSource} from '../../state/createDataSource'; -import {DataSource} from '../DataSource'; +import {DataSource, createDataSource} from '../DataSource'; type Todo = { id: string; @@ -487,7 +484,7 @@ test('clear', () => { function testEvents( initial: T[], - op: (ds: DataSource) => void, + op: (ds: DataSource) => void, key?: keyof T, ): any[] { const ds = createDataSource(initial, {key}); diff --git a/desktop/flipper-plugin/src/data-source/__tests__/datasource-perf.node.tsx b/desktop/flipper-plugin/src/data-source/__tests__/datasource-perf.node.tsx index 1667f818f..474d13042 100644 --- a/desktop/flipper-plugin/src/data-source/__tests__/datasource-perf.node.tsx +++ b/desktop/flipper-plugin/src/data-source/__tests__/datasource-perf.node.tsx @@ -7,10 +7,7 @@ * @format */ -// ok for now, should be factored if this becomes a stand-alone lib -// eslint-disable-next-line -import {createDataSource} from '../../state/createDataSource'; -import {DataSource} from '../DataSource'; +import {DataSource, createDataSource} from '../DataSource'; type Todo = { id: string; diff --git a/desktop/flipper-plugin/src/data-source/index.tsx b/desktop/flipper-plugin/src/data-source/index.tsx index d9b0a757d..83c38e45e 100644 --- a/desktop/flipper-plugin/src/data-source/index.tsx +++ b/desktop/flipper-plugin/src/data-source/index.tsx @@ -7,7 +7,7 @@ * @format */ -export {DataSource, ExtractKeyType} from './DataSource'; +export {DataSource, createDataSource, DataSourceOptions} from './DataSource'; export { DataSourceRendererVirtual, DataSourceVirtualizer, diff --git a/desktop/flipper-plugin/src/data-source/package.json b/desktop/flipper-plugin/src/data-source/package.json index 3a3022cca..aee1fe6ef 100644 --- a/desktop/flipper-plugin/src/data-source/package.json +++ b/desktop/flipper-plugin/src/data-source/package.json @@ -1,6 +1,6 @@ { "name": "flipper-data-source", - "version": "0.0.1", + "version": "0.0.2", "description": "Library to power streamig data visualisations", "repository": "https://github.com/facebook/flipper", "homepage": "https://github.com/facebook/flipper/blob/master/desktop/flipper-plugin/src/data-source/README.md", diff --git a/desktop/flipper-plugin/src/state/createDataSource.tsx b/desktop/flipper-plugin/src/state/createDataSource.tsx index 2366b525c..a820c6c7a 100644 --- a/desktop/flipper-plugin/src/state/createDataSource.tsx +++ b/desktop/flipper-plugin/src/state/createDataSource.tsx @@ -7,20 +7,17 @@ * @format */ -import {DataSource, ExtractKeyType} from '../data-source/index'; +import { + DataSource, + createDataSource as baseCreateDataSource, + DataSourceOptions as BaseDataSourceOptions, +} from '../data-source/index'; import {registerStorageAtom} from '../plugin/PluginBase'; -type CreateDataSourceOptions = { - /** - * If a key is set, the given field of the records is assumed to be unique, - * and it's value can be used to perform lookups and upserts. - */ - key?: K; - /** - * The maximum amount of records that this DataSource will store. - * If the limit is exceeded, the oldest records will automatically be dropped to make place for the new ones - */ - limit?: number; +type CreateDataSourceOptions = BaseDataSourceOptions< + T, + K +> & { /** * Should this state persist when exporting a plugin? * If set, the dataSource will be saved / loaded under the key provided @@ -29,21 +26,15 @@ type CreateDataSourceOptions = { }; export function createDataSource( - initialSet: T[], + initialSet: readonly T[], options: CreateDataSourceOptions, -): DataSource>; -export function createDataSource( - initialSet?: T[], -): DataSource; +): DataSource; +export function createDataSource(initialSet?: readonly T[]): DataSource; export function createDataSource( - initialSet: T[] = [], + initialSet: readonly T[] = [], options?: CreateDataSourceOptions, -): DataSource { - const ds = new DataSource(options?.key); - if (options?.limit !== undefined) { - ds.limit = options.limit; - } +): DataSource { + const ds = baseCreateDataSource(initialSet, options); registerStorageAtom(options?.persist, ds); - initialSet.forEach((value) => ds.append(value)); return ds; } diff --git a/desktop/flipper-plugin/src/ui/DataList.tsx b/desktop/flipper-plugin/src/ui/DataList.tsx index 49e40fdf9..8db683eba 100644 --- a/desktop/flipper-plugin/src/ui/DataList.tsx +++ b/desktop/flipper-plugin/src/ui/DataList.tsx @@ -30,6 +30,7 @@ import styled from '@emotion/styled'; import {DataTableManager} from './data-table/DataTableManager'; import {Atom, createState} from '../state/atom'; import {useAssertStableRef} from '../utils/useAssertStableRef'; +import {DataSource} from '../data-source'; const {Text} = Typography; @@ -62,7 +63,7 @@ interface DataListBaseProps { /** * Items to display. Per item at least a title and unique id should be provided */ - items: readonly Item[]; + items: DataSource | readonly Item[]; /** * Custom render function. By default the component will render the `title` in bold and description (if any) below it */ @@ -152,7 +153,8 @@ export const DataList: React.FC> = function DataList< {...tableProps} tableManagerRef={tableManagerRef} - records={items} + records={Array.isArray(items) ? items : undefined} + dataSource={Array.isArray(items) ? undefined : (items as any)} recordsKey="id" columns={dataListColumns} onSelect={handleSelect} diff --git a/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx b/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx index 82197cb75..168c3a8a9 100644 --- a/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx +++ b/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx @@ -51,6 +51,7 @@ import {Formatter} from '../DataFormatter'; import {usePluginInstance} from '../../plugin/PluginContext'; import {debounce} from 'lodash'; import {useInUnitTest} from '../../utils/useInUnitTest'; +import {createDataSource} from 'flipper-plugin/src/state/createDataSource'; interface DataTableBaseProps { columns: DataTableColumn[]; @@ -66,9 +67,7 @@ interface DataTableBaseProps { tableManagerRef?: RefObject | undefined>; // Actually we want a MutableRefObject, but that is not what React.createRef() returns, and we don't want to put the burden on the plugin dev to cast it... onCopyRows?(records: T[]): string; onContextMenu?: (selection: undefined | T) => React.ReactElement; - onRenderEmpty?: - | null - | ((dataSource?: DataSource) => React.ReactElement); + onRenderEmpty?: null | ((dataSource?: DataSource) => React.ReactElement); } export type ItemRenderer = ( @@ -79,7 +78,7 @@ export type ItemRenderer = ( type DataTableInput = | { - dataSource: DataSource; + dataSource: DataSource; records?: undefined; recordsKey?: undefined; } @@ -525,11 +524,9 @@ function normalizeDataSourceInput(props: DataTableInput): DataSource { return props.dataSource; } if (props.records) { - const [dataSource] = useState(() => { - const ds = new DataSource(props.recordsKey); - syncRecordsToDataSource(ds, props.records); - return ds; - }); + const [dataSource] = useState(() => + createDataSource(props.records, {key: props.recordsKey}), + ); useEffect(() => { syncRecordsToDataSource(dataSource, props.records); }, [dataSource, props.records]); diff --git a/desktop/flipper-plugin/src/ui/data-table/DataTableManager.tsx b/desktop/flipper-plugin/src/ui/data-table/DataTableManager.tsx index 938d48f54..8db6be00c 100644 --- a/desktop/flipper-plugin/src/ui/data-table/DataTableManager.tsx +++ b/desktop/flipper-plugin/src/ui/data-table/DataTableManager.tsx @@ -64,7 +64,7 @@ type DataManagerActions = | Action< 'selectItemById', { - id: string | number; + id: string; addToSelection?: boolean; } > @@ -213,7 +213,7 @@ export const dataTableManagerReducer = produce< } case 'setColumnFilterFromSelection': { const items = getSelectedItems( - config.dataSource as DataSource, + config.dataSource as DataSource, draft.selection, ); items.forEach((item, index) => { @@ -258,7 +258,7 @@ export type DataTableManager = { end: number, allowUnselect?: boolean, ): void; - selectItemById(id: string | number, addToSelection?: boolean): void; + selectItemById(id: string, addToSelection?: boolean): void; clearSelection(): void; getSelectedItem(): T | undefined; getSelectedItems(): readonly T[]; diff --git a/desktop/plugins/public/network/types.tsx b/desktop/plugins/public/network/types.tsx index 251e763c7..b24684f24 100644 --- a/desktop/plugins/public/network/types.tsx +++ b/desktop/plugins/public/network/types.tsx @@ -33,7 +33,7 @@ export interface Request { insights?: Insights; } -export type Requests = DataSource; +export type Requests = DataSource; export type SerializedRequest = Omit< Request,