From 26b5cd89c23fe29ac57a78b5d459b198f778ccc4 Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Mon, 13 Jan 2020 14:12:25 +0100 Subject: [PATCH] [State Management] State containers improvements (#54436) (#54562) Some maintenance and minor fixes to state containers based on experience while working with them in #53582 Patch unit tests to use current "terminology" (e.g. "transition" vs "mutation") Fix docs where "store" was used instead of "state container" Allow to create state container without transition. Fix freeze function to deeply freeze objects. Restrict State to BaseState with extends object. in set() function, make sure the flow goes through dispatch to make sure middleware see this update Improve type inference for useTransition() Improve type inference for createStateContainer(). Other issues noticed, but didn't fix in reasonable time: Can't use addMiddleware without explicit type casting #54438 Transitions and Selectors allow any state, not bind to container's state #54439 --- .../state_containers_examples/public/todo.tsx | 17 +- package.json | 2 + renovate.json5 | 8 + src/plugins/kibana_utils/demos/demos.test.ts | 2 +- .../demos/state_containers/counter.ts | 22 ++- .../demos/state_containers/todomvc.ts | 57 +++++-- .../kibana_utils/demos/state_sync/url.ts | 4 +- .../docs/state_containers/README.md | 17 +- .../docs/state_containers/creation.md | 2 +- .../docs/state_containers/no_react.md | 6 +- .../docs/state_containers/react.md | 2 +- .../create_state_container.test.ts | 159 +++++++++--------- .../create_state_container.ts | 44 +++-- ...ate_state_container_react_helpers.test.tsx | 112 ++++++------ .../create_state_container_react_helpers.ts | 2 +- .../public/state_containers/types.ts | 32 ++-- .../public/state_sync/state_sync.test.ts | 25 +-- .../public/state_sync/state_sync.ts | 6 +- .../kibana_utils/public/state_sync/types.ts | 7 +- yarn.lock | 10 ++ 20 files changed, 305 insertions(+), 231 deletions(-) diff --git a/examples/state_containers_examples/public/todo.tsx b/examples/state_containers_examples/public/todo.tsx index 84defb4a91e3..84f64f99d017 100644 --- a/examples/state_containers_examples/public/todo.tsx +++ b/examples/state_containers_examples/public/todo.tsx @@ -41,6 +41,7 @@ import { PureTransition, syncStates, getStateFromKbnUrl, + BaseState, } from '../../../src/plugins/kibana_utils/public'; import { useUrlTracker } from '../../../src/plugins/kibana_react/public'; import { @@ -79,7 +80,7 @@ const TodoApp: React.FC = ({ filter }) => { const { setText } = GlobalStateHelpers.useTransitions(); const { text } = GlobalStateHelpers.useState(); const { edit: editTodo, delete: deleteTodo, add: addTodo } = useTransitions(); - const todos = useState(); + const todos = useState().todos; const filteredTodos = todos.filter(todo => { if (!filter) return true; if (filter === 'completed') return todo.completed; @@ -306,7 +307,7 @@ export const TodoAppPage: React.FC<{ ); }; -function withDefaultState( +function withDefaultState( stateContainer: BaseStateContainer, // eslint-disable-next-line no-shadow defaultState: State @@ -314,14 +315,10 @@ function withDefaultState( return { ...stateContainer, set: (state: State | null) => { - if (Array.isArray(defaultState)) { - stateContainer.set(state || defaultState); - } else { - stateContainer.set({ - ...defaultState, - ...state, - }); - } + stateContainer.set({ + ...defaultState, + ...state, + }); }, }; } diff --git a/package.json b/package.json index 4263b0e95035..15da8bb697f3 100644 --- a/package.json +++ b/package.json @@ -162,6 +162,7 @@ "custom-event-polyfill": "^0.3.0", "d3": "3.5.17", "d3-cloud": "1.2.5", + "deep-freeze-strict": "^1.1.1", "deepmerge": "^4.2.2", "del": "^5.1.0", "elastic-apm-node": "^3.2.0", @@ -308,6 +309,7 @@ "@types/classnames": "^2.2.9", "@types/d3": "^3.5.43", "@types/dedent": "^0.7.0", + "@types/deep-freeze-strict": "^1.1.0", "@types/delete-empty": "^2.0.0", "@types/elasticsearch": "^5.0.33", "@types/enzyme": "^3.9.0", diff --git a/renovate.json5 b/renovate.json5 index 7c2228944c46..5294b8b0e4c6 100644 --- a/renovate.json5 +++ b/renovate.json5 @@ -340,6 +340,14 @@ '@types/dedent', ], }, + { + groupSlug: 'deep-freeze-strict', + groupName: 'deep-freeze-strict related packages', + packageNames: [ + 'deep-freeze-strict', + '@types/deep-freeze-strict', + ], + }, { groupSlug: 'delete-empty', groupName: 'delete-empty related packages', diff --git a/src/plugins/kibana_utils/demos/demos.test.ts b/src/plugins/kibana_utils/demos/demos.test.ts index 5c50e152ad46..b905aeff41f1 100644 --- a/src/plugins/kibana_utils/demos/demos.test.ts +++ b/src/plugins/kibana_utils/demos/demos.test.ts @@ -38,7 +38,7 @@ describe('demos', () => { describe('state sync', () => { test('url sync demo works', async () => { expect(await urlSyncResult).toMatchInlineSnapshot( - `"http://localhost/#?_s=!((completed:!f,id:0,text:'Learning%20state%20containers'),(completed:!f,id:2,text:test))"` + `"http://localhost/#?_s=(todos:!((completed:!f,id:0,text:'Learning%20state%20containers'),(completed:!f,id:2,text:test)))"` ); }); }); diff --git a/src/plugins/kibana_utils/demos/state_containers/counter.ts b/src/plugins/kibana_utils/demos/state_containers/counter.ts index 643763cc4cee..4ddf532c1506 100644 --- a/src/plugins/kibana_utils/demos/state_containers/counter.ts +++ b/src/plugins/kibana_utils/demos/state_containers/counter.ts @@ -19,14 +19,24 @@ import { createStateContainer } from '../../public/state_containers'; -const container = createStateContainer(0, { - increment: (cnt: number) => (by: number) => cnt + by, - double: (cnt: number) => () => cnt * 2, -}); +interface State { + count: number; +} + +const container = createStateContainer( + { count: 0 }, + { + increment: (state: State) => (by: number) => ({ count: state.count + by }), + double: (state: State) => () => ({ count: state.count * 2 }), + }, + { + count: (state: State) => () => state.count, + } +); container.transitions.increment(5); container.transitions.double(); -console.log(container.get()); // eslint-disable-line +console.log(container.selectors.count()); // eslint-disable-line -export const result = container.get(); +export const result = container.selectors.count(); diff --git a/src/plugins/kibana_utils/demos/state_containers/todomvc.ts b/src/plugins/kibana_utils/demos/state_containers/todomvc.ts index 6d0c960e2a5b..e807783a56f3 100644 --- a/src/plugins/kibana_utils/demos/state_containers/todomvc.ts +++ b/src/plugins/kibana_utils/demos/state_containers/todomvc.ts @@ -25,15 +25,19 @@ export interface TodoItem { id: number; } -export type TodoState = TodoItem[]; +export interface TodoState { + todos: TodoItem[]; +} -export const defaultState: TodoState = [ - { - id: 0, - text: 'Learning state containers', - completed: false, - }, -]; +export const defaultState: TodoState = { + todos: [ + { + id: 0, + text: 'Learning state containers', + completed: false, + }, + ], +}; export interface TodoActions { add: PureTransition; @@ -44,17 +48,34 @@ export interface TodoActions { clearCompleted: PureTransition; } +export interface TodosSelectors { + todos: (state: TodoState) => () => TodoItem[]; + todo: (state: TodoState) => (id: number) => TodoItem | null; +} + export const pureTransitions: TodoActions = { - add: state => todo => [...state, todo], - edit: state => todo => state.map(item => (item.id === todo.id ? { ...item, ...todo } : item)), - delete: state => id => state.filter(item => item.id !== id), - complete: state => id => - state.map(item => (item.id === id ? { ...item, completed: true } : item)), - completeAll: state => () => state.map(item => ({ ...item, completed: true })), - clearCompleted: state => () => state.filter(({ completed }) => !completed), + add: state => todo => ({ todos: [...state.todos, todo] }), + edit: state => todo => ({ + todos: state.todos.map(item => (item.id === todo.id ? { ...item, ...todo } : item)), + }), + delete: state => id => ({ todos: state.todos.filter(item => item.id !== id) }), + complete: state => id => ({ + todos: state.todos.map(item => (item.id === id ? { ...item, completed: true } : item)), + }), + completeAll: state => () => ({ todos: state.todos.map(item => ({ ...item, completed: true })) }), + clearCompleted: state => () => ({ todos: state.todos.filter(({ completed }) => !completed) }), }; -const container = createStateContainer(defaultState, pureTransitions); +export const pureSelectors: TodosSelectors = { + todos: state => () => state.todos, + todo: state => id => state.todos.find(todo => todo.id === id) ?? null, +}; + +const container = createStateContainer( + defaultState, + pureTransitions, + pureSelectors +); container.transitions.add({ id: 1, @@ -64,6 +85,6 @@ container.transitions.add({ container.transitions.complete(0); container.transitions.complete(1); -console.log(container.get()); // eslint-disable-line +console.log(container.selectors.todos()); // eslint-disable-line -export const result = container.get(); +export const result = container.selectors.todos(); diff --git a/src/plugins/kibana_utils/demos/state_sync/url.ts b/src/plugins/kibana_utils/demos/state_sync/url.ts index 657b64f55a77..2c426cae6733 100644 --- a/src/plugins/kibana_utils/demos/state_sync/url.ts +++ b/src/plugins/kibana_utils/demos/state_sync/url.ts @@ -18,7 +18,7 @@ */ import { defaultState, pureTransitions, TodoActions, TodoState } from '../state_containers/todomvc'; -import { BaseStateContainer, createStateContainer } from '../../public/state_containers'; +import { BaseState, BaseStateContainer, createStateContainer } from '../../public/state_containers'; import { createKbnUrlStateStorage, syncState, @@ -55,7 +55,7 @@ export const result = Promise.resolve() return window.location.href; }); -function withDefaultState( +function withDefaultState( // eslint-disable-next-line no-shadow stateContainer: BaseStateContainer, // eslint-disable-next-line no-shadow diff --git a/src/plugins/kibana_utils/docs/state_containers/README.md b/src/plugins/kibana_utils/docs/state_containers/README.md index 3b7a8b8bd462..583f8f65ce6b 100644 --- a/src/plugins/kibana_utils/docs/state_containers/README.md +++ b/src/plugins/kibana_utils/docs/state_containers/README.md @@ -18,14 +18,21 @@ your services or apps. ```ts import { createStateContainer } from 'src/plugins/kibana_utils'; -const container = createStateContainer(0, { - increment: (cnt: number) => (by: number) => cnt + by, - double: (cnt: number) => () => cnt * 2, -}); +const container = createStateContainer( + { count: 0 }, + { + increment: (state: {count: number}) => (by: number) => ({ count: state.count + by }), + double: (state: {count: number}) => () => ({ count: state.count * 2 }), + }, + { + count: (state: {count: number}) => () => state.count, + } +); container.transitions.increment(5); container.transitions.double(); -console.log(container.get()); // 10 + +console.log(container.selectors.count()); // 10 ``` diff --git a/src/plugins/kibana_utils/docs/state_containers/creation.md b/src/plugins/kibana_utils/docs/state_containers/creation.md index 66d28bbd8603..f8ded75ed3f4 100644 --- a/src/plugins/kibana_utils/docs/state_containers/creation.md +++ b/src/plugins/kibana_utils/docs/state_containers/creation.md @@ -32,7 +32,7 @@ Create your a state container. ```ts import { createStateContainer } from 'src/plugins/kibana_utils'; -const container = createStateContainer(defaultState, {}); +const container = createStateContainer(defaultState); console.log(container.get()); ``` diff --git a/src/plugins/kibana_utils/docs/state_containers/no_react.md b/src/plugins/kibana_utils/docs/state_containers/no_react.md index 7a15483d83b4..a72995f4f1ea 100644 --- a/src/plugins/kibana_utils/docs/state_containers/no_react.md +++ b/src/plugins/kibana_utils/docs/state_containers/no_react.md @@ -1,13 +1,13 @@ # Consuming state in non-React setting -To read the current `state` of the store use `.get()` method. +To read the current `state` of the store use `.get()` method or `getState()` alias method. ```ts -store.get(); +stateContainer.get(); ``` To listen for latest state changes use `.state$` observable. ```ts -store.state$.subscribe(state => { ... }); +stateContainer.state$.subscribe(state => { ... }); ``` diff --git a/src/plugins/kibana_utils/docs/state_containers/react.md b/src/plugins/kibana_utils/docs/state_containers/react.md index 363fd9253d44..1bab1af1d5f6 100644 --- a/src/plugins/kibana_utils/docs/state_containers/react.md +++ b/src/plugins/kibana_utils/docs/state_containers/react.md @@ -9,7 +9,7 @@ ```ts import { createStateContainer, createStateContainerReactHelpers } from 'src/plugins/kibana_utils'; -const container = createStateContainer({}, {}); +const container = createStateContainer({}); export const { Provider, Consumer, diff --git a/src/plugins/kibana_utils/public/state_containers/create_state_container.test.ts b/src/plugins/kibana_utils/public/state_containers/create_state_container.test.ts index 95f4c35f2ce0..d4877acaa5ca 100644 --- a/src/plugins/kibana_utils/public/state_containers/create_state_container.test.ts +++ b/src/plugins/kibana_utils/public/state_containers/create_state_container.test.ts @@ -19,18 +19,9 @@ import { createStateContainer } from './create_state_container'; -const create = (state: S, transitions: T = {} as T) => { - const pureTransitions = { - set: () => (newState: S) => newState, - ...transitions, - }; - const store = createStateContainer(state, pureTransitions); - return { store, mutators: store.transitions }; -}; - -test('can create store', () => { - const { store } = create({}); - expect(store).toMatchObject({ +test('can create state container', () => { + const stateContainer = createStateContainer({}); + expect(stateContainer).toMatchObject({ getState: expect.any(Function), state$: expect.any(Object), transitions: expect.any(Object), @@ -45,9 +36,9 @@ test('can set default state', () => { const defaultState = { foo: 'bar', }; - const { store } = create(defaultState); - expect(store.get()).toEqual(defaultState); - expect(store.getState()).toEqual(defaultState); + const stateContainer = createStateContainer(defaultState); + expect(stateContainer.get()).toEqual(defaultState); + expect(stateContainer.getState()).toEqual(defaultState); }); test('can set state', () => { @@ -57,12 +48,12 @@ test('can set state', () => { const newState = { foo: 'baz', }; - const { store, mutators } = create(defaultState); + const stateContainer = createStateContainer(defaultState); - mutators.set(newState); + stateContainer.set(newState); - expect(store.get()).toEqual(newState); - expect(store.getState()).toEqual(newState); + expect(stateContainer.get()).toEqual(newState); + expect(stateContainer.getState()).toEqual(newState); }); test('does not shallow merge states', () => { @@ -72,22 +63,22 @@ test('does not shallow merge states', () => { const newState = { foo2: 'baz', }; - const { store, mutators } = create(defaultState); + const stateContainer = createStateContainer(defaultState); - mutators.set(newState as any); + stateContainer.set(newState as any); - expect(store.get()).toEqual(newState); - expect(store.getState()).toEqual(newState); + expect(stateContainer.get()).toEqual(newState); + expect(stateContainer.getState()).toEqual(newState); }); test('can subscribe and unsubscribe to state changes', () => { - const { store, mutators } = create({}); + const stateContainer = createStateContainer({}); const spy = jest.fn(); - const subscription = store.state$.subscribe(spy); - mutators.set({ a: 1 }); - mutators.set({ a: 2 }); + const subscription = stateContainer.state$.subscribe(spy); + stateContainer.set({ a: 1 }); + stateContainer.set({ a: 2 }); subscription.unsubscribe(); - mutators.set({ a: 3 }); + stateContainer.set({ a: 3 }); expect(spy).toHaveBeenCalledTimes(2); expect(spy.mock.calls[0][0]).toEqual({ a: 1 }); @@ -95,16 +86,16 @@ test('can subscribe and unsubscribe to state changes', () => { }); test('multiple subscribers can subscribe', () => { - const { store, mutators } = create({}); + const stateContainer = createStateContainer({}); const spy1 = jest.fn(); const spy2 = jest.fn(); - const subscription1 = store.state$.subscribe(spy1); - const subscription2 = store.state$.subscribe(spy2); - mutators.set({ a: 1 }); + const subscription1 = stateContainer.state$.subscribe(spy1); + const subscription2 = stateContainer.state$.subscribe(spy2); + stateContainer.set({ a: 1 }); subscription1.unsubscribe(); - mutators.set({ a: 2 }); + stateContainer.set({ a: 2 }); subscription2.unsubscribe(); - mutators.set({ a: 3 }); + stateContainer.set({ a: 3 }); expect(spy1).toHaveBeenCalledTimes(1); expect(spy2).toHaveBeenCalledTimes(2); @@ -120,19 +111,19 @@ test('can create state container without transitions', () => { expect(stateContainer.get()).toEqual(state); }); -test('creates impure mutators from pure mutators', () => { - const { mutators } = create( +test('creates transitions', () => { + const stateContainer = createStateContainer( {}, { setFoo: () => (bar: any) => ({ foo: bar }), } ); - expect(typeof mutators.setFoo).toBe('function'); + expect(typeof stateContainer.transitions.setFoo).toBe('function'); }); -test('mutators can update state', () => { - const { store, mutators } = create( +test('transitions can update state', () => { + const stateContainer = createStateContainer( { value: 0, foo: 'bar', @@ -143,30 +134,30 @@ test('mutators can update state', () => { } ); - expect(store.get()).toEqual({ + expect(stateContainer.get()).toEqual({ value: 0, foo: 'bar', }); - mutators.add(11); - mutators.setFoo('baz'); + stateContainer.transitions.add(11); + stateContainer.transitions.setFoo('baz'); - expect(store.get()).toEqual({ + expect(stateContainer.get()).toEqual({ value: 11, foo: 'baz', }); - mutators.add(-20); - mutators.setFoo('bazooka'); + stateContainer.transitions.add(-20); + stateContainer.transitions.setFoo('bazooka'); - expect(store.get()).toEqual({ + expect(stateContainer.get()).toEqual({ value: -9, foo: 'bazooka', }); }); -test('mutators methods are not bound', () => { - const { store, mutators } = create( +test('transitions methods are not bound', () => { + const stateContainer = createStateContainer( { value: -3 }, { add: (state: { value: number }) => (increment: number) => ({ @@ -176,13 +167,13 @@ test('mutators methods are not bound', () => { } ); - expect(store.get()).toEqual({ value: -3 }); - mutators.add(4); - expect(store.get()).toEqual({ value: 1 }); + expect(stateContainer.get()).toEqual({ value: -3 }); + stateContainer.transitions.add(4); + expect(stateContainer.get()).toEqual({ value: 1 }); }); -test('created mutators are saved in store object', () => { - const { store, mutators } = create( +test('created transitions are saved in stateContainer object', () => { + const stateContainer = createStateContainer( { value: -3 }, { add: (state: { value: number }) => (increment: number) => ({ @@ -192,55 +183,57 @@ test('created mutators are saved in store object', () => { } ); - expect(typeof store.transitions.add).toBe('function'); - mutators.add(5); - expect(store.get()).toEqual({ value: 2 }); + expect(typeof stateContainer.transitions.add).toBe('function'); + stateContainer.transitions.add(5); + expect(stateContainer.get()).toEqual({ value: 2 }); }); -test('throws when state is modified inline - 1', () => { - const container = createStateContainer({ a: 'b' }, {}); +test('throws when state is modified inline', () => { + const container = createStateContainer({ a: 'b', array: [{ a: 'b' }] }); - let error: TypeError | null = null; - try { + expect(() => { (container.get().a as any) = 'c'; - } catch (err) { - error = err; - } + }).toThrowErrorMatchingInlineSnapshot( + `"Cannot assign to read only property 'a' of object '#'"` + ); - expect(error).toBeInstanceOf(TypeError); -}); - -test('throws when state is modified inline - 2', () => { - const container = createStateContainer({ a: 'b' }, {}); - - let error: TypeError | null = null; - try { + expect(() => { (container.getState().a as any) = 'c'; - } catch (err) { - error = err; - } + }).toThrowErrorMatchingInlineSnapshot( + `"Cannot assign to read only property 'a' of object '#'"` + ); - expect(error).toBeInstanceOf(TypeError); + expect(() => { + (container.getState().array as any).push('c'); + }).toThrowErrorMatchingInlineSnapshot(`"Cannot add property 1, object is not extensible"`); + + expect(() => { + (container.getState().array[0] as any).c = 'b'; + }).toThrowErrorMatchingInlineSnapshot(`"Cannot add property c, object is not extensible"`); + + expect(() => { + container.set(null as any); + expect(container.getState()).toBeNull(); + }).not.toThrow(); }); -test('throws when state is modified inline in subscription', done => { +test('throws when state is modified inline in subscription', () => { const container = createStateContainer({ a: 'b' }, { set: () => (newState: any) => newState }); container.subscribe(value => { - let error: TypeError | null = null; - try { + expect(() => { (value.a as any) = 'd'; - } catch (err) { - error = err; - } - expect(error).toBeInstanceOf(TypeError); - done(); + }).toThrowErrorMatchingInlineSnapshot( + `"Cannot assign to read only property 'a' of object '#'"` + ); }); + container.transitions.set({ a: 'c' }); }); describe('selectors', () => { test('can specify no selectors, or can skip them', () => { + createStateContainer({}); createStateContainer({}, {}); createStateContainer({}, {}, {}); }); diff --git a/src/plugins/kibana_utils/public/state_containers/create_state_container.ts b/src/plugins/kibana_utils/public/state_containers/create_state_container.ts index b949a9daed0a..d420aec30f06 100644 --- a/src/plugins/kibana_utils/public/state_containers/create_state_container.ts +++ b/src/plugins/kibana_utils/public/state_containers/create_state_container.ts @@ -20,34 +20,52 @@ import { BehaviorSubject } from 'rxjs'; import { skip } from 'rxjs/operators'; import { RecursiveReadonly } from '@kbn/utility-types'; +import deepFreeze from 'deep-freeze-strict'; import { PureTransitionsToTransitions, PureTransition, ReduxLikeStateContainer, PureSelectorsToSelectors, + BaseState, } from './types'; const $$observable = (typeof Symbol === 'function' && (Symbol as any).observable) || '@@observable'; +const $$setActionType = '@@SET'; const freeze: (value: T) => RecursiveReadonly = process.env.NODE_ENV !== 'production' ? (value: T): RecursiveReadonly => { - if (!value) return value as RecursiveReadonly; - if (value instanceof Array) return value as RecursiveReadonly; - if (typeof value === 'object') return Object.freeze({ ...value }) as RecursiveReadonly; - else return value as RecursiveReadonly; + const isFreezable = value !== null && typeof value === 'object'; + if (isFreezable) return deepFreeze(value) as RecursiveReadonly; + return value as RecursiveReadonly; } : (value: T) => value as RecursiveReadonly; -export const createStateContainer = < - State, - PureTransitions extends object = {}, - PureSelectors extends object = {} +export function createStateContainer( + defaultState: State +): ReduxLikeStateContainer; +export function createStateContainer( + defaultState: State, + pureTransitions: PureTransitions +): ReduxLikeStateContainer; +export function createStateContainer< + State extends BaseState, + PureTransitions extends object, + PureSelectors extends object +>( + defaultState: State, + pureTransitions: PureTransitions, + pureSelectors: PureSelectors +): ReduxLikeStateContainer; +export function createStateContainer< + State extends BaseState, + PureTransitions extends object, + PureSelectors extends object >( defaultState: State, pureTransitions: PureTransitions = {} as PureTransitions, pureSelectors: PureSelectors = {} as PureSelectors -): ReduxLikeStateContainer => { +): ReduxLikeStateContainer { const data$ = new BehaviorSubject>(freeze(defaultState)); const state$ = data$.pipe(skip(1)); const get = () => data$.getValue(); @@ -56,9 +74,13 @@ export const createStateContainer = < state$, getState: () => data$.getValue(), set: (state: State) => { - data$.next(freeze(state)); + container.dispatch({ type: $$setActionType, args: [state] }); }, reducer: (state, action) => { + if (action.type === $$setActionType) { + return freeze(action.args[0] as State); + } + const pureTransition = (pureTransitions as Record>)[ action.type ]; @@ -86,4 +108,4 @@ export const createStateContainer = < [$$observable]: state$, }; return container; -}; +} diff --git a/src/plugins/kibana_utils/public/state_containers/create_state_container_react_helpers.test.tsx b/src/plugins/kibana_utils/public/state_containers/create_state_container_react_helpers.test.tsx index c1a35441b637..0f25f65c30ad 100644 --- a/src/plugins/kibana_utils/public/state_containers/create_state_container_react_helpers.test.tsx +++ b/src/plugins/kibana_utils/public/state_containers/create_state_container_react_helpers.test.tsx @@ -23,15 +23,6 @@ import { act, Simulate } from 'react-dom/test-utils'; import { createStateContainer } from './create_state_container'; import { createStateContainerReactHelpers } from './create_state_container_react_helpers'; -const create = (state: S, transitions: T = {} as T) => { - const pureTransitions = { - set: () => (newState: S) => newState, - ...transitions, - }; - const store = createStateContainer(state, pureTransitions); - return { store, mutators: store.transitions }; -}; - let container: HTMLDivElement | null; beforeEach(() => { @@ -56,12 +47,12 @@ test('can create React context', () => { }); test(' passes state to ', () => { - const { store } = create({ hello: 'world' }); - const { Provider, Consumer } = createStateContainerReactHelpers(); + const stateContainer = createStateContainer({ hello: 'world' }); + const { Provider, Consumer } = createStateContainerReactHelpers(); ReactDOM.render( - - {(s: typeof store) => s.get().hello} + + {(s: typeof stateContainer) => s.get().hello} , container ); @@ -79,8 +70,8 @@ interface Props1 { } test(' passes state to connect()()', () => { - const { store } = create({ hello: 'Bob' }); - const { Provider, connect } = createStateContainerReactHelpers(); + const stateContainer = createStateContainer({ hello: 'Bob' }); + const { Provider, connect } = createStateContainerReactHelpers(); const Demo: React.FC = ({ message, stop }) => ( <> @@ -92,7 +83,7 @@ test(' passes state to connect()()', () => { const DemoConnected = connect(mergeProps)(Demo); ReactDOM.render( - + , container @@ -101,14 +92,14 @@ test(' passes state to connect()()', () => { expect(container!.innerHTML).toBe('Bob?'); }); -test('context receives Redux store', () => { - const { store } = create({ foo: 'bar' }); - const { Provider, context } = createStateContainerReactHelpers(); +test('context receives stateContainer', () => { + const stateContainer = createStateContainer({ foo: 'bar' }); + const { Provider, context } = createStateContainerReactHelpers(); ReactDOM.render( /* eslint-disable no-shadow */ - - {store => store.get().foo} + + {stateContainer => stateContainer.get().foo} , /* eslint-enable no-shadow */ container @@ -117,21 +108,21 @@ test('context receives Redux store', () => { expect(container!.innerHTML).toBe('bar'); }); -xtest('can use multiple stores in one React app', () => {}); +test.todo('can use multiple stores in one React app'); describe('hooks', () => { describe('useStore', () => { - test('can select store using useStore hook', () => { - const { store } = create({ foo: 'bar' }); - const { Provider, useContainer } = createStateContainerReactHelpers(); + test('can select store using useContainer hook', () => { + const stateContainer = createStateContainer({ foo: 'bar' }); + const { Provider, useContainer } = createStateContainerReactHelpers(); const Demo: React.FC<{}> = () => { // eslint-disable-next-line no-shadow - const store = useContainer(); - return <>{store.get().foo}; + const stateContainer = useContainer(); + return <>{stateContainer.get().foo}; }; ReactDOM.render( - + , container @@ -143,15 +134,15 @@ describe('hooks', () => { describe('useState', () => { test('can select state using useState hook', () => { - const { store } = create({ foo: 'qux' }); - const { Provider, useState } = createStateContainerReactHelpers(); + const stateContainer = createStateContainer({ foo: 'qux' }); + const { Provider, useState } = createStateContainerReactHelpers(); const Demo: React.FC<{}> = () => { const { foo } = useState(); return <>{foo}; }; ReactDOM.render( - + , container @@ -161,23 +152,20 @@ describe('hooks', () => { }); test('re-renders when state changes', () => { - const { - store, - mutators: { setFoo }, - } = create( + const stateContainer = createStateContainer( { foo: 'bar' }, { setFoo: (state: { foo: string }) => (foo: string) => ({ ...state, foo }), } ); - const { Provider, useState } = createStateContainerReactHelpers(); + const { Provider, useState } = createStateContainerReactHelpers(); const Demo: React.FC<{}> = () => { const { foo } = useState(); return <>{foo}; }; ReactDOM.render( - + , container @@ -185,7 +173,7 @@ describe('hooks', () => { expect(container!.innerHTML).toBe('bar'); act(() => { - setFoo('baz'); + stateContainer.transitions.setFoo('baz'); }); expect(container!.innerHTML).toBe('baz'); }); @@ -193,7 +181,7 @@ describe('hooks', () => { describe('useTransitions', () => { test('useTransitions hook returns mutations that can update state', () => { - const { store } = create( + const stateContainer = createStateContainer( { cnt: 0, }, @@ -206,7 +194,7 @@ describe('hooks', () => { ); const { Provider, useState, useTransitions } = createStateContainerReactHelpers< - typeof store + typeof stateContainer >(); const Demo: React.FC<{}> = () => { const { cnt } = useState(); @@ -220,7 +208,7 @@ describe('hooks', () => { }; ReactDOM.render( - + , container @@ -240,7 +228,7 @@ describe('hooks', () => { describe('useSelector', () => { test('can select deeply nested value', () => { - const { store } = create({ + const stateContainer = createStateContainer({ foo: { bar: { baz: 'qux', @@ -248,14 +236,14 @@ describe('hooks', () => { }, }); const selector = (state: { foo: { bar: { baz: string } } }) => state.foo.bar.baz; - const { Provider, useSelector } = createStateContainerReactHelpers(); + const { Provider, useSelector } = createStateContainerReactHelpers(); const Demo: React.FC<{}> = () => { const value = useSelector(selector); return <>{value}; }; ReactDOM.render( - + , container @@ -265,7 +253,7 @@ describe('hooks', () => { }); test('re-renders when state changes', () => { - const { store, mutators } = create({ + const stateContainer = createStateContainer({ foo: { bar: { baz: 'qux', @@ -280,7 +268,7 @@ describe('hooks', () => { }; ReactDOM.render( - + , container @@ -288,7 +276,7 @@ describe('hooks', () => { expect(container!.innerHTML).toBe('qux'); act(() => { - mutators.set({ + stateContainer.set({ foo: { bar: { baz: 'quux', @@ -300,9 +288,9 @@ describe('hooks', () => { }); test("re-renders only when selector's result changes", async () => { - const { store, mutators } = create({ a: 'b', foo: 'bar' }); + const stateContainer = createStateContainer({ a: 'b', foo: 'bar' }); const selector = (state: { foo: string }) => state.foo; - const { Provider, useSelector } = createStateContainerReactHelpers(); + const { Provider, useSelector } = createStateContainerReactHelpers(); let cnt = 0; const Demo: React.FC<{}> = () => { @@ -311,7 +299,7 @@ describe('hooks', () => { return <>{value}; }; ReactDOM.render( - + , container @@ -321,14 +309,14 @@ describe('hooks', () => { expect(cnt).toBe(1); act(() => { - mutators.set({ a: 'c', foo: 'bar' }); + stateContainer.set({ a: 'c', foo: 'bar' }); }); await new Promise(r => setTimeout(r, 1)); expect(cnt).toBe(1); act(() => { - mutators.set({ a: 'd', foo: 'bar 2' }); + stateContainer.set({ a: 'd', foo: 'bar 2' }); }); await new Promise(r => setTimeout(r, 1)); @@ -336,9 +324,9 @@ describe('hooks', () => { }); test('does not re-render on same shape object', async () => { - const { store, mutators } = create({ foo: { bar: 'baz' } }); + const stateContainer = createStateContainer({ foo: { bar: 'baz' } }); const selector = (state: { foo: any }) => state.foo; - const { Provider, useSelector } = createStateContainerReactHelpers(); + const { Provider, useSelector } = createStateContainerReactHelpers(); let cnt = 0; const Demo: React.FC<{}> = () => { @@ -347,7 +335,7 @@ describe('hooks', () => { return <>{JSON.stringify(value)}; }; ReactDOM.render( - + , container @@ -357,14 +345,14 @@ describe('hooks', () => { expect(cnt).toBe(1); act(() => { - mutators.set({ foo: { bar: 'baz' } }); + stateContainer.set({ foo: { bar: 'baz' } }); }); await new Promise(r => setTimeout(r, 1)); expect(cnt).toBe(1); act(() => { - mutators.set({ foo: { bar: 'qux' } }); + stateContainer.set({ foo: { bar: 'qux' } }); }); await new Promise(r => setTimeout(r, 1)); @@ -372,7 +360,7 @@ describe('hooks', () => { }); test('can set custom comparator function to prevent re-renders on deep equality', async () => { - const { store, mutators } = create( + const stateContainer = createStateContainer( { foo: { bar: 'baz' } }, { set: () => (newState: { foo: { bar: string } }) => newState, @@ -380,7 +368,7 @@ describe('hooks', () => { ); const selector = (state: { foo: any }) => state.foo; const comparator = (prev: any, curr: any) => JSON.stringify(prev) === JSON.stringify(curr); - const { Provider, useSelector } = createStateContainerReactHelpers(); + const { Provider, useSelector } = createStateContainerReactHelpers(); let cnt = 0; const Demo: React.FC<{}> = () => { @@ -389,7 +377,7 @@ describe('hooks', () => { return <>{JSON.stringify(value)}; }; ReactDOM.render( - + , container @@ -399,13 +387,13 @@ describe('hooks', () => { expect(cnt).toBe(1); act(() => { - mutators.set({ foo: { bar: 'baz' } }); + stateContainer.set({ foo: { bar: 'baz' } }); }); await new Promise(r => setTimeout(r, 1)); expect(cnt).toBe(1); }); - xtest('unsubscribes when React un-mounts', () => {}); + test.todo('unsubscribes when React un-mounts'); }); }); diff --git a/src/plugins/kibana_utils/public/state_containers/create_state_container_react_helpers.ts b/src/plugins/kibana_utils/public/state_containers/create_state_container_react_helpers.ts index 45b34b13251f..36903f2d7c90 100644 --- a/src/plugins/kibana_utils/public/state_containers/create_state_container_react_helpers.ts +++ b/src/plugins/kibana_utils/public/state_containers/create_state_container_react_helpers.ts @@ -35,7 +35,7 @@ export const createStateContainerReactHelpers = useContainer().transitions; + const useTransitions: () => Container['transitions'] = () => useContainer().transitions; const useSelector = ( selector: (state: UnboxState) => Result, diff --git a/src/plugins/kibana_utils/public/state_containers/types.ts b/src/plugins/kibana_utils/public/state_containers/types.ts index e120f60e72b8..5f27a3d2c1dc 100644 --- a/src/plugins/kibana_utils/public/state_containers/types.ts +++ b/src/plugins/kibana_utils/public/state_containers/types.ts @@ -20,12 +20,13 @@ import { Observable } from 'rxjs'; import { Ensure, RecursiveReadonly } from '@kbn/utility-types'; +export type BaseState = object; export interface TransitionDescription { type: Type; args: Args; } -export type Transition = (...args: Args) => State; -export type PureTransition = ( +export type Transition = (...args: Args) => State; +export type PureTransition = ( state: RecursiveReadonly ) => Transition; export type EnsurePureTransition = Ensure>; @@ -34,15 +35,15 @@ export type PureTransitionsToTransitions = { [K in keyof T]: PureTransitionToTransition>; }; -export interface BaseStateContainer { +export interface BaseStateContainer { get: () => RecursiveReadonly; set: (state: State) => void; state$: Observable>; } export interface StateContainer< - State, - PureTransitions extends object = {}, + State extends BaseState, + PureTransitions extends object, PureSelectors extends object = {} > extends BaseStateContainer { transitions: Readonly>; @@ -50,7 +51,7 @@ export interface StateContainer< } export interface ReduxLikeStateContainer< - State, + State extends BaseState, PureTransitions extends object = {}, PureSelectors extends object = {} > extends StateContainer { @@ -63,14 +64,16 @@ export interface ReduxLikeStateContainer< } export type Dispatch = (action: T) => void; - -export type Middleware = ( +export type Middleware = ( store: Pick, 'getState' | 'dispatch'> ) => ( next: (action: TransitionDescription) => TransitionDescription | any ) => Dispatch; -export type Reducer = (state: State, action: TransitionDescription) => State; +export type Reducer = ( + state: State, + action: TransitionDescription +) => State; export type UnboxState< Container extends StateContainer @@ -80,7 +83,7 @@ export type UnboxTransitions< > = Container extends StateContainer ? T : never; export type Selector = (...args: Args) => Result; -export type PureSelector = ( +export type PureSelector = ( state: State ) => Selector; export type EnsurePureSelector = Ensure>; @@ -93,7 +96,12 @@ export type PureSelectorsToSelectors = { export type Comparator = (previous: Result, current: Result) => boolean; -export type MapStateToProps = (state: State) => StateProps; -export type Connect = ( +export type MapStateToProps = ( + state: State +) => StateProps; +export type Connect = < + Props extends object, + StatePropKeys extends keyof Props +>( mapStateToProp: MapStateToProps> ) => (component: React.ComponentType) => React.FC>; diff --git a/src/plugins/kibana_utils/public/state_sync/state_sync.test.ts b/src/plugins/kibana_utils/public/state_sync/state_sync.test.ts index cc513bc674d0..08ad1551420d 100644 --- a/src/plugins/kibana_utils/public/state_sync/state_sync.test.ts +++ b/src/plugins/kibana_utils/public/state_sync/state_sync.test.ts @@ -17,7 +17,7 @@ * under the License. */ -import { BaseStateContainer, createStateContainer } from '../state_containers'; +import { BaseState, BaseStateContainer, createStateContainer } from '../state_containers'; import { defaultState, pureTransitions, @@ -89,7 +89,7 @@ describe('state_sync', () => { // initial sync of storage to state is not happening expect(container.getState()).toEqual(defaultState); - const storageState2 = [{ id: 1, text: 'todo', completed: true }]; + const storageState2 = { todos: [{ id: 1, text: 'todo', completed: true }] }; (testStateStorage.get as jest.Mock).mockImplementation(() => storageState2); storageChange$.next(storageState2); @@ -124,7 +124,7 @@ describe('state_sync', () => { start(); const originalState = container.getState(); - const storageState = [...originalState]; + const storageState = { ...originalState }; (testStateStorage.get as jest.Mock).mockImplementation(() => storageState); storageChange$.next(storageState); @@ -134,7 +134,7 @@ describe('state_sync', () => { }); it('storage change to null should notify state', () => { - container.set([{ completed: false, id: 1, text: 'changed' }]); + container.set({ todos: [{ completed: false, id: 1, text: 'changed' }] }); const { stop, start } = syncStates([ { stateContainer: withDefaultState(container, defaultState), @@ -189,8 +189,8 @@ describe('state_sync', () => { ]); start(); - const newStateFromUrl = [{ completed: false, id: 1, text: 'changed' }]; - history.replace('/#?_s=!((completed:!f,id:1,text:changed))'); + const newStateFromUrl = { todos: [{ completed: false, id: 1, text: 'changed' }] }; + history.replace('/#?_s=(todos:!((completed:!f,id:1,text:changed)))'); expect(container.getState()).toEqual(newStateFromUrl); expect(JSON.parse(sessionStorage.getItem(key)!)).toEqual(newStateFromUrl); @@ -220,7 +220,7 @@ describe('state_sync', () => { expect(history.length).toBe(startHistoryLength + 1); expect(getCurrentUrl()).toMatchInlineSnapshot( - `"/#?_s=!((completed:!t,id:0,text:'Learning%20state%20containers'),(completed:!t,id:2,text:'2'),(completed:!t,id:3,text:'3'))"` + `"/#?_s=(todos:!((completed:!t,id:0,text:'Learning%20state%20containers'),(completed:!t,id:2,text:'2'),(completed:!t,id:3,text:'3')))"` ); stop(); @@ -248,14 +248,14 @@ describe('state_sync', () => { expect(history.length).toBe(startHistoryLength + 1); expect(getCurrentUrl()).toMatchInlineSnapshot( - `"/#?_s=!((completed:!t,id:0,text:'Learning%20state%20containers'),(completed:!t,id:2,text:'2'),(completed:!t,id:3,text:'3'))"` + `"/#?_s=(todos:!((completed:!t,id:0,text:'Learning%20state%20containers'),(completed:!t,id:2,text:'2'),(completed:!t,id:3,text:'3')))"` ); await tick(); expect(history.length).toBe(startHistoryLength + 1); expect(getCurrentUrl()).toMatchInlineSnapshot( - `"/#?_s=!((completed:!t,id:0,text:'Learning%20state%20containers'),(completed:!t,id:2,text:'2'),(completed:!t,id:3,text:'3'))"` + `"/#?_s=(todos:!((completed:!t,id:0,text:'Learning%20state%20containers'),(completed:!t,id:2,text:'2'),(completed:!t,id:3,text:'3')))"` ); stop(); @@ -294,7 +294,7 @@ describe('state_sync', () => { }); }); -function withDefaultState( +function withDefaultState( stateContainer: BaseStateContainer, // eslint-disable-next-line no-shadow defaultState: State @@ -302,7 +302,10 @@ function withDefaultState( return { ...stateContainer, set: (state: State | null) => { - stateContainer.set(state || defaultState); + stateContainer.set({ + ...defaultState, + ...state, + }); }, }; } diff --git a/src/plugins/kibana_utils/public/state_sync/state_sync.ts b/src/plugins/kibana_utils/public/state_sync/state_sync.ts index f0ef1423dec7..9c1116e5da53 100644 --- a/src/plugins/kibana_utils/public/state_sync/state_sync.ts +++ b/src/plugins/kibana_utils/public/state_sync/state_sync.ts @@ -23,6 +23,7 @@ import defaultComparator from 'fast-deep-equal'; import { IStateSyncConfig } from './types'; import { IStateStorage } from './state_sync_state_storage'; import { distinctUntilChangedWithInitialValue } from '../../common'; +import { BaseState } from '../state_containers'; /** * Utility for syncing application state wrapped in state container @@ -86,7 +87,10 @@ export interface ISyncStateRef({ +export function syncState< + State extends BaseState, + StateStorage extends IStateStorage = IStateStorage +>({ storageKey, stateStorage, stateContainer, diff --git a/src/plugins/kibana_utils/public/state_sync/types.ts b/src/plugins/kibana_utils/public/state_sync/types.ts index 0f7395ad0f0e..3009c1d161a5 100644 --- a/src/plugins/kibana_utils/public/state_sync/types.ts +++ b/src/plugins/kibana_utils/public/state_sync/types.ts @@ -17,10 +17,11 @@ * under the License. */ -import { BaseStateContainer } from '../state_containers/types'; +import { BaseState, BaseStateContainer } from '../state_containers/types'; import { IStateStorage } from './state_sync_state_storage'; -export interface INullableBaseStateContainer extends BaseStateContainer { +export interface INullableBaseStateContainer + extends BaseStateContainer { // State container for stateSync() have to accept "null" // for example, set() implementation could handle null and fallback to some default state // this is required to handle edge case, when state in storage becomes empty and syncing is in progress. @@ -29,7 +30,7 @@ export interface INullableBaseStateContainer extends BaseStateContainer { /** diff --git a/yarn.lock b/yarn.lock index e1c00c4072a6..1b7e2a1cac78 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3929,6 +3929,11 @@ resolved "https://registry.yarnpkg.com/@types/dedent/-/dedent-0.7.0.tgz#155f339ca404e6dd90b9ce46a3f78fd69ca9b050" integrity sha512-EGlKlgMhnLt/cM4DbUSafFdrkeJoC9Mvnj0PUCU7tFmTjMjNRT957kXCx0wYm3JuEq4o4ZsS5vG+NlkM2DMd2A== +"@types/deep-freeze-strict@^1.1.0": + version "1.1.0" + resolved "https://registry.yarnpkg.com/@types/deep-freeze-strict/-/deep-freeze-strict-1.1.0.tgz#447a6a2576191344aa42310131dd3df5c41492c4" + integrity sha1-RHpqJXYZE0SqQjEBMd099cQUksQ= + "@types/delete-empty@^2.0.0": version "2.0.0" resolved "https://registry.yarnpkg.com/@types/delete-empty/-/delete-empty-2.0.0.tgz#1647ae9e68f708a6ba778531af667ec55bc61964" @@ -10523,6 +10528,11 @@ deep-extend@^0.6.0: resolved "https://registry.yarnpkg.com/deep-extend/-/deep-extend-0.6.0.tgz#c4fa7c95404a17a9c3e8ca7e1537312b736330ac" integrity sha512-LOHxIOaPYdHlJRtCQfDIVZtfw/ufM8+rVj649RIHzcm/vGwQRXFt6OPqIFWsm2XEMrNIEtWR64sY1LEKD2vAOA== +deep-freeze-strict@^1.1.1: + version "1.1.1" + resolved "https://registry.yarnpkg.com/deep-freeze-strict/-/deep-freeze-strict-1.1.1.tgz#77d0583ca24a69be4bbd9ac2fae415d55523e5b0" + integrity sha1-d9BYPKJKab5LvZrC+uQV1VUj5bA= + deep-is@^0.1.3, deep-is@~0.1.3: version "0.1.3" resolved "https://registry.yarnpkg.com/deep-is/-/deep-is-0.1.3.tgz#b369d6fb5dbc13eecf524f91b070feedc357cf34"