Skip to content

IndexPattern class - no longer use getConfig or uiSettingsValues#75717

Merged
mattkime merged 9 commits intoelastic:masterfrom
mattkime:index_pattern_no_longer_uses_gett_config
Aug 25, 2020
Merged

IndexPattern class - no longer use getConfig or uiSettingsValues#75717
mattkime merged 9 commits intoelastic:masterfrom
mattkime:index_pattern_no_longer_uses_gett_config

Conversation

@mattkime
Copy link
Contributor

@mattkime mattkime commented Aug 22, 2020

Summary

  • IndexPattern class no longer uses getConfig or uiSettingsValues dependencies. It now takes shortDotsEnable (boolean) and metaFields (string[]) as arguments - these were formerly provided by uiSettings

Checklist

Delete any items that are not applicable to this PR.

For maintainers

Dev docs

IndexPattern class no longer uses getConfig or uiSettingsValues dependencies. It now takes shortDotsEnable (boolean) and metaFields (string[]) as arguments - these were formerly provided by uiSettings

@elastic elastic deleted a comment from kibanamachine Aug 23, 2020
@elastic elastic deleted a comment from kibanamachine Aug 23, 2020
@mattkime mattkime changed the title remove getConfig from IndexPattern class IndexPattern class - no longer use getConfig or uiSettingsValues Aug 23, 2020
@mattkime mattkime added Feature:Data Views Data Views code and UI - index patterns before 8.0 Team:AppArch v8.0.0 v7.10.0 labels Aug 23, 2020
@mattkime mattkime marked this pull request as ready for review August 23, 2020 05:13
@mattkime mattkime requested a review from a team as a code owner August 23, 2020 05:13
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@mattkime mattkime added the release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. label Aug 23, 2020
@mattkime mattkime linked an issue Aug 24, 2020 that may be closed by this pull request
@mattkime
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
data 543 +1 542

page load bundle size

id value diff baseline
data 1.4MB +2.3KB 1.4MB

History

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

Copy link
Contributor

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code LGTM, but i am wondering if this does have some functional changes ?

before we would use getConfig to read the config value at the time we needed it. now we read the config value once when we create the index pattern instance and reuse it later when using the index pattern. Could this lead to any differences in behaviour where a setting would be changed from the time we create index pattern till we use the method on it that uses the config ?

i am assuming no, since the only way to change the setting is to navigate to management. Navigating from there to anywhere in kibana we will recreate the index pattern instances. We need to make sure we don't put index pattern instances to any global state. If we would need to make index pattern available globally we should share the spec rather than the instance.

maybe this is worth putting into a comment (jsdocs for index pattern class)

@mattkime
Copy link
Contributor Author

Navigating from there to anywhere in kibana we will recreate the index pattern instances.

This is correct. Yes, its worth documenting.

@mattkime mattkime merged commit 7fa23a4 into elastic:master Aug 25, 2020
mattkime added a commit to mattkime/kibana that referenced this pull request Aug 25, 2020
…lastic#75717)

* remove getConfig and uiSettingsValues from IndexPattern class
# Conflicts:
#	docs/development/plugins/data/public/kibana-plugin-plugins-data-public.indexpattern._constructor_.md
#	docs/development/plugins/data/public/kibana-plugin-plugins-data-public.indexpattern.md
#	src/plugins/data/common/index_patterns/index_patterns/index_pattern.test.ts
#	src/plugins/data/common/index_patterns/index_patterns/index_pattern.ts
#	src/plugins/data/common/index_patterns/index_patterns/index_patterns.ts
#	src/plugins/data/public/public.api.md
mattkime added a commit that referenced this pull request Aug 25, 2020
…75717) (#75919)

* remove getConfig and uiSettingsValues from IndexPattern class
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Data Views Data Views code and UI - index patterns before 8.0 release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. v7.10.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make index pattern field / field list classes static

4 participants