Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Discover] Create enhanced central state container #140765

Closed
wants to merge 86 commits into from

Conversation

kertal
Copy link
Member

@kertal kertal commented Sep 14, 2022

Summary

WIP to work on Discover's state management, it's scattered around the code base (saved search, appState, dataState via observables), cleaning, and centralizing stuff, not intended to be merged for now. This PR would be an intermediate step to clean up the current way centralized state is being handled. Its focus is to get a clearer structure, better separation from UI, removing of hook dependencies making it hard to debug the code. So refactoring without regressions, which makes it easier to further migrate to e.g. redux-toolkit

Parts of this PR

  • Extending discover_state.ts, which was originally just used to access and sync appState with the URL. the new extended DiscoverStateContainer consists of 4 main parts (appState, savedSearch, dataState, internalState)
  • Remove the useSavedSearch hook with it's endless hook dependencies
  • Consolidation of the usage of searchSource and savedSearch. Despite searchSource being part of savedSearch, we were using those variables in parallel. Now savedSearch is part of DiscoverStateContainer, and this is used to get searchSource when needed
  • Before this PR the state container was initialized twice, now there's just a single instance
  • SearchSource is not used as intended. Now there's a parent searchSource, that's also persisted in the savedSearch saved object, and for fetching the document, histogram, the number of hits, volatile search sources children are created and additional parameters are applied.
  • It adds a logger (addLog) function, that can be enabled using the browser's console, making it easier to debug state changes ... also for other use cases of course. By entering window?.ELASTIC_DISCOVER_LOGGER=true in the browser's console, logging messages are displayed in the console like this:
    Bildschirmfoto 2022-10-28 um 09 45 58 This was very useful to debug, it was also helpful to add it in the root plugin of Discover for functional testing.
  • There's now an "Unsaved changes" badge displayed, like in Dashboard when changes are applied to a new or persisted saved search.
    Bildschirmfoto 2022-10-28 um 10 07 15

DiscoverStateContainer

export interface DiscoverStateContainer {
  /**
   * App state, the _a part of the URL - This is btw a redux like state
   */
  appStateContainer: ReduxLikeStateContainer<AppState>;
  /**
   * New, to access savedSearch related functionality (also for searchSource) 
  **/
  savedSearchContainer
  /**
   * New, data fetching related state migrated from `use_saved_search.ts`
   **/
  dataStateContainer
  /**
   * Internal state that's used at several places in the UI
   */
   internalState: InternalStateContainer;
...
some more stuff from the old container
...
 /**
  * functionality triggered from UI
  **/
  actions: {
    resetSavedSearch: (id: string) => void;
    onOpenSavedSearch: (newSavedSearchId: string) => void;
    onUpdateQuery: (
      payload: { dateRange: TimeRange; query?: Query | AggregateQuery },
      isUpdate?: boolean
    ) => void;
    updateSavedQueryId: (newSavedQueryId: string | undefined) => void;
    /**
     * Pause the auto refresh interval without pushing an entry to history
     */
    pauseAutoRefreshInterval: () => Promise<void>;
    fetch: (reset?: boolean) => void;
  }
 /**
  * And even more functions
  **/
}

Checklist

For maintainers

@kertal kertal self-assigned this Sep 20, 2022
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
discover 512 517 +5

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
dataViewFieldEditor 30 32 +2
savedSearch 43 46 +3
total +5

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
dataViewFieldEditor 154.4KB 154.4KB -5.0B
discover 461.0KB 457.0KB -4.0KB
total -4.0KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
dataViewFieldEditor 22.8KB 22.8KB +4.0B
discover 26.7KB 26.6KB -70.0B
savedSearch 5.1KB 5.2KB +54.0B
unifiedSearch 49.6KB 49.7KB +30.0B
total +18.0B
Unknown metric groups

API count

id before after diff
dataViewFieldEditor 60 62 +2
savedSearch 43 46 +3
total +5

async chunk count

id before after diff
discover 10 11 +1

ESLint disabled line counts

id before after diff
discover 35 37 +2

References to deprecated APIs

id before after diff
discover 22 15 -7

Total ESLint disabled count

id before after diff
discover 36 38 +2

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @kertal

@kertal
Copy link
Member Author

kertal commented Mar 23, 2023

closing in favor of #153528

@kertal
Copy link
Member Author

kertal commented Apr 17, 2023

Closing since it's finished in #153528

@kertal kertal closed this Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Discover Discover Application Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. WIP Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants