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

[APM] @ts-error -> @ts-expect-error #76492

Merged

Conversation

sorenlouv
Copy link
Member

@sorenlouv sorenlouv commented Sep 2, 2020

I noticed a couple of unnecessary @ts-ignore declarations and decided to replace them all with @ts-expect-error which uncovered a bunch more that could be removed.

The advantage of @ts-expect-error is that we'll be forced to remove them as they are no longer relevant. We could also start taking advantage of them in tests.
Example: is used @ts-expect-error in a test where I wanted to ensure that a property was both undefined from a type perspective and from a runtime value perspective:

https://github.com/sqren/backport/blob/acdbebda269bfb472ffc69f4650c036765a694f6/src/options/cliArgs.test.ts#L92-L93

@sorenlouv sorenlouv added backport:skip This commit does not require backporting v7.10.0 labels Sep 2, 2020
@sorenlouv sorenlouv requested a review from a team as a code owner September 2, 2020 10:32
@botelastic botelastic bot added the Team:APM All issues that need APM UI Team support label Sep 2, 2020
@elasticmachine
Copy link
Contributor

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

@sorenlouv sorenlouv added release_note:skip Skip the PR/issue when compiling release notes and removed backport:skip This commit does not require backporting labels Sep 2, 2020
@@ -12,7 +12,6 @@ import { px, unit } from '../../../style/variables';
import { Stacktrace } from '.';
import { Stackframe } from '../../../../typings/es_schemas/raw/fields/stackframe';

// @ts-ignore Styled Components has trouble inferring the types of the default props here.
Copy link
Member Author

Choose a reason for hiding this comment

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

Apparently not anymore :D

Copy link
Contributor

@smith smith left a comment

Choose a reason for hiding this comment

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

👍 once conflicts fixed.

tenor-34944576

@sorenlouv
Copy link
Member Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

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

@sorenlouv sorenlouv merged commit e0f4775 into elastic:master Sep 4, 2020
@sorenlouv sorenlouv deleted the replace-ts-error-with-ts-expect-error branch September 4, 2020 04:52
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 4, 2020
…rok/new-patterns-component-use-array

* 'master' of github.com:elastic/kibana: (75 commits)
  Remove legacy ui-apps-mixin (elastic#76604)
  remove unused test_utils (elastic#76528)
  [ML] Functional tests - add UI permission tests (elastic#76368)
  [APM] @ts-error -> @ts-expect-error (elastic#76492)
  [APM] Avoid negative offset for error marker on timeline (elastic#76638)
  [Reporting] rename interfaces to align with task manager integration (elastic#76716)
  Revert back ESO migration for alerting, added try/catch logic to avoid failing Kibana on start (elastic#76220)
  Test reverting "Add plugin status API (elastic#75819)" (elastic#76707)
  [Security Solution][Detections] Removes ML Job Settings SIEM copy and fixes link to ML app for creating custom jobs (elastic#76595)
  [Maps] remove region/coordinate-maps visualizations from sample data (elastic#76399)
  [DOCS] Dashboard-first docs refresh (elastic#76194)
  Updated ServiceNow description in docs and actions management UI to contains correct info (elastic#76344)
  [DOCS] Identifies cloud settings in reporting (elastic#76691)
  [Security Solution] Refactor timeline details to use search strategy (elastic#75591)
  es-archiver: Drop invalid index settings, support --query flag  (elastic#76522)
  [DOCS] Identifies graph settings available on cloud (elastic#76661)
  Add more info about a11y tests (elastic#76045)
  [data.search.SearchSource] Remove legacy ES client APIs. (elastic#75943)
  [release notes] automatically retry on Github API 5xx errors (elastic#76447)
  [es_ui_shared] Fix eslint exhaustive deps rule (elastic#76392)
  ...
sorenlouv added a commit to sorenlouv/kibana that referenced this pull request Sep 4, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
sorenlouv added a commit that referenced this pull request Sep 4, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support v7.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants