From c654d9d27d5f23a983e3c4764c8e41fbe5a45ef6 Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Mon, 13 Jan 2020 13:57:26 +0300 Subject: [PATCH] [State Management] State containers improvements (#54436) 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 | 157 +++++++++--------- .../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, 304 insertions(+), 230 deletions(-) diff --git a/examples/state_containers_examples/public/todo.tsx b/examples/state_containers_examples/public/todo.tsx index 84defb4a91e3f16..84f64f99d017982 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 0ed74dd65d1ab9e..6b9640d214a5efe 100644 --- a/package.json +++ b/package.json @@ -166,6 +166,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", @@ -314,6 +315,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 560403046b0a511..7f67fae89411048 100644 --- a/renovate.json5 +++ b/renovate.json5 @@ -210,6 +210,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 5c50e152ad46ca2..b905aeff41f1ff7 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 643763cc4cee932..4ddf532c1506dbd 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 6d0c960e2a5b26a..e807783a56f3196 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) }), +}; + +export const pureSelectors: TodosSelectors = { + todos: state => () => state.todos, + todo: state => id => state.todos.find(todo => todo.id === id) ?? null, }; -const container = createStateContainer(defaultState, pureTransitions); +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 657b64f55a77661..2c426cae6733a4d 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 3b7a8b8bd4621da..583f8f65ce6b693 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 66d28bbd8603f0e..f8ded75ed3f45c9 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 7a15483d83b44fa..a72995f4f1eaed2 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 363fd9253d44fdc..1bab1af1d5f6885 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 95f4c35f2ce01b4..d4877acaa5ca002 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); -}); + expect(() => { + (container.getState().a as any) = 'c'; + }).toThrowErrorMatchingInlineSnapshot( + `"Cannot assign to read only property 'a' of object '#'"` + ); -test('throws when state is modified inline - 2', () => { - const container = createStateContainer({ a: 'b' }, {}); + expect(() => { + (container.getState().array as any).push('c'); + }).toThrowErrorMatchingInlineSnapshot(`"Cannot add property 1, object is not extensible"`); - let error: TypeError | null = null; - try { - (container.getState().a as any) = 'c'; - } catch (err) { - error = err; - } + expect(() => { + (container.getState().array[0] as any).c = 'b'; + }).toThrowErrorMatchingInlineSnapshot(`"Cannot add property c, object is not extensible"`); - expect(error).toBeInstanceOf(TypeError); + 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 b949a9daed0ae63..d420aec30f068c2 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 c1a35441b637b4a..0f25f65c30ade04 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 45b34b13251f44a..36903f2d7c90f73 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 e120f60e72b8f36..5f27a3d2c1dcade 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 cc513bc674d0fc0..08ad1551420d2ed 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 f0ef1423dec71be..9c1116e5da5318b 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 0f7395ad0f0e56c..3009c1d161a5325 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 3dd7dbe37b2e94c..ff098b7b9c891fd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3439,6 +3439,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" @@ -10034,6 +10039,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"