feat: TypeScript v5 prep work #7340

tkajtoch · 2023-11-03T16:14:17Z

Summary

This PR adds the code changes needed for TypeScript v5 to work with EUI and not report any type errors.

I'm not upgrading to typescript version 5 here. It requires additional work to get prop-loader.js updated to use the new compiler API and will be added in a separate PR.

QA

Build this branch locally and use it in a local kibana instance to ensure there are no type errors there

General checklist

Docs site QA
- Props have proper autodocs (using @default if default values are missing) and playground toggles
- Checked Code Sandbox works for any docs examples
Code quality checklist
Release checklist
- A changelog entry exists and is marked appropriately.
- If applicable, added the breaking change issue label (and filled out the breaking change checklist)

cee-chen

I have a couple super minor nitty comments, all optional! I have 1-2 more higher level questions after this that I'll ask separately

upcoming_changelogs/7340.md

src/utils/prop_types/is.ts

cee-chen · 2023-11-06T20:06:08Z

src/components/side_nav/side_nav.tsx

@@ -146,7 +146,9 @@ export class EuiSideNav<T> extends Component<EuiSideNavProps<T>> {
          items={renderedItems}
          key={id}
          depth={depth}
-          renderItem={renderItem}
+          renderItem={
+            renderItem as PropsOf<typeof EuiSideNavItem>['renderItem']


[optional] Just curious, why not import EuiSideNavItemProps directly and do EuiSideNavItemProps['renderItem']? Does TS still yell about that?

The EuiSideNavItem props type is defined as T extends _EuiSideNavItemButtonProps & _EuiSideNavItemProps & { renderItem?: (props: any) => JSX.Element }, but the EuiSideNavItemProps type is T extends { renderItem: Function } ? T & { renderItem: RenderItem<T> } : T.

When working on that, I think I decided to keep the underlying component type the same and just use PropsOf instead of updating renderItem type to accept stricter RenderItem<T> and EuiSideNavItemProps export the props type of EuiSideNavItem instead of being a weird conditional that I don't fully understand the necessity of.

I think the reason why renderItem isn't included in _EuiSideNavItemProps is because it's coming from the EuiSideNav component and is meant to be only used internally, and EuiSideNavItem component isn't exported as a public component. The way this is achieved, though, is really confusing, and I'd prefer to take a deeper look at it in a separate PR.

👍 Super valid, thanks for explaining!

src/components/markdown_editor/markdown_editor.tsx

cee-chen

LGTM, I only had nits! Feel free to address or ignore at your discretion. Would love answers to the below questions if you don't mind educating me!

cee-chen · 2023-11-06T20:28:55Z

upcoming_changelogs/7340.md

+- Updated generic types of `EuiBasicTable`, `EuiInMemoryTable`
+  and `EuiSearchBar.Query.execute` to add `extends object` constraint


[attaching to a semi-random line for threading]

If this resolves vscode's millions of problems every time I open up the basic table/memory table files, you are a god

Can you ELI5 to me what changed here between TS 4 and TS 5 that made this change necessary? Is it just that TS 5 no longer auto infers the extend?

The amount of extends objectss we had to add feels like a lot - I'm wondering if there's a more "Typescript-y" way to DRY that out (no worries if the answer is no, I'm just thinking out loud)

TS5 auto infers types as before, but the strict mode and overall type resolution work better now, reporting more errors where our types weren't strong enough to error out before. Adding these constraints fixes these implicit any types

…it any` errors

…`false` to fix no type overlap typescript error

…ps object spread

…rops>` to `React.Children.map`

kibanamachine · 2023-12-19T18:07:34Z

Preview staging links for this PR:

Docs site: https://eui.elastic.co/pr_7340/
Storybook: https://eui.elastic.co/pr_7340/storybook

elasticmachine · 2023-12-19T18:07:37Z

💚 Build Succeeded

Buildkite Build
Commit: 9c03df9

History

💚 Build #1177 succeeded b58dc38
💚 Build #988 succeeded d044f05
💚 Build #981 succeeded ed236bc

cc @tkajtoch

cee-chen

Changes look great - thanks so much for answering my questions!

`v91.3.1`⏩`v92.0.0-backport.0` --- ## [`v92.0.0-backport.0`](https://github.com/elastic/eui/releases/v92.0.0-backport.0) **This is a backport release only intended for use by Kibana.** **Bug fixes** - Fixed an `EuiTreeView` JSX Typescript error ([#7452](elastic/eui#7452)) - Fixed a color console warning being generated by disabled `EuiStep`s ([#7454](elastic/eui#7454)) ## [`v92.0.0`](https://github.com/elastic/eui/releases/v92.0.0) - Updated generic types of `EuiBasicTable`, `EuiInMemoryTable` and `EuiSearchBar.Query.execute` to add `extends object` constraint ([#7340](elastic/eui#7340)) - This change should have no impact on your applications since the updated types only affect properties that exclusively accept object values. - Added a new `EuiFlyoutResizable` component ([#7439](elastic/eui#7439)) - Updated `EuiTextArea` to accept `isClearable` and `icon` as props ([#7449](elastic/eui#7449)) **Bug fixes** - `EuiRange`/`EuiDualRange`'s track ticks & highlights now update their positions on resize ([#7442](elastic/eui#7442)) **Deprecations** - Updated `EuiFilterButton` to remove the second `.euiFilterButton__textShift` span wrapper. Target `.euiFilterButton__text` instead ([#7444](elastic/eui#7444)) **Breaking changes** - Removed deprecated `EuiNotificationEvent`. We recommend copying the component to your application if necessary ([#7434](elastic/eui#7434)) - Removed deprecated `EuiControlBar`. We recommend using `EuiBottomBar` instead ([#7435](elastic/eui#7435))

`v91.3.1`⏩`v92.0.0-backport.0` --- ## [`v92.0.0-backport.0`](https://github.com/elastic/eui/releases/v92.0.0-backport.0) **This is a backport release only intended for use by Kibana.** **Bug fixes** - Fixed an `EuiTreeView` JSX Typescript error ([elastic#7452](elastic/eui#7452)) - Fixed a color console warning being generated by disabled `EuiStep`s ([elastic#7454](elastic/eui#7454)) ## [`v92.0.0`](https://github.com/elastic/eui/releases/v92.0.0) - Updated generic types of `EuiBasicTable`, `EuiInMemoryTable` and `EuiSearchBar.Query.execute` to add `extends object` constraint ([elastic#7340](elastic/eui#7340)) - This change should have no impact on your applications since the updated types only affect properties that exclusively accept object values. - Added a new `EuiFlyoutResizable` component ([elastic#7439](elastic/eui#7439)) - Updated `EuiTextArea` to accept `isClearable` and `icon` as props ([elastic#7449](elastic/eui#7449)) **Bug fixes** - `EuiRange`/`EuiDualRange`'s track ticks & highlights now update their positions on resize ([elastic#7442](elastic/eui#7442)) **Deprecations** - Updated `EuiFilterButton` to remove the second `.euiFilterButton__textShift` span wrapper. Target `.euiFilterButton__text` instead ([elastic#7444](elastic/eui#7444)) **Breaking changes** - Removed deprecated `EuiNotificationEvent`. We recommend copying the component to your application if necessary ([elastic#7434](elastic/eui#7434)) - Removed deprecated `EuiControlBar`. We recommend using `EuiBottomBar` instead ([elastic#7435](elastic/eui#7435))

tkajtoch self-assigned this Nov 3, 2023

tkajtoch requested a review from a team as a code owner November 3, 2023 16:14

tkajtoch force-pushed the feat/typescript-5-prep branch from ed236bc to d044f05 Compare November 3, 2023 22:48

This was referenced Nov 6, 2023

Upgrade typescript to latest v5 #7344

Open

Typescript 5 #6981

Open

cee-chen reviewed Nov 6, 2023

View reviewed changes

cee-chen approved these changes Nov 6, 2023

View reviewed changes

tkajtoch force-pushed the feat/typescript-5-prep branch from d044f05 to b58dc38 Compare December 19, 2023 17:08

tkajtoch added 15 commits December 19, 2023 18:38

fix: explicitly convert possible symbols to strings

08ce906

fix: use generics to type Object.freeze return value to fix `implic…

bf4ca03

…it any` errors

fix: move property initializers using this.context to constructor()

1215e91

fix: add extends object constraints to generics that required it

ef8f0f8

fix: check type of onFocusBadge instead of strictly comparing with …

1136a6f

…`false` to fix no type overlap typescript error

fix: ensure currentHeight is not 'full' before comparing numbers

35f7513

fix: remove optional string type from the minimumContrast property

a47b6fd

fix: replace unknown type with object type in page_bottom_bar pro…

8fae823

…ps object spread

fix: cast element props type instead of passing `ReactElement<CommonP…

bd0e1d6

…rops>` to `React.Children.map`

fix: fix renderItem type resolution

dfe73d8

chore: add changelog

94c8649

chore: add changelog

ff11d71

style: optimize conditionals

621a97e

style: use toString instead of String()

4dbc01f

fix(EuiBasicTable): add extends object generics in missing places

9c03df9

tkajtoch force-pushed the feat/typescript-5-prep branch from b58dc38 to 9c03df9 Compare December 19, 2023 17:40

cee-chen approved these changes Dec 20, 2023

View reviewed changes

tkajtoch merged commit a1b5168 into main Dec 21, 2023
7 checks passed

tkajtoch deleted the feat/typescript-5-prep branch December 21, 2023 13:06

cee-chen mentioned this pull request Jan 8, 2024

Upgrade EUI to v92.0.0 elastic/kibana#174487

Merged

cee-chen mentioned this pull request Jan 9, 2024

[EuiTreeView] JSX Typescript error #7452

Merged

1 task

This was referenced Aug 25, 2024

Update typescript peer dependency version string to accept version 5.x #7978

Closed

Accept typescript version 5.x as peer dependency #7980

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: TypeScript v5 prep work #7340

feat: TypeScript v5 prep work #7340

tkajtoch commented Nov 3, 2023 •

edited

Loading

cee-chen left a comment

cee-chen Nov 6, 2023 •

edited

Loading

tkajtoch Dec 19, 2023

cee-chen Dec 19, 2023

cee-chen left a comment •

edited

Loading

cee-chen Nov 6, 2023

tkajtoch Dec 19, 2023

kibanamachine commented Dec 19, 2023

elasticmachine commented Dec 19, 2023

cee-chen left a comment

		- Updated generic types of `EuiBasicTable`, `EuiInMemoryTable`
		and `EuiSearchBar.Query.execute` to add `extends object` constraint

feat: TypeScript v5 prep work #7340

feat: TypeScript v5 prep work #7340

Conversation

tkajtoch commented Nov 3, 2023 • edited Loading

Summary

QA

General checklist

cee-chen left a comment

Choose a reason for hiding this comment

cee-chen Nov 6, 2023 • edited Loading

Choose a reason for hiding this comment

tkajtoch Dec 19, 2023

Choose a reason for hiding this comment

cee-chen Dec 19, 2023

Choose a reason for hiding this comment

cee-chen left a comment • edited Loading

Choose a reason for hiding this comment

cee-chen Nov 6, 2023

Choose a reason for hiding this comment

tkajtoch Dec 19, 2023

Choose a reason for hiding this comment

kibanamachine commented Dec 19, 2023

elasticmachine commented Dec 19, 2023

💚 Build Succeeded

History

cee-chen left a comment

Choose a reason for hiding this comment

tkajtoch commented Nov 3, 2023 •

edited

Loading

cee-chen Nov 6, 2023 •

edited

Loading

cee-chen left a comment •

edited

Loading