From 84c0cfee5b4bb33884a25245d89bca271374e662 Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Tue, 3 Dec 2019 15:25:14 +0100 Subject: [PATCH] POC: Batching url updates improve improve improve --- .../edit_index_pattern.html | 2 + .../edit_index_pattern/edit_index_pattern.js | 97 ++++++--- src/plugins/kibana_utils/public/store/sync.ts | 202 ++++++++---------- src/plugins/kibana_utils/public/url/index.ts | 101 ++++++--- 4 files changed, 241 insertions(+), 161 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/edit_index_pattern/edit_index_pattern.html b/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/edit_index_pattern/edit_index_pattern.html index f51913cb33650..5d0bfe6491e03 100644 --- a/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/edit_index_pattern/edit_index_pattern.html +++ b/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/edit_index_pattern/edit_index_pattern.html @@ -14,6 +14,8 @@ delete="removePattern()" > + +

diff --git a/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/edit_index_pattern/edit_index_pattern.js b/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/edit_index_pattern/edit_index_pattern.js index eddf16618c8f2..bf3fa9c1c1fb2 100644 --- a/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/edit_index_pattern/edit_index_pattern.js +++ b/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/edit_index_pattern/edit_index_pattern.js @@ -42,7 +42,6 @@ import { getEditBreadcrumbs } from '../breadcrumbs'; import { createStore, syncState, - InitialTruthSource, SyncStrategy, } from '../../../../../../../../plugins/kibana_utils/public'; const REACT_SOURCE_FILTERS_DOM_ELEMENT_ID = 'reactSourceFiltersTable'; @@ -213,34 +212,76 @@ uiModules.get('apps/management') }); handleTabChange($scope, store.get().tab); + $scope.crazyBatchUpdate = () => { + store.set({ ...store.get(), tab: 'indexedFiles' }); + store.set({ ...store.get() }); + store.set({ ...store.get(), fieldFilter: 'BATCH!' }); + }; + $scope.$$postDigest(() => { - // just an artificial example of advanced syncState util setup - // 1. different strategies are used for different slices - // 2. to/from storage mappers are used to shorten state keys - $scope.destroyStateSync = syncState([ - { - syncKey: '_a', - store, - initialTruthSource: InitialTruthSource.Storage, - syncStrategy: SyncStrategy.Url, - toStorageMapper: state => ({ t: state.tab }), - fromStorageMapper: storageState => ({ tab: storageState.t || 'indexedFields' }), - }, - { - syncKey: '_b', - store, - initialTruthSource: InitialTruthSource.Storage, - syncStrategy: config.get('state:storeInSessionStorage') ? SyncStrategy.HashedUrl : SyncStrategy.Url, - toStorageMapper: state => ({ f: state.fieldFilter, i: state.indexedFieldTypeFilter, l: state.scriptedFieldLanguageFilter }), - fromStorageMapper: storageState => ( - { - fieldFilter: storageState.f || '', - indexedFieldTypeFilter: storageState.i || '', - scriptedFieldLanguageFilter: storageState.l || '' - } - ), - }, - ]); + // 1. the simplest use case + $scope.destroyStateSync = syncState({ + syncKey: '_s', + store, + }); + + // 2. conditionally picking sync strategy + // $scope.destroyStateSync = syncState({ + // syncKey: '_s', + // store, + // syncStrategy: config.get('state:storeInSessionStorage') ? SyncStrategy.HashedUrl : SyncStrategy.Url + // }); + + // 3. implementing custom sync strategy + // const localStorageSyncStrategy = { + // toStorage: (syncKey, state) => localStorage.setItem(syncKey, JSON.stringify(state)), + // fromStorage: (syncKey) => localStorage.getItem(syncKey) ? JSON.parse(localStorage.getItem(syncKey)) : null + // }; + // $scope.destroyStateSync = syncState({ + // syncKey: '_s', + // store, + // syncStrategy: localStorageSyncStrategy + // }); + + // 4. syncing only part of state + // $scope.destroyStateSync = syncState({ + // syncKey: '_s', + // store, + // toStorageMapper: s => ({ tab: s.tab }) + // }); + + // 5. transform state before serialising + // this could be super useful for backward compatibility + // $scope.destroyStateSync = syncState({ + // syncKey: '_s', + // store, + // toStorageMapper: s => ({ t: s.tab }), + // fromStorageMapper: s => ({ tab: s.t }) + // }); + + // 6. multiple different sync configs + // $scope.destroyStateSync = syncState([ + // { + // syncKey: '_a', + // store, + // syncStrategy: SyncStrategy.Url, + // toStorageMapper: s => ({ t: s.tab }), + // fromStorageMapper: s => ({ tab: s.t }) + // }, + // { + // syncKey: '_b', + // store, + // syncStrategy: SyncStrategy.HashedUrl, + // toStorageMapper: state => ({ f: state.fieldFilter, i: state.indexedFieldTypeFilter, l: state.scriptedFieldLanguageFilter }), + // fromStorageMapper: storageState => ( + // { + // fieldFilter: storageState.f || '', + // indexedFieldTypeFilter: storageState.i || '', + // scriptedFieldLanguageFilter: storageState.l || '' + // } + // ), + // }, + // ]); }); const indexPatternListProvider = Private(IndexPatternListFactory)(); diff --git a/src/plugins/kibana_utils/public/store/sync.ts b/src/plugins/kibana_utils/public/store/sync.ts index a875966a0250d..c76937c04d8aa 100644 --- a/src/plugins/kibana_utils/public/store/sync.ts +++ b/src/plugins/kibana_utils/public/store/sync.ts @@ -19,7 +19,7 @@ import { MonoTypeOperatorFunction, Observable, Subscription } from 'rxjs'; import { distinctUntilChanged, map, share, skip, startWith } from 'rxjs/operators'; -import { createUrlControls, getStateFromUrl, setStateToUrl } from '../url'; +import { createUrlControls, getStateFromUrl, IUrlControls, setStateToUrl } from '../url'; /** * Configuration of StateSync utility @@ -33,7 +33,7 @@ export interface IStateSyncConfig< > { /** * Storage key to use for syncing, - * e.g. having syncKey '_a' will sync state to ?_a query param + * e.g. syncKey '_a' should be synced state to ?_a query param */ syncKey: string; /** @@ -54,7 +54,7 @@ export interface IStateSyncConfig< * * SyncStrategy.Url is default */ - syncStrategy?: SyncStrategy | SyncStrategyFactory; + syncStrategy?: SyncStrategy | ISyncStrategy; /** * These mappers are needed to transform application state to a different shape we want to store @@ -173,22 +173,15 @@ interface ISyncStrategy { * Take in a state object, should serialise and persist */ // TODO: replace sounds like something url specific ... - toStorage: (state: StorageState, opts: { replace: boolean }) => void; + toStorage: (syncKey: string, state: StorageState, opts: { replace: boolean }) => Promise; /** * Should retrieve state from the storage and deserialize it */ - fromStorage: () => StorageState; + fromStorage: (syncKey: string) => Promise; /** * Should notify when the storage has changed */ - storageChange$: Observable; -} - -export type SyncStrategyFactory = (syncKey: string) => ISyncStrategy; -export function isSyncStrategyFactory( - syncStrategy: SyncStrategy | SyncStrategyFactory | void -): syncStrategy is SyncStrategyFactory { - return typeof syncStrategy === 'function'; + storageChange$?: (syncKey: string) => Observable; } /** @@ -197,47 +190,58 @@ export function isSyncStrategyFactory( * Both expanded and hashed use cases */ const createUrlSyncStrategyFactory = ( - { useHash = false }: { useHash: boolean } = { useHash: false } -): SyncStrategyFactory => (syncKey: string): ISyncStrategy => { - const { update: updateUrl, listen: listenUrl } = createUrlControls(); + { useHash = false }: { useHash: boolean } = { useHash: false }, + { updateAsync: updateUrlAsync, listen: listenUrl }: IUrlControls = createUrlControls() +): ISyncStrategy => { return { - toStorage: (state: BaseState, { replace = false } = { replace: false }) => { - const newUrl = setStateToUrl(syncKey, state, { useHash }); - updateUrl(newUrl, replace); + toStorage: async ( + syncKey: string, + state: BaseState, + { replace = false } = { replace: false } + ) => { + await updateUrlAsync( + currentUrl => setStateToUrl(syncKey, state, { useHash }, currentUrl), + replace + ); }, - fromStorage: () => getStateFromUrl(syncKey), - storageChange$: new Observable(observer => { - const unlisten = listenUrl(() => { - observer.next(); - }); - - return () => { - unlisten(); - }; - }).pipe( - map(() => getStateFromUrl(syncKey)), - distinctUntilChangedWithInitialValue(getStateFromUrl(syncKey), shallowEqual), - share() - ), + fromStorage: async syncKey => getStateFromUrl(syncKey), + storageChange$: (syncKey: string) => + new Observable(observer => { + const unlisten = listenUrl(() => { + observer.next(); + }); + + return () => { + unlisten(); + }; + }).pipe( + map(() => getStateFromUrl(syncKey)), + distinctUntilChangedWithInitialValue(getStateFromUrl(syncKey), shallowEqual), + share() + ), }; }; -/** - * SyncStrategy.Url: the same as old persisting of expanded state in rison format to url - * SyncStrategy.HashedUrl: the same as old persisting of hashed state using sessionStorage for storing expanded state - * - * Possible to provide own custom SyncStrategy by implementing ISyncStrategy - * - * SyncStrategy.Url is default - */ -const Strategies: { [key in SyncStrategy]: (syncKey: string) => ISyncStrategy } = { - [SyncStrategy.Url]: createUrlSyncStrategyFactory({ useHash: false }), - [SyncStrategy.HashedUrl]: createUrlSyncStrategyFactory({ useHash: true }), - // Other SyncStrategies: LocalStorage, es, somewhere else... +export function isSyncStrategy( + syncStrategy: SyncStrategy | ISyncStrategy | void +): syncStrategy is ISyncStrategy { + return typeof syncStrategy === 'object'; +} + +// strategies provided out of the box +const createStrategies: () => { + [key in SyncStrategy]: ISyncStrategy; +} = () => { + const urlControls = createUrlControls(); + return { + [SyncStrategy.Url]: createUrlSyncStrategyFactory({ useHash: false }, urlControls), + [SyncStrategy.HashedUrl]: createUrlSyncStrategyFactory({ useHash: true }, urlControls), + // Other SyncStrategies: LocalStorage, es, somewhere else... + }; }; /** - * Utility for syncing application state wrapped in IState container shape + * Utility for syncing application state wrapped in IStore container * with some kind of storage (e.g. URL) * * Minimal usage example: @@ -331,78 +335,42 @@ export function syncState(config: IStateSyncConfig[] | IStateSyncConfig): Destro const stateSyncConfigs = Array.isArray(config) ? config : [config]; const subscriptions: Subscription[] = []; - // flags are needed to be able to skip our own state / storage updates - // e.g. when we trigger state because storage changed, - // we want to make sure we won't run into infinite cycle - let ignoreStateUpdate = false; - let ignoreStorageUpdate = false; + const syncStrategies = createStrategies(); stateSyncConfigs.forEach(stateSyncConfig => { const toStorageMapper = stateSyncConfig.toStorageMapper || (s => s); const fromStorageMapper = stateSyncConfig.fromStorageMapper || (s => s); - const { toStorage, fromStorage, storageChange$ } = (isSyncStrategyFactory( - stateSyncConfig.syncStrategy - ) + const { toStorage, fromStorage, storageChange$ } = isSyncStrategy(stateSyncConfig.syncStrategy) ? stateSyncConfig.syncStrategy - : Strategies[stateSyncConfig.syncStrategy || SyncStrategy.Url])(stateSyncConfig.syncKey); + : syncStrategies[stateSyncConfig.syncStrategy || SyncStrategy.Url]; // returned boolean indicates if update happen - const updateState = (): boolean => { - if (ignoreStateUpdate) return false; - const update = (): boolean => { - const storageState = fromStorage(); - if (!storageState) { - ignoreStorageUpdate = false; - return false; - } - - if (storageState) { - stateSyncConfig.store.set({ - ...stateSyncConfig.store.get(), - ...fromStorageMapper(storageState), - }); - return true; - } - + const updateState = async (): Promise => { + const storageState = await fromStorage(stateSyncConfig.syncKey); + if (!storageState) { return false; - }; - - ignoreStorageUpdate = true; - const updated = update(); - ignoreStorageUpdate = false; - return updated; - }; - - // returned boolean indicates if update happen - const updateStorage = ({ replace = false } = {}): boolean => { - if (ignoreStorageUpdate) return false; + } - const update = () => { - const newStorageState = toStorageMapper(stateSyncConfig.store.get()); - toStorage(newStorageState, { replace }); + if (storageState) { + stateSyncConfig.store.set({ + ...stateSyncConfig.store.get(), + ...fromStorageMapper(storageState), + }); return true; - }; + } - ignoreStateUpdate = true; - const hasUpdated = update(); - ignoreStateUpdate = false; - return hasUpdated; + return false; }; - // initial syncing of store state and storage state - const initialTruthSource = stateSyncConfig.initialTruthSource ?? InitialTruthSource.Storage; - if (initialTruthSource === InitialTruthSource.Storage) { - const hasUpdated = updateState(); - // if there is nothing by state key in storage - // then we should fallback and consider state source of truth - if (!hasUpdated) { - updateStorage({ replace: true }); - } - } else if (initialTruthSource === InitialTruthSource.Store) { - updateStorage({ replace: true }); - } + // returned boolean indicates if update happen + const updateStorage = async ({ replace = false } = {}): Promise => { + const newStorageState = toStorageMapper(stateSyncConfig.store.get()); + await toStorage(stateSyncConfig.syncKey, newStorageState, { replace }); + return true; + }; + // subscribe to state and storage updates subscriptions.push( stateSyncConfig.store.state$ .pipe( @@ -413,14 +381,30 @@ export function syncState(config: IStateSyncConfig[] | IStateSyncConfig): Destro ) ) .subscribe(() => { - // TODO: batch storage updates updateStorage(); - }), - storageChange$.subscribe(() => { - // TODO: batch state updates? or should it be handled by state containers instead? - updateState(); - }) + }) ); + if (storageChange$) { + subscriptions.push( + storageChange$(stateSyncConfig.syncKey).subscribe(() => { + updateState(); + }) + ); + } + + // initial syncing of store state and storage state + const initialTruthSource = stateSyncConfig.initialTruthSource ?? InitialTruthSource.Storage; + if (initialTruthSource === InitialTruthSource.Storage) { + updateState().then(hasUpdated => { + // if there is nothing by state key in storage + // then we should fallback and consider state source of truth + if (!hasUpdated) { + updateStorage({ replace: true }); + } + }); + } else if (initialTruthSource === InitialTruthSource.Store) { + updateStorage({ replace: true }); + } }); return () => { diff --git a/src/plugins/kibana_utils/public/url/index.ts b/src/plugins/kibana_utils/public/url/index.ts index 9be82e781f6df..5f2763cd8ef31 100644 --- a/src/plugins/kibana_utils/public/url/index.ts +++ b/src/plugins/kibana_utils/public/url/index.ts @@ -33,10 +33,11 @@ import { HashedItemStoreSingleton, } from '../../../../legacy/ui/public/state_management/state_storage'; -const parseUrl = (url: string) => _parseUrl(url, true); -const parseUrlHash = (url: string) => parseUrl(parseUrl(url).hash!.slice(1)); -const parseCurrentUrl = () => parseUrl(window.location.href); -const parseCurrentUrlHash = () => parseUrlHash(window.location.href); +export const parseUrl = (url: string) => _parseUrl(url, true); +export const parseUrlHash = (url: string) => parseUrl(parseUrl(url).hash!.slice(1)); +export const getCurrentUrl = () => window.location.href; +export const parseCurrentUrl = () => parseUrl(getCurrentUrl()); +export const parseCurrentUrlHash = () => parseUrlHash(getCurrentUrl()); // encodeUriQuery implements the less-aggressive encoding done naturally by // the browser. We use it to generate the same urls the browser would @@ -136,34 +137,86 @@ export function setStateToUrl( /** * A tiny wrapper around history library to listen for url changes and update url * History library handles a bunch of cross browser edge cases - * - * listen(cb) - accepts a callback which will be called whenever url has changed - * update(url: string, replace: boolean) - get an absolute / relative url to update the location to */ -export const createUrlControls = () => { +export interface IUrlControls { + /** + * Allows to listen for url changes + * @param cb - get's called when url has been changed + */ + listen: (cb: () => void) => () => void; + + /** + * Updates url synchronously + * @param url - url to update to + * @param replace - use replace instead of push + */ + update: (url: string, replace: boolean) => string; + + /** + * Schedules url update to next microtask, + * Useful to ignore sync changes to url + * @param updater - fn which receives current url and should return next url to update to + * @param replace - use replace instead of push + */ + updateAsync: (updater: UrlUpdaterFnType, replace: boolean) => Promise; +} +export type UrlUpdaterFnType = (currentUrl: string) => string; + +export const createUrlControls = (): IUrlControls => { const history = createBrowserHistory(); + const updateQueue: Array<(currentUrl: string) => string> = []; + + // if we should replace or push with next async update, + // if any call in a queue asked to push, then we should push + let shouldReplace = true; + return { listen: (cb: () => void) => history.listen(() => { cb(); }), - update: (url: string, replace = false) => { - const { pathname, search } = parseUrl(url); - const parsedHash = parseUrlHash(url); - const searchQueryString = stringifyQueryString(parsedHash.query); - const location = { - pathname, - hash: formatUrl({ - pathname: parsedHash.pathname, - search: searchQueryString, - }), - search, - }; - if (replace) { - history.replace(location); - } else { - history.push(location); + update: (newUrl: string, replace = false) => updateUrl(newUrl, replace), + updateAsync: (updater: (currentUrl: string) => string, replace = false) => { + updateQueue.push(updater); + if (shouldReplace) { + shouldReplace = replace; } + + // Schedule url update to the next microtask + return Promise.resolve().then(() => { + if (updater.length === 0) return getCurrentUrl(); + const resultUrl = updateQueue.reduce((url, nextUpdate) => nextUpdate(url), getCurrentUrl()); + const newUrl = updateUrl(resultUrl, shouldReplace); + // queue clean up + updateQueue.splice(0, updateQueue.length); + shouldReplace = true; + + return newUrl; + }); }, }; + + function updateUrl(newUrl: string, replace = false): string { + if (newUrl === getCurrentUrl()) return getCurrentUrl(); + + const { pathname, search } = parseUrl(newUrl); + const parsedHash = parseUrlHash(newUrl); + const searchQueryString = stringifyQueryString(parsedHash.query); + const location = { + pathname, + hash: formatUrl({ + pathname: parsedHash.pathname, + search: searchQueryString, + }), + search, + }; + if (replace) { + history.replace(location); + } else { + history.push(location); + } + return getCurrentUrl(); + + return newUrl; + } };