From 5b76a0c717f40a32fe6e8a060e105b07e560926d Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Tue, 16 Mar 2021 14:54:53 -0700 Subject: [PATCH] Introduce subscribing to output changes Summary: This diff introduces the possibility to subscribe to the `output` set of the datasource. It emits three possible event: `reset`, `update`, `shift`. Reviewed By: jknoxville Differential Revision: D26100104 fbshipit-source-id: b5fac2289206fab9fb8a437b96ab84034a8b5832 --- .../src/state/datasource/DataSource.tsx | 158 ++++++++++++---- .../__tests__/datasource-basics.node.tsx | 169 ++++++++++++++++-- 2 files changed, 280 insertions(+), 47 deletions(-) diff --git a/desktop/flipper-plugin/src/state/datasource/DataSource.tsx b/desktop/flipper-plugin/src/state/datasource/DataSource.tsx index 69371f192..83f82e147 100644 --- a/desktop/flipper-plugin/src/state/datasource/DataSource.tsx +++ b/desktop/flipper-plugin/src/state/datasource/DataSource.tsx @@ -18,11 +18,14 @@ import { // TODO: separate views from datasource to be able to support multiple transformation simultanously // TODO: expose interface with public members only // TODO: replace forEach with faster for loops +// TODO: delete & unset operation +// TODO: support listener for input events? -type ExtractKeyType< - T extends object, - KEY extends keyof T -> = T[KEY] extends string ? string : T[KEY] extends number ? number : never; +type ExtractKeyType = T[KEY] extends string + ? string + : T[KEY] extends number + ? number + : never; type AppendEvent = { type: 'append'; @@ -47,8 +50,28 @@ type Entry = { type Primitive = number | string | boolean | null | undefined; -class DataSource< - T extends object, +type OutputChange = + | { + type: 'shift'; + index: number; + location: 'before' | 'in' | 'after'; // relative to current window + delta: number; + newCount: number; + } + | { + // an item, inside the current window, was changed + type: 'update'; + index: number; + } + | { + // something big and awesome happened. Drop earlier updates to the floor and start again + // like: clear, filter or sorting change, etc + type: 'reset'; + newCount: number; + }; + +export class DataSource< + T, KEY extends keyof T, KEY_TYPE extends string | number | never = ExtractKeyType > { @@ -67,6 +90,11 @@ class DataSource< private dataUpdateQueue: DataEvent[] = []; + private windowStart = 0; + private windowEnd = 0; + + private outputChangeListener?: (change: OutputChange) => void; + // TODO: // private viewRecords: T[] = []; // private nextViewRecords: T[] = []; // for double buffering @@ -105,6 +133,25 @@ class DataSource< this.setSortBy(undefined); } + /** + * Returns a defensive copy of the current output. + * Sort, filter, reverse and window are applied, but windowing isn't. + * Start and end behave like slice, and default to the current window + */ + public getOutput( + start = this.windowStart, + end = this.windowEnd, + ): readonly T[] { + if (this.reverse) { + return this.output + .slice(this.output.length - end, this.output.length - start) + .reverse() + .map((e) => e.value); + } else { + return this.output.slice(start, end).map((e) => e.value); + } + } + private assertKeySet() { if (!this.keyAttribute) { throw new Error( @@ -212,6 +259,17 @@ class DataSource< throw new Error('Not Implemented'); } + setWindow(start: number, end: number) { + this.windowStart = start; + this.windowEnd = end; + } + + setOutputChangeListener( + listener: typeof DataSource['prototype']['outputChangeListener'], + ) { + this.outputChangeListener = listener; + } + setSortBy(sortBy: undefined | keyof T | ((a: T) => Primitive)) { if (this.sortBy === sortBy) { return; @@ -268,6 +326,52 @@ class DataSource< this.processEvents(); } + normalizeIndex(viewIndex: number): number { + return this.reverse ? this.output.length - 1 - viewIndex : viewIndex; + } + + getItem(viewIndex: number) { + return this.output[this.normalizeIndex(viewIndex)].value; + } + + notifyItemUpdated(viewIndex: number) { + viewIndex = this.normalizeIndex(viewIndex); + if ( + !this.outputChangeListener || + viewIndex < this.windowStart || + viewIndex >= this.windowEnd + ) { + return; + } + this.outputChangeListener({ + type: 'update', + index: viewIndex, + }); + } + + notifyItemShift(viewIndex: number, delta: number) { + if (!this.outputChangeListener) { + return; + } + viewIndex = this.normalizeIndex(viewIndex); + if (this.reverse && delta < 0) { + viewIndex -= delta; // we need to correct for normalize already using the new length after applying this change + } + // TODO: for 'before' shifts, should the window be adjusted automatically? + this.outputChangeListener({ + type: 'shift', + delta, + index: viewIndex, + newCount: this.output.length, + location: + viewIndex < this.windowStart + ? 'before' + : viewIndex >= this.windowEnd + ? 'after' + : 'in', + }); + } + processEvents() { const events = this.dataUpdateQueue.splice(0); events.forEach(this.processEvent); @@ -287,7 +391,7 @@ class DataSource< // no sorting? insert at the end, or beginning entry.approxIndex = output.length; output.push(entry); - // TODO: shift window if following the end or beginning + this.notifyItemShift(entry.approxIndex, 1); } else { this.insertSorted(entry); } @@ -297,7 +401,7 @@ class DataSource< // short circuit; no view active so update straight away if (!filter && !sortBy) { output[event.index].approxIndex = event.index; - // TODO: notify updated + this.notifyItemUpdated(event.index); } else if (!event.oldVisible) { if (!entry.visible) { // Done! @@ -307,12 +411,11 @@ class DataSource< } } else { // Entry was visible previously + const existingIndex = this.getSortedIndex(entry, event.oldValue); if (!entry.visible) { // Remove from output - const existingIndex = this.getSortedIndex(entry, event.oldValue); // TODO: lift this lookup if needed for events? output.splice(existingIndex, 1); - // TODO: notify visible count reduced - // TODO: notify potential effect on window + this.notifyItemShift(existingIndex, -1); } else { // Entry was and still is visible if ( @@ -320,22 +423,15 @@ class DataSource< this.sortBy(event.oldValue) === this.sortBy(entry.value) ) { // Still at same position, so done! - // TODO: notify of update + this.notifyItemUpdated(existingIndex); } else { // item needs to be moved cause of sorting - const existingIndex = this.getSortedIndex(entry, event.oldValue); + // TODO: possible optimization: if we discover that old and new index would be the same, + // despite different sort values, we could still only emit an update output.splice(existingIndex, 1); + this.notifyItemShift(existingIndex, -1); // find new sort index - const newIndex = sortedLastIndexBy( - this.output, - entry, - this.sortHelper, - ); - entry.approxIndex = newIndex; - output.splice(newIndex, 0, entry); - // item has moved - // remove and replace - // TODO: notify entry moved (or replaced, in case newIndex === existingIndex + this.insertSorted(entry); } } } @@ -348,7 +444,7 @@ class DataSource< rebuildOutput() { const {sortBy, filter, sortHelper} = this; - // copy base array or run filter (with side effect update of visible) + // copy base array or run filter (with side effecty update of visible) // TODO: pending on the size, should we batch this in smaller steps? (and maybe merely reuse append) let output = filter ? this._records.filter((entry) => { @@ -368,7 +464,10 @@ class DataSource< entry.visible = true; }); this.output = output; - // TODO: bunch of events + this.outputChangeListener?.({ + type: 'reset', + newCount: output.length, + }); } private sortHelper = (a: Entry) => @@ -412,19 +511,18 @@ class DataSource< ); entry.approxIndex = insertionIndex; this.output.splice(insertionIndex, 0, entry); - // TODO: notify window shift if applicable - // TODO: shift window if following the end or beginning + this.notifyItemShift(insertionIndex, 1); } } -export function createDataSource( +export function createDataSource( initialSet: T[], keyAttribute: KEY, ): DataSource>; -export function createDataSource( +export function createDataSource( initialSet?: T[], ): DataSource; -export function createDataSource( +export function createDataSource( initialSet: T[] = [], keyAttribute?: KEY | undefined, ): DataSource { diff --git a/desktop/flipper-plugin/src/state/datasource/__tests__/datasource-basics.node.tsx b/desktop/flipper-plugin/src/state/datasource/__tests__/datasource-basics.node.tsx index 908fe2229..42219fc44 100644 --- a/desktop/flipper-plugin/src/state/datasource/__tests__/datasource-basics.node.tsx +++ b/desktop/flipper-plugin/src/state/datasource/__tests__/datasource-basics.node.tsx @@ -7,7 +7,7 @@ * @format */ -import {createDataSource} from '../DataSource'; +import {createDataSource, DataSource} from '../DataSource'; type Todo = { id: string; @@ -276,6 +276,9 @@ test('filter + sort + index', () => { ds.setSortBy(undefined); // key insertion order expect(unwrap(ds.output)).toEqual([newCookie, newCoffee, submitBug, a, b]); + // verify getOutput + expect(unwrap(ds.output.slice(1, 3))).toEqual([newCoffee, submitBug]); + expect(ds.getOutput(1, 3)).toEqual([newCoffee, submitBug]); }); test('filter', () => { @@ -318,34 +321,39 @@ test('filter', () => { expect(unwrap(ds.output)).toEqual([newCookie, newCoffee, submitBug, a, b]); }); -test.skip('reverse without sorting', () => { +test('reverse without sorting', () => { const ds = createDataSource([eatCookie, drinkCoffee]); - expect(unwrap(ds.output)).toEqual([eatCookie, drinkCoffee]); + ds.setWindow(0, 100); + expect(ds.getOutput()).toEqual([eatCookie, drinkCoffee]); ds.toggleReversed(); - expect(unwrap(ds.output)).toEqual([drinkCoffee, eatCookie]); + expect(ds.getOutput(1, 2)).toEqual([eatCookie]); + expect(ds.getOutput(0, 1)).toEqual([drinkCoffee]); + expect(ds.getOutput(0, 2)).toEqual([drinkCoffee, eatCookie]); + + expect(ds.getOutput()).toEqual([drinkCoffee, eatCookie]); ds.append(submitBug); expect(ds.records).toEqual([eatCookie, drinkCoffee, submitBug]); - expect(unwrap(ds.output)).toEqual([submitBug, drinkCoffee, eatCookie]); + expect(ds.getOutput()).toEqual([submitBug, drinkCoffee, eatCookie]); const x = {id: 'x', title: 'x'}; ds.update(0, x); expect(ds.records).toEqual([x, drinkCoffee, submitBug]); - expect(unwrap(ds.output)).toEqual([submitBug, drinkCoffee, x]); + expect(ds.getOutput()).toEqual([submitBug, drinkCoffee, x]); const y = {id: 'y', title: 'y'}; const z = {id: 'z', title: 'z'}; ds.update(2, z); ds.update(1, y); expect(ds.records).toEqual([x, y, z]); - expect(unwrap(ds.output)).toEqual([z, y, x]); + expect(ds.getOutput()).toEqual([z, y, x]); ds.setReversed(false); - expect(unwrap(ds.output)).toEqual([x, y, z]); + expect(ds.getOutput()).toEqual([x, y, z]); }); -test.skip('reverse with sorting', () => { +test('reverse with sorting', () => { type N = { $: string; name: string; @@ -358,22 +366,23 @@ test.skip('reverse with sorting', () => { const c = {$: 'c', name: 'c'}; const ds = createDataSource([]); + ds.setWindow(0, 100); ds.setReversed(true); ds.append(b1); ds.append(c); - expect(unwrap(ds.output)).toEqual([c, b1]); + expect(ds.getOutput()).toEqual([c, b1]); ds.setSortBy('$'); - expect(unwrap(ds.output)).toEqual([c, b1]); + expect(ds.getOutput()).toEqual([c, b1]); ds.append(b2); - expect(unwrap(ds.output)).toEqual([c, b2, b1]); + expect(ds.getOutput()).toEqual([c, b2, b1]); ds.append(a); - expect(unwrap(ds.output)).toEqual([c, b2, b1, a]); + expect(ds.getOutput()).toEqual([c, b2, b1, a]); ds.append(b3); - expect(unwrap(ds.output)).toEqual([c, b3, b2, b1, a]); + expect(ds.getOutput()).toEqual([c, b3, b2, b1, a]); // if we append a new item with existig item, it should end up in the end const b4 = { @@ -381,7 +390,7 @@ test.skip('reverse with sorting', () => { name: 'b4', }; ds.append(b4); - expect(unwrap(ds.output)).toEqual([c, b4, b3, b2, b1, a]); + expect(ds.getOutput()).toEqual([c, b4, b3, b2, b1, a]); // if we replace the middle item, it should end up in the middle const b2r = { @@ -389,7 +398,7 @@ test.skip('reverse with sorting', () => { name: 'b2replacement', }; ds.update(2, b2r); - expect(unwrap(ds.output)).toEqual([c, b4, b3, b2r, b1, a]); + expect(ds.getOutput()).toEqual([c, b4, b3, b2r, b1, a]); // if we replace something with a different sort value, it should be sorted properly, and the old should disappear const b3r = { @@ -397,7 +406,7 @@ test.skip('reverse with sorting', () => { name: 'b3replacement', }; ds.update(4, b3r); - expect(unwrap(ds.output)).toEqual([c, b4, b2r, b1, b3r, a]); + expect(ds.getOutput()).toEqual([c, b4, b2r, b1, b3r, a]); }); test('reset', () => { @@ -430,3 +439,129 @@ test('clear', () => { // resets in the same ordering as view preferences were preserved expect(unwrap(ds.output)).toEqual([drinkCoffee, submitBug]); }); + +function testEvents( + initial: T[], + op: (ds: DataSource) => void, +): any[] { + const ds = createDataSource(initial); + const events: any[] = []; + ds.setOutputChangeListener((e) => events.push(e)); + op(ds); + ds.setOutputChangeListener(undefined); + return events; +} + +test('it emits the right events - zero window', () => { + expect( + testEvents(['a', 'b'], (ds) => { + ds.append('c'); + ds.update(1, 'x'); + }), + ).toEqual([ + { + delta: 1, + index: 2, + location: 'after', + newCount: 3, + type: 'shift', + }, + ]); +}); + +test('it emits the right events - small window', () => { + expect( + testEvents(['a', 'b'], (ds) => { + ds.setWindow(0, 3); + ds.append('c'); + ds.update(1, 'x'); + }), + ).toEqual([ + {delta: 1, location: 'in', newCount: 3, type: 'shift', index: 2}, + {index: 1, type: 'update'}, + ]); +}); + +test('it emits the right events - view change', () => { + expect( + testEvents(['a', 'b'], (ds) => { + ds.setWindow(1, 2); + ds.setSortBy((x) => x); + // a, [b] + ds.update(0, 'x'); + // b, [x] + expect(ds.getItem(0)).toEqual('b'); + expect(ds.getItem(1)).toEqual('x'); + ds.append('y'); + // b, [x], y + ds.append('c'); + // b, [c], x, y + }), + ).toEqual([ + {newCount: 2, type: 'reset'}, + {index: 0, delta: -1, location: 'before', newCount: 1, type: 'shift'}, // remove a + {index: 1, delta: 1, location: 'in', newCount: 2, type: 'shift'}, // pre-insert x + {index: 2, delta: 1, location: 'after', newCount: 3, type: 'shift'}, // y happened after + {index: 1, delta: 1, location: 'in', newCount: 4, type: 'shift'}, // c becomes the 'in' new indow + ]); +}); + +test('it emits the right events - reversed view change', () => { + expect( + testEvents(['a', 'b'], (ds) => { + ds.setWindow(1, 2); + ds.setSortBy((x) => x); + ds.setReversed(true); + // b, [a] + ds.update(0, 'x'); + // x, [b] + expect(ds.getItem(0)).toEqual('x'); + expect(ds.getItem(1)).toEqual('b'); + ds.append('y'); + // y, [x], b + ds.append('c'); + // y, [x], c, b + ds.append('a'); + // y, [x], c, b, a + }), + ).toEqual([ + {newCount: 2, type: 'reset'}, + {newCount: 2, type: 'reset'}, // FIXME: ideally dedupe these, but due to scheduling will do little harm + {index: 1, delta: -1, location: 'in', newCount: 1, type: 'shift'}, // remove a + {index: 0, delta: 1, location: 'before', newCount: 2, type: 'shift'}, // pre-insert x + {index: 0, delta: 1, location: 'before', newCount: 3, type: 'shift'}, + {index: 2, delta: 1, location: 'after', newCount: 4, type: 'shift'}, + {index: 4, delta: 1, location: 'after', newCount: 5, type: 'shift'}, + ]); +}); + +test('it emits the right events - reversed view change with filter', () => { + expect( + testEvents(['a', 'b'], (ds) => { + ds.setWindow(0, 2); + ds.setSortBy((x) => x); + ds.setReversed(true); + ds.setFilter((x) => ['a', 'b'].includes(x)); + // [b, a] + ds.update(0, 'x'); + // [b, ] + expect(ds.getItem(0)).toEqual('b'); + expect(ds.output.length).toBe(1); + ds.append('y'); + // [b, ] + ds.append('c'); + // [b, ] + ds.append('a'); + // [b, a] + ds.append('a'); + // [b, a, a] // N.b. the new a is in the *middle* + }), + ).toEqual([ + {newCount: 2, type: 'reset'}, + {newCount: 2, type: 'reset'}, // FIXME: ideally dedupe these, but due to scheduling will do little harm + {newCount: 2, type: 'reset'}, // FIXME: ideally dedupe these, but due to scheduling will do little harm + {index: 1, delta: -1, location: 'in', newCount: 1, type: 'shift'}, // remove a + {index: 1, delta: 1, location: 'in', newCount: 2, type: 'shift'}, + {index: 1, delta: 1, location: 'in', newCount: 3, type: 'shift'}, + ]); +});