From d16c6061c1f50fe282faa48da0a09dda27f8a784 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Wed, 1 Jul 2020 08:58:40 -0700 Subject: [PATCH] Introduce `createState` which can be used in components to propagate updates Summary: Introduced a minimal state abstraction so that state can be maintained in the plugin instance, but subscribe to by the UI. At a later point we could pick an off the shelve solution like Recoil or MobX, or introduce cursors to read something deep etc etc, but for now that doesn't seem to be needed at all, and I think this will be pretty comprehensible for plugin authors (see also the 25th diff in this stack). The api ``` createState(initialValue): Atom Atom { get() // returns current value set(newValue) // sets a new value update(draft => { }) // updates a complex value using Immer behind the scenes } // hook, subscribes to the updates of the Atom and always returns the current value useValue(atom) ``` Reviewed By: nikoant Differential Revision: D22306673 fbshipit-source-id: c49f5af85ae9929187e4d8a051311a07c1b88eb5 --- desktop/flipper-plugin/package.json | 3 +- .../src/__tests__/TestPlugin.tsx | 12 ++- .../src/__tests__/test-utils.node.tsx | 26 ++++++- desktop/flipper-plugin/src/index.ts | 1 + desktop/flipper-plugin/src/state/atom.tsx | 75 +++++++++++++++++++ .../src/test-utils/test-utils.tsx | 5 +- desktop/yarn.lock | 5 ++ 7 files changed, 117 insertions(+), 10 deletions(-) create mode 100644 desktop/flipper-plugin/src/state/atom.tsx diff --git a/desktop/flipper-plugin/package.json b/desktop/flipper-plugin/package.json index 2037956b2..bc2bb11a4 100644 --- a/desktop/flipper-plugin/package.json +++ b/desktop/flipper-plugin/package.json @@ -9,7 +9,8 @@ "license": "MIT", "bugs": "https://github.com/facebook/flipper/issues", "dependencies": { - "@testing-library/react": "^10.4.3" + "@testing-library/react": "^10.4.3", + "immer": "^7.0.5" }, "devDependencies": { "@types/jest": "^26.0.3", diff --git a/desktop/flipper-plugin/src/__tests__/TestPlugin.tsx b/desktop/flipper-plugin/src/__tests__/TestPlugin.tsx index 66c533742..53cd5d7bf 100644 --- a/desktop/flipper-plugin/src/__tests__/TestPlugin.tsx +++ b/desktop/flipper-plugin/src/__tests__/TestPlugin.tsx @@ -10,6 +10,7 @@ import * as React from 'react'; import {FlipperClient} from '../plugin/Plugin'; import {usePlugin} from '../plugin/PluginContext'; +import {createState, useValue} from '../state/atom'; type Events = { inc: { @@ -25,9 +26,9 @@ export function plugin(client: FlipperClient) { const connectStub = jest.fn(); const disconnectStub = jest.fn(); const destroyStub = jest.fn(); - const state = { + const state = createState({ count: 0, - }; + }); // TODO: add tests for sending and receiving data T68683442 // including typescript assertions @@ -36,7 +37,9 @@ export function plugin(client: FlipperClient) { client.onDisconnect(disconnectStub); client.onDestroy(destroyStub); client.onMessage('inc', ({delta}) => { - state.count += delta; + state.update((draft) => { + draft.count += delta; + }); }); function _unused_JustTypeChecks() { @@ -69,10 +72,11 @@ export function plugin(client: FlipperClient) { export function Component() { const api = usePlugin(plugin); + const count = useValue(api.state).count; // @ts-expect-error api.bla; // TODO N.b.: state updates won't be visible - return

Hi from test plugin {api.state.count}

; + return

Hi from test plugin {count}

; } diff --git a/desktop/flipper-plugin/src/__tests__/test-utils.node.tsx b/desktop/flipper-plugin/src/__tests__/test-utils.node.tsx index 911909da5..b7d62ac46 100644 --- a/desktop/flipper-plugin/src/__tests__/test-utils.node.tsx +++ b/desktop/flipper-plugin/src/__tests__/test-utils.node.tsx @@ -47,7 +47,7 @@ test('it can start a plugin and lifecycle events', () => { }); test('it can render a plugin', () => { - const {renderer} = TestUtils.renderPlugin(testPlugin); + const {renderer, sendEvent, instance} = TestUtils.renderPlugin(testPlugin); expect(renderer.baseElement).toMatchInlineSnapshot(` @@ -59,7 +59,25 @@ test('it can render a plugin', () => { `); - // TODO: test sending updates T68683442 + + sendEvent('inc', {delta: 3}); + + expect(renderer.baseElement).toMatchInlineSnapshot(` + +
+

+ Hi from test plugin + 3 +

+
+ + `); + + // @ts-ignore + expect(instance.state.listeners.length).toBe(1); + renderer.unmount(); + // @ts-ignore + expect(instance.state.listeners.length).toBe(0); }); test('a plugin can send messages', async () => { @@ -97,8 +115,8 @@ test('a plugin cannot send messages after being disconnected', async () => { test('a plugin can receive messages', async () => { const {instance, sendEvent} = TestUtils.startPlugin(testPlugin); - expect(instance.state.count).toBe(0); + expect(instance.state.get().count).toBe(0); sendEvent('inc', {delta: 2}); - expect(instance.state.count).toBe(2); + expect(instance.state.get().count).toBe(2); }); diff --git a/desktop/flipper-plugin/src/index.ts b/desktop/flipper-plugin/src/index.ts index 4470f96b1..a92139618 100644 --- a/desktop/flipper-plugin/src/index.ts +++ b/desktop/flipper-plugin/src/index.ts @@ -13,6 +13,7 @@ export {SandyPluginInstance, FlipperClient} from './plugin/Plugin'; export {SandyPluginDefinition} from './plugin/SandyPluginDefinition'; export {SandyPluginRenderer} from './plugin/PluginRenderer'; export {SandyPluginContext, usePlugin} from './plugin/PluginContext'; +export {createState as createValue, useValue, Atom} from './state/atom'; // It's not ideal that this exists in flipper-plugin sources directly, // but is the least pain for plugin authors. diff --git a/desktop/flipper-plugin/src/state/atom.tsx b/desktop/flipper-plugin/src/state/atom.tsx new file mode 100644 index 000000000..15a64b09a --- /dev/null +++ b/desktop/flipper-plugin/src/state/atom.tsx @@ -0,0 +1,75 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + */ + +import {produce} from 'immer'; +import {useState, useEffect} from 'react'; + +export type Atom = { + get(): T; + set(newValue: T): void; + update(recipe: (draft: T) => void): void; +}; + +class AtomValue implements Atom { + value: T; + listeners: ((value: T) => void)[] = []; + + constructor(initialValue: T) { + this.value = initialValue; + } + + get() { + return this.value; + } + + set(nextValue: T) { + if (nextValue !== this.value) { + this.value = nextValue; + this.notifyChanged(); + } + } + + update(recipe: (draft: T) => void) { + this.set(produce(this.value, recipe)); + } + + notifyChanged() { + // TODO: add scheduling + this.listeners.slice().forEach((l) => l(this.value)); + } + + subscribe(listener: (value: T) => void) { + this.listeners.push(listener); + } + + unsubscribe(listener: (value: T) => void) { + const idx = this.listeners.indexOf(listener); + if (idx !== -1) { + this.listeners.splice(idx, 1); + } + } +} + +export function createState(initialValue: T): Atom { + return new AtomValue(initialValue); +} + +export function useValue(atom: Atom): T { + const [localValue, setLocalValue] = useState(atom.get()); + useEffect(() => { + // atom might have changed between mounting and effect setup + // in that case, this will cause a re-render, otherwise not + setLocalValue(atom.get()); + (atom as AtomValue).subscribe(setLocalValue); + return () => { + (atom as AtomValue).unsubscribe(setLocalValue); + }; + }, [atom]); + return localValue; +} diff --git a/desktop/flipper-plugin/src/test-utils/test-utils.tsx b/desktop/flipper-plugin/src/test-utils/test-utils.tsx index 9620fb3a9..e1e55dd70 100644 --- a/desktop/flipper-plugin/src/test-utils/test-utils.tsx +++ b/desktop/flipper-plugin/src/test-utils/test-utils.tsx @@ -25,6 +25,7 @@ import { FlipperPluginModule, } from '../plugin/SandyPluginDefinition'; import {SandyPluginRenderer} from '../plugin/PluginRenderer'; +import {act} from '@testing-library/react'; type Renderer = RenderResult; @@ -148,7 +149,9 @@ export function startPlugin>( ]); }, sendEvents: (messages) => { - pluginInstance.receiveMessages(messages as any); + act(() => { + pluginInstance.receiveMessages(messages as any); + }); }, }; // @ts-ignore diff --git a/desktop/yarn.lock b/desktop/yarn.lock index a77fb0385..d73d5644a 100644 --- a/desktop/yarn.lock +++ b/desktop/yarn.lock @@ -6414,6 +6414,11 @@ immer@^6.0.0: resolved "https://registry.yarnpkg.com/immer/-/immer-6.0.3.tgz#94d5051cd724668160a900d66d85ec02816f29bd" integrity sha512-12VvNrfSrXZdm/BJgi/KDW2soq5freVSf3I1+4CLunUM8mAGx2/0Njy0xBVzi5zewQZiwM7z1/1T+8VaI7NkmQ== +immer@^7.0.5: + version "7.0.5" + resolved "https://registry.yarnpkg.com/immer/-/immer-7.0.5.tgz#8af347db5b60b40af8ae7baf1784ea4d35b5208e" + integrity sha512-TtRAKZyuqld2eYjvWgXISLJ0ZlOl1OOTzRmrmiY8SlB0dnAhZ1OiykIDL5KDFNaPHDXiLfGQFNJGtet8z8AEmg== + immutable@^4.0.0-rc.12: version "4.0.0-rc.12" resolved "https://registry.yarnpkg.com/immutable/-/immutable-4.0.0-rc.12.tgz#ca59a7e4c19ae8d9bf74a97bdf0f6e2f2a5d0217"