Skip to content

chore(typecheck): clean up unused parameter#167396

Merged
jgowdyelastic merged 1 commit intoelastic:mainfrom
delanni:fix-ml-ui-ts-error
Sep 27, 2023
Merged

chore(typecheck): clean up unused parameter#167396
jgowdyelastic merged 1 commit intoelastic:mainfrom
delanni:fix-ml-ui-ts-error

Conversation

@delanni
Copy link
Contributor

@delanni delanni commented Sep 27, 2023

Summary

This unused parameter is causing typescript error in many builds:
https://buildkite.com/elastic/kibana-pull-request/builds/162839#018ad64e-f2db-4572-a2df-2a9257777077

proc [tsc] x-pack/plugins/transform/public/app/sections/create_transform/components/step_define/step_define_form.tsx:358:19 - error TS2322: Type '{ frozenDataPreference: FrozenTierPreference; setFrozenDataPreference: (value: FrozenTierPreference \| undefined) => void; dataView: DataView; query: undefined; disabled: false; timefilter: TimefilterContract; hideFrozenDataTierChoice: boolean; }' is not assignable to type 'IntrinsicAttributes & FullTimeRangeSelectorProps & { children?: ReactNode; }'.
--
  | proc [tsc]   Property 'hideFrozenDataTierChoice' does not exist on type 'IntrinsicAttributes & FullTimeRangeSelectorProps & { children?: ReactNode; }'.
  | proc [tsc]
  | proc [tsc] 358                   hideFrozenDataTierChoice={!showNodeInfo}

@delanni delanni added chore release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting v8.11.0 labels Sep 27, 2023
@delanni delanni marked this pull request as ready for review September 27, 2023 12:02
@delanni delanni requested a review from a team as a code owner September 27, 2023 12:02
Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for taking care of this, this happened probably because of two overlapping PRs merged in parallel.

@delanni
Copy link
Contributor Author

delanni commented Sep 27, 2023

@walterra - indeed, that's what I uncovered from my investigations 🕵️‍♂️ I hope this is a good fix

@delanni delanni enabled auto-merge (squash) September 27, 2023 12:37
@jgowdyelastic
Copy link
Member

jgowdyelastic commented Sep 27, 2023

I'd like to replace this PR with one that fixes the types, but also fixes the functionality which is broken because of this error.
#167404
If that's ok?


Edit: we discussed this in the team and agreed that getting this PR in first to unblock CI is better.
So I'll update my PR once this is merged.

@jgowdyelastic jgowdyelastic enabled auto-merge (squash) September 27, 2023 12:58
@delanni
Copy link
Contributor Author

delanni commented Sep 27, 2023

@jgowdyelastic - thanks, I tried to understand the behaviour, I thought it was a benign leftover, and the functionality is not broken.
Ultimately, I'll leave it up for your decision, it's your code. I would however agree, I think it's better to clean up main's type errors first to reduce noise for everyone.

@kibana-ci
Copy link

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
transform 401.1KB 400.8KB -245.0B

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

@jgowdyelastic jgowdyelastic merged commit c666e4d into elastic:main Sep 27, 2023
@delanni delanni deleted the fix-ml-ui-ts-error branch May 2, 2024 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting chore release_note:skip Skip the PR/issue when compiling release notes v8.11.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants