-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[APM] Prefer APIReturnType
over PromiseReturnType
#83843
[APM] Prefer APIReturnType
over PromiseReturnType
#83843
Conversation
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
'GET /api/apm/services/{serviceName}/transaction_groups/distribution' | ||
>; | ||
|
||
type DistributionBucket = DistributionApiResponse['buckets'][0]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I usually use ValuesType
(from utility-types
), as it works for both arrays and objects, and uses a number
as an indexer. If you have a tuple for instance, if you use 0 as an indexer, the type would be of the first item of the array, but with ValuesType it would (more correctly) be the union type of all the items in the array. But it's more typing ofc, and [0]
is correct in most of the cases. 😀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, I'm good with using ValueType
instead of [0]
. I think there is a bunch of other places where we already use [0]
. You okay with merging this as-is and then we change it everywhere in another PR (or gradually)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Pinging @elastic/apm-ui (Team:apm) |
* master: (38 commits) [ML] Data frame analytics: Adds functionality to map view (elastic#83710) Add usage collection for savedObject tagging (elastic#83160) [SECURITY_SOLUTION] 145: Advanced Policy Tests (elastic#82898) [APM] Service overview transactions table (elastic#83429) [ML] Fix Single Metric Viewer not loading if job is metric with no partition (elastic#83880) do not export types from 3rd party modules as 'type' (elastic#83803) [Fleet] Allow to send SETTINGS action (elastic#83707) Fixes Failing test: Chrome X-Pack UI Functional Tests.x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/details·ts - Actions and Triggers app Alert Details Alert Instances renders the active alert instances (elastic#83478) [Uptime]Reduce chart height on monitor detail page (elastic#83777) [APM] Prefer `APIReturnType` over `PromiseReturnType` (elastic#83843) [Observability] Fix telemetry for Observability Overview (elastic#83847) [Alerting] Adds generic UI for the definition of conditions for Action Groups (elastic#83278) ensure workload agg doesnt run until next interval when it fails (elastic#83632) [ILM] Policy form should not throw away data (elastic#83077) [Monitoring] Stop collecting Kibana Usage in bulkUploader (elastic#83546) [TSVB] fix wrong imports (elastic#83798) [APM] Correlations UI POC (elastic#82256) list all the refs in tsconfig.json (elastic#83678) Bump jest (and related packages) to v26.6.3 (elastic#83724) Functional tests - stabilize reporting tests for cloud execution (elastic#83787) ...
Instead of using
PromiseReturnType
I suggest we useAPIReturnType
since it will return a type that is closer to the actual API response.Additionally this will help us get rid of the pesky eslint-ignore flags.