[react@18] Fix useCallback breaking type changes

[patrykkopycinski]

Summary

Prep work for React@18 bump https://github.com/elastic/kibana/issues/138222

In React@18 useCallback types has changed that introduced breaking changes: https://github.com/DefinitelyTyped/DefinitelyTyped/issues/46691

Fixed using: https://github.com/eps1lon/types-react-codemod?tab=readme-ov-file#usecallback-implicit-any

Tried to do my best with fixing the types, but if you disagree or have a better idea how it should be solved feel free to suggest changes or commit directly to the branch 🙏

[patrykkopycinski]

/ci

[patrykkopycinski]

/ci

[elasticmachine]

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

[elasticmachine]

Pinging @elastic/fleet (Team:Fleet)

[elasticmachine]

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

[semd]

Thanks @patrykkopycinski! Committed changes in a couple of files here https://github.com/elastic/kibana/pull/182344/commits/fa09b52d9b2e6ef180eb0ab1ac8401434ef7334a

[patrykkopycinski]

Thank you so much for your support @semd @sebelga @nickpeihl @stratoula @rylnd 🙇

[patrykkopycinski]

@patrykkopycinski thank you for this huge effort to address useCallback typing changes 👍

My opinion and comments below should be considered as nit.

I see that there is more than one way to specify typings here. I may add confusion which approach is better and what should be used. In my own opinion the following rules should be used

• simpler is better (easy to understand, less typings, less imports, less symbols)
• for primitive or well known types should be specified as function params, e.g.

const handleChange = useCallback((change: string) => { /*...*/ });

or

const handleMouseClick = useCallback((change: React.MouseEvent) => { /*...*/ });

• for one parameter is usually simpler to specify it explicitly, e.g.

const onTimeRangeChange = useCallback(
  ({ dateRange }: { dateRange: TimeRange }) => { /* ... */ });

• prefer const typing instead passing to useCallback, e.g.

const handleApplyAssignees: AssigneesApplyPanelProps['onApply'] = useCallback();

• avoid using of NonNullable when possible. IMO it obscures the initial goal which is defining a callback function with non any input params. At the next step TS will make sure this callback function is compatible with input property.

Unfortunately sometimes there is absence of explicit exported input type which represents a union of two or more types. Redefining it locally of course doesn't make sense since it will make code more brittle since any further change in component typings may affect our code. In this case it's much better to type the function referencec constant or the whole callback function.

Thank you @maximpn 🙇

I agree that there is more than one way to define typing here, the approach I tried to follow was to use the reuse component's prop types as much as possible to make sure we are using the right props etc. I do also agree that in many cases it would be easier and cleaner just use the correct type instead of adding imports, NonNullable and others, but at the same time, as I was going through the codebase I have noticed that we tend to duplicate the same types over and over, so I assumed the approach I took would be more beneficial in the longterm. But like I wrote at the beginning, I'm not going to introduce or force any of the ways, my goal was only to prepare the codebase for the React@18 migration, so I leave it to you all the decision about the preferred way :)

[qn895]

ML changes LGTM 🎉

[patrykkopycinski]

/ci

[Dosant]

@elastic/appex-sharedux team is looking into upgrading to React@18 and merging this impressive PR is a logical next step. @patrykkopycinski is on PTO, but I hope he wouldn't mind if we try to finalize it without waiting for him to return. I merged in the main and resolved the conflicts. It doesn't seem like there're any blockers apart from the remaining reviews and green ci.

[patrykkopycinski]

Thank you @Dosant 🙏

[Dosant]

@elasticmachine merge upstream

[kibana-ci]

:yellow_heart: Build succeeded, but was flaky

• Buildkite Build
• Commit: 1c5000369354a9ffc777c742f380d269e938f208
• Storybooks Preview
• Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-182344-1c5000369354

Failed CI Steps

• FTR Configs #47

Test Failures

• [job] [logs] FTR Configs #47 / Cloud Security Posture Test adding Cloud Security Posture Integrations CSPM AWS CIS_AWS Organization Manual Direct Access CIS_AWS Organization Manual Direct Access Workflow

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
@kbn/dom-drag-drop	27	30	+3
@kbn/visualization-ui-components	143	134	-9
logsShared	272	282	+10
visDefaultEditor	49	63	+14
total			+18

Async chunks

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

id	before	after	diff
cases	480.2KB	480.2KB	-40.0B
cloudSecurityPosture	481.1KB	481.1KB	+23.0B
discover	855.8KB	855.8KB	+23.0B
esqlDataGrid	129.4KB	129.5KB	+23.0B
infra	1.5MB	1.5MB	+20.0B
ingestPipelines	375.1KB	375.1KB	+1.0B
lens	1.5MB	1.5MB	+76.0B
securitySolution	18.0MB	18.0MB	+473.0B
slo	916.6KB	916.6KB	+23.0B
stackConnectors	580.3KB	580.3KB	+12.0B
triggersActionsUi	1.6MB	1.6MB	+38.0B
unifiedHistogram	67.2KB	67.2KB	+46.0B
unifiedSearch	219.0KB	219.0KB	-7.0B
total			+711.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
@kbn/grouping	10	11	+1
@kbn/visualization-ui-components	3	4	+1
total			+2

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
visDefaultEditor	24.0KB	24.0KB	+24.0B

Unknown metric groups

API count

id	before	after	diff
@kbn/dom-drag-drop	41	57	+16
@kbn/visualization-ui-components	146	137	-9
logsShared	300	310	+10
visDefaultEditor	56	70	+14
total			+31

ESLint disabled in files

id	before	after	diff
securitySolution	85	86	+1

ESLint disabled line counts

id	before	after	diff
infra	38	39	+1

Total ESLint disabled count

id	before	after	diff
infra	47	48	+1
securitySolution	622	623	+1
total			+2

History

• :broken_heart: Build #211879 failed 2f54dcc54dd44c1cb05d2dd41077c5dfc381894a
• :green_heart: Build #210921 succeeded 3ceb84d024904c82dff50372b7961843305a3edc
• :broken_heart: Build #210912 failed 1625165a23adc93ab423563517450f5d5f65cf96
• :broken_heart: Build #210840 failed 79877ffea239019b17ce29257e4887bbbb66feed
• :broken_heart: Build #210597 failed aae243c9f2336339295b87e54daac89e66386b8c

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

cc @patrykkopycinski