Skip to content

[APM] Refactor hooks and context#84615

Merged
sorenlouv merged 5 commits intoelastic:masterfrom
sorenlouv:refactor-hooks-context
Dec 2, 2020
Merged

[APM] Refactor hooks and context#84615
sorenlouv merged 5 commits intoelastic:masterfrom
sorenlouv:refactor-hooks-context

Conversation

@sorenlouv
Copy link
Copy Markdown
Contributor

@sorenlouv sorenlouv commented Dec 1, 2020

This PR refactors the hooks and context files:

  • Co-locate "context-consuming"-hooks with context (eg. co-locate use_url_params.ts and url_params_context.ts)
  • Move hooks that are only consumed by a single component inline where used (or to the folder where used)
  • adding "*Fetcher" suffix to data fetching hooks to indicate the implications of consuming it (this is different from calling a hook that consumes a context that internally fetches data, eg use_annotations).
  • Change casing from camelCase to snake_case

@sorenlouv sorenlouv requested a review from a team December 1, 2020 10:40
@sorenlouv sorenlouv requested a review from a team as a code owner December 1, 2020 10:40
@botelastic botelastic bot added Team:APM - DEPRECATED Use Team:obs-ux-infra_services. Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability labels Dec 1, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Dec 1, 2020

Pinging @elastic/apm-ui (Team:apm)

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Dec 1, 2020

Pinging @elastic/uptime (Team:uptime)

sorenlouv
sorenlouv commented Dec 1, 2020
View reviewed changes
cauemarcondes
cauemarcondes reviewed Dec 1, 2020
View reviewed changes
cauemarcondes
cauemarcondes reviewed Dec 1, 2020
View reviewed changes
cauemarcondes
cauemarcondes reviewed Dec 1, 2020
View reviewed changes
cauemarcondes
cauemarcondes reviewed Dec 1, 2020
View reviewed changes
x-pack/plugins/apm/public/components/app/ErrorGroupDetails/DetailView/index.tsx Outdated
Comment on lines 22 to 25
Copy link
Copy Markdown
Contributor

@cauemarcondes cauemarcondes Dec 1, 2020

Choose a reason for hiding this comment

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

Should we start importing types like this?
import type { IUrlParams } from '../../../../context/url_params_context/types';

cauemarcondes
cauemarcondes approved these changes Dec 1, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@cauemarcondes cauemarcondes left a comment

Choose a reason for hiding this comment

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

LGTM. just some suggestions.

@sorenlouv sorenlouv force-pushed the refactor-hooks-context branch from eb7bb6a to 5007f28 Compare December 1, 2020 15:15
@sorenlouv sorenlouv force-pushed the refactor-hooks-context branch from 5007f28 to cabd189 Compare December 1, 2020 23:10
@sorenlouv sorenlouv added release_note:skip Skip the PR/issue when compiling release notes v7.11.0 labels Dec 2, 2020
@sorenlouv
Copy link
Copy Markdown
Contributor Author

sorenlouv commented Dec 2, 2020

retest

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Dec 2, 2020

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
apm 1154 1155 +1

Async chunks

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

id before after diff
apm 3.1MB 3.1MB +3.0KB

History

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

Kerry350
Kerry350 approved these changes Dec 2, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@Kerry350 Kerry350 left a comment

Choose a reason for hiding this comment

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

UX changes LGTM

@sorenlouv sorenlouv merged commit 452f841 into elastic:master Dec 2, 2020
@sorenlouv sorenlouv deleted the refactor-hooks-context branch December 2, 2020 12:10
@sorenlouv sorenlouv mentioned this pull request Dec 2, 2020
Merged
gmmorris added a commit to gmmorris/kibana that referenced this pull request Dec 2, 2020
@gmmorris
Merge branch 'master' into alerts/force-schedule
261977c 
* master: (72 commits)
  Make alert status fetching more resilient (elastic#84676)
  [APM] Refactor hooks and context (elastic#84615)
  Added word break styles to the texts in the item details card. (elastic#84654)
  [Search] Disable "send to background" when auto-refresh is enabled (elastic#84106)
  Add readme for new palette service (elastic#84512)
  Make all providers to preserve original URL when session expires. (elastic#84229)
  [Lens] Show color in flyout instead of auto (elastic#84532)
  [Lens] Use index pattern through service instead of reading saved object (elastic#84432)
  Make it possible to use Kibana anonymous authentication provider with ES anonymous access. (elastic#84074)
  TelemetryCollectionManager: Use X-Pack strategy as an OSS overwrite (elastic#84477)
  migrate away from rest_total_hits_as_int (elastic#84508)
  [Input Control] Custom renderer (elastic#84423)
  Attempt to more granularly separate App Search vs Workplace Search vs shared GitHub notifications (elastic#84713)
  [Security Solutino][Case] Case connector alert UI (elastic#82405)
  [Maps] Support runtime fields in tooltips (elastic#84377)
  [CCR] Fix row actions in follower index and auto-follow pattern tables (elastic#84433)
  [Enterprise Search] Migrate shared Indexing Status component (elastic#84571)
  [maps] remove fields from index-pattern test artifacts (elastic#84379)
  Add routes for use in Sources Schema (elastic#84579)
  Changes UI links for drilldowns (elastic#83971)
  ...
sorenlouv added a commit that referenced this pull request Dec 2, 2020
@sorenlouv
[7.x] [APM] Refactor hooks and context (#84615) (#84756)
12bba53 
* Refactor hooks and context

* Fix tsc issues

* address feedback

* Fix lint

* type-only import
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cauemarcondes cauemarcondes cauemarcondes approved these changes

@Kerry350 Kerry350 Kerry350 approved these changes

Assignees

No one assigned

Labels

release_note:skip Skip the PR/issue when compiling release notes Team:APM - DEPRECATED Use Team:obs-ux-infra_services. Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.11.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@sorenlouv @elasticmachine @kibanamachine @Kerry350 @cauemarcondes