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

[7.x] [Enterprise Search] Move http out of React Context and to Kea Logic; prefer directly mounting (#78167) #78204

Merged
merged 1 commit into from
Sep 22, 2020

Conversation

cee-chen
Copy link
Member

Backports the following commits to 7.x:

…prefer directly mounting (elastic#78167)

* Remove HttpProvider in favor of mounting HttpLogic directly w/ props

- removes need for initializeHttp call
- ensures http value is loaded into HttpLogic as soon as possible / should never load in as null, reducing # of rerenders/checks

see: https://kea.js.org/docs/guide/advanced#mounting-and-unmounting

* Update simplest components using http for sendTelemetry

* Update simplest tests for components using HttpLogic + default Kea mocks

- Kea mock import should now contain mock default values which can be overridden

* Update moderately complex tests using HttpLogic

send_telemetry:
- refactor to use shallow (w/ useEffect mocked) vs mount
- check mockHttpValues directly

engine_table:
- refactor to use mount w/ an I18nProvider rather than mountWithContext helper (which we'll likely need to overhaul in the future)
- assert mockHttpValues directly

* Update EngineOverview to HttpLogic + refactors

EngineOverview:
- Change use of FormattedMessage to i18n.translate (simpler, no provider required)

Tests:
- Create mock values/actions for FlashMessages, since EngineOverview calls it
- Create combined mockAllValues obj for easier overriding
- Create setMockValues helper for easier test overriding (credit to @scottybollinger for the idea!)
- Update engine_overview tests to setMockValues instead of passing context to mountWithAsyncContext
- Fix mountWithAsyncContext to accept an undefined obj

* Remove http from KibanaContext

- it should now only live in HttpLogic 🔥

* Remove FlashMessagesProvider in favor of mounting logic directly w/ props

- send history as prop
- refactor out now-unnecessary listenToHistory (we can just do it directly in afterMount without worrying about duplicate react rerenders)
- add mount helper

Tests:
- refactor history.listen mock to mockHistory (so that set_message_helpers can use it as well)
- use mountFlashMessagesLogic + create an even shorter mount() helper (credit to @JasonStoltz for the idea!)
- refactor out DEFAULT_VALUES since we're not really using it anywhere else in the file, and it's not super applicable to this store
- update history listener tests to account for logic occurring immediately on mount
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
enterpriseSearch 275 -2 277

async chunks size

id value diff baseline
enterpriseSearch 509.4KB -2.6KB 512.0KB

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

@cee-chen cee-chen merged commit 52c37b8 into elastic:7.x Sep 22, 2020
@cee-chen cee-chen deleted the backport/7.x/pr-78167 branch September 22, 2020 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants