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] Introduce Redux and migrate DiscoverInternalStateContainer #213304

Closed
davismcphee opened this issue Mar 6, 2025 · 1 comment · Fixed by #208784
Closed

[Discover] Introduce Redux and migrate DiscoverInternalStateContainer #213304

davismcphee opened this issue Mar 6, 2025 · 1 comment · Fixed by #208784
Assignees
Labels
Feature:Discover Discover Application impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:medium Medium Level of Effort Project:OneDiscover Enrich Discover with contextual awareness refactoring Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL.

Comments

@davismcphee
Copy link
Contributor

📓 Summary

The first step of the state refactoring to support tabs is introducing a proper state management library to help centralize all Discover state. Tabs require all state to be serializable so it can be persisted to local storage, which is enforced by default in Redux Toolkit. That in addition to precedent elsewhere in Kibana Analytics apps, as well as broad support in the React ecosystem, make Redux a good fit.

To build the foundation, we will migrate DiscoverInternalStateContainer to Redux, one of several "state containers" currently used in Discover that will be centralized in a single Redux store for tabs support.

Additionally, we will build a lightweight RuntimeStateManager to centrally manage non-serializable ephemeral state such as the current data view. This will only be used for state which does not need to be persisted and has no serializable representation.

✔️ Acceptance criteria

  • Create a new central Redux store in Discover.
  • Migrate DiscoverInternalStateContainer to Redux and remove the existing state container.
  • Create a lightweight RuntimeStateManager for managing non-serializable runtime state.
  • There should be no functionality changes in Discover.
@davismcphee davismcphee added Feature:Discover Discover Application impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:medium Medium Level of Effort Project:OneDiscover Enrich Discover with contextual awareness refactoring Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. labels Mar 6, 2025
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@davismcphee davismcphee self-assigned this Mar 6, 2025
davismcphee added a commit to davismcphee/kibana that referenced this issue Mar 7, 2025
…`InternalStateStore` (elastic#208784)

## Summary

This PR replaces Discover's current `DiscoverInternalStateContainer`
(based on Kibana's custom `ReduxLikeStateContainer`) with an actual
Redux store using Redux Toolkit. It's the first step toward migrating
all of Discover's state management to Redux as part of the Discover tabs
project.

Part of elastic#210160.
Resolves elastic#213304.

### Checklist

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [x] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit ccae358)

# Conflicts:
#	src/platform/packages/shared/kbn-discover-utils/src/__mocks__/data_view.ts
#	src/platform/plugins/shared/discover/public/application/main/components/top_nav/on_save_search.tsx
#	src/platform/plugins/shared/discover/public/application/main/data_fetching/update_search_source.test.ts
#	src/platform/plugins/shared/discover/public/application/main/data_fetching/update_search_source.ts
#	src/platform/plugins/shared/discover/public/context_awareness/hooks/use_default_ad_hoc_data_views.test.tsx
#	src/platform/plugins/shared/discover/tsconfig.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Discover Discover Application impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:medium Medium Level of Effort Project:OneDiscover Enrich Discover with contextual awareness refactoring Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL.
Projects
None yet
2 participants