Skip to content

[Fix] fix type issues from unparameterized PropsWithChildren type usages#182014

Merged
mistic merged 4 commits intoelastic:mainfrom
delanni:fix-type-errors-re-propswithchildren
Apr 29, 2024
Merged

[Fix] fix type issues from unparameterized PropsWithChildren type usages#182014
mistic merged 4 commits intoelastic:mainfrom
delanni:fix-type-errors-re-propswithchildren

Conversation

@delanni
Copy link
Copy Markdown
Member

@delanni delanni commented Apr 29, 2024 •
edited by kibanamachine
Loading

Summary

Original problem: PropsWithChildren require a generic type parameter (there's no default). This was not made visible in the merged PR, because we had type-checking on the PRs temporarily (accidentally) removed.

Thsi PR fixes the fallout from #181257 => Errors: https://buildkite.com/elastic/kibana-on-merge/builds/44454

@delanni delanni added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels Apr 29, 2024
@delanni delanni requested review from a team as code owners April 29, 2024 17:09
@delanni delanni added v8.14.0 v8.15.0 and removed backport:skip This PR does not require backporting labels Apr 29, 2024
@delanni delanni requested a review from a team as a code owner April 29, 2024 18:01
@delanni delanni mentioned this pull request Apr 29, 2024
Closed
@delanni delanni requested review from a team as code owners April 29, 2024 19:49
@delanni delanni requested a review from a team April 29, 2024 19:49
@delanni delanni requested review from a team as code owners April 29, 2024 19:49
@delanni delanni requested a review from a team April 29, 2024 19:49
@delanni delanni requested review from a team as code owners April 29, 2024 19:49
@botelastic botelastic bot added ci:project-deploy-observability Create an Observability project Team:Obs AI Assistant Observability AI Assistant Team:obs-ux-infra_services - DEPRECATED DEPRECATED - Use Team:obs-presentation. labels Apr 29, 2024
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Apr 29, 2024

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

@pzl pzl requested review from tomsonpl and removed request for pzl April 29, 2024 19:50
delanni
delanni commented Apr 29, 2024
View reviewed changes
.../plugins/ml/public/application/components/field_stats_flyout/field_stats_flyout_provider.tsx

export const FieldStatsFlyoutProvider: FC<{
children: React.ReactNode;
children?: React.ReactNode;
Copy link
Copy Markdown
Member Author

@delanni delanni Apr 29, 2024

Choose a reason for hiding this comment

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

This was causing an error in x-pack/plugins/ml/public/application/components/field_stats_flyout/field_stats_flyout_provider.tsx if left as a required field

delanni
delanni commented Apr 29, 2024
View reviewed changes
...olution/infra/public/pages/metrics/metrics_explorer/hooks/use_metrics_explorer_data.test.tsx

const renderUseMetricsExplorerDataHook = () => {
const wrapper: FC<PropsWithChildren> = ({ children }) => {
const wrapper: FC<PropsWithChildren<any>> = ({ children }) => {
Copy link
Copy Markdown
Member Author

@delanni delanni Apr 29, 2024

Choose a reason for hiding this comment

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

This was an any prior to the original PR as well, by committing the type for FC, this just makes it explicit. Would require a precise type.

delanni
delanni commented Apr 29, 2024
View reviewed changes
x-pack/plugins/threat_intelligence/public/mocks/test_providers.tsx
};

export const TestProvidersComponent: FC<PropsWithChildren> = ({ children }) => (
export const TestProvidersComponent: FC<PropsWithChildren<any>> = ({ children }) => (
Copy link
Copy Markdown
Member Author

@delanni delanni Apr 29, 2024

Choose a reason for hiding this comment

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

If left as unknown it would cause issues where TestProvidersComponent is used

@kibana-ci
Copy link
Copy Markdown

kibana-ci commented Apr 29, 2024

💚 Build Succeeded

Metrics [docs]

Any counts in public APIs

Total count of every any typed public API. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats any for more detailed information.

id before after diff
@kbn/i18n-react 1 0 -1
@kbn/ml-url-state 2 1 -1
esUiShared 4 3 -1
presentationUtil 3 2 -1
total -4

History

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

tsullivan
tsullivan approved these changes Apr 29, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@tsullivan tsullivan left a comment

Choose a reason for hiding this comment

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

Shared UX changes LGTM

@mistic mistic merged commit 4a90df2 into elastic:main Apr 29, 2024
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Apr 29, 2024

💔 All backports failed

Status Branch Result
❌ 8.14 Backport failed because of merge conflicts

You might need to backport the following PRs to 8.14:
- Add explicit children types (#181257)

Manual backport

To create the backport manually run:

node scripts/backport --pr 182014

Questions ?

Please refer to the Backport tool documentation

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented May 1, 2024

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 182014 locally

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label May 1, 2024
@kibanamachine kibanamachine mentioned this pull request May 13, 2024
Merged
@kibanamachine kibanamachine mentioned this pull request Jul 4, 2024
Merged
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Jul 31, 2024

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 182014 locally

2 similar comments
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Aug 2, 2024

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 182014 locally

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Aug 5, 2024

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 182014 locally

@delanni delanni mentioned this pull request Aug 6, 2024
Merged
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Aug 8, 2024

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 182014 locally

@delanni delanni added backport:skip This PR does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. v8.14.0 labels Aug 9, 2024
@delanni delanni deleted the fix-type-errors-re-propswithchildren branch August 9, 2024 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tsullivan tsullivan tsullivan approved these changes

@mistic mistic mistic approved these changes

@paul-tavares paul-tavares Awaiting requested review from paul-tavares paul-tavares was automatically assigned from elastic/security-defend-workflows

@dplumlee dplumlee Awaiting requested review from dplumlee dplumlee was automatically assigned from elastic/security-detection-rule-management

@tomsonpl tomsonpl Awaiting requested review from tomsonpl

Assignees

No one assigned

Labels

apm:review backport:skip This PR does not require backporting ci:project-deploy-observability Create an Observability project release_note:skip Skip the PR/issue when compiling release notes Team:Obs AI Assistant Observability AI Assistant Team:obs-ux-infra_services - DEPRECATED DEPRECATED - Use Team:obs-presentation. v8.15.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@delanni @elasticmachine @kibana-ci @kibanamachine @tsullivan @mistic @smith