Skip to content

[React 18] Fix remaining TypeScript errors#6988

Merged
tkajtoch merged 17 commits intoelastic:feature/react-18from
tkajtoch:feat/react-18-update-remaining-typings
Jul 25, 2023
Merged

[React 18] Fix remaining TypeScript errors#6988
tkajtoch merged 17 commits intoelastic:feature/react-18from
tkajtoch:feat/react-18-update-remaining-typings

Conversation

@tkajtoch
Copy link
Copy Markdown
Member

Summary

This PR fixes types in places where they were previously any, unknown or mistakenly ignored by {} being a part of the ReactNode type in React 17 and below.

QA

  1. Checkout this branch
  2. Run yarn
  3. Run yarn tsc --noEmit and confirm there are no errors

@tkajtoch tkajtoch requested review from a team and cee-chen July 25, 2023 14:43
@tkajtoch tkajtoch self-assigned this Jul 25, 2023
@kibanamachine
Copy link
Copy Markdown

Preview documentation changes for this PR: https://eui.elastic.co/pr_6988/

Copy link
Copy Markdown
Contributor

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

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

The majority of these PR comments are just this meme, apologies 😅

Meme making fun of code reviews catching typos in comments and missing actual bugs

The only comment that's an actual cleanup change request is last 2 ones around basic table mobileOptions.render one.

By the way, when I pull down this branch and run yarn lint I'm still getting a bunch of TS failures around errors in our node_modules - is that something that's only happening in my local? Or is it something that we'll need to address in a separate PR?

Comment on lines +232 to +233
const setInputValidityRef = useCallback<
RefCallback<Component & { input: HTMLInputElement }>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice work figuring this one out - it looks like a huge pain 😅

) : (
icon
);
const iconRender = isValidElement(icon) ? (
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice catch/switch on this one as well!

@tkajtoch tkajtoch force-pushed the feat/react-18-update-remaining-typings branch from b664f36 to 729a23b Compare July 25, 2023 20:26
Copy link
Copy Markdown
Contributor

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

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

🎉 Changes look great - thanks for the super speed!!

@kibanamachine
Copy link
Copy Markdown

Preview documentation changes for this PR: https://eui.elastic.co/pr_6988/

@tkajtoch tkajtoch merged commit 2d13686 into elastic:feature/react-18 Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants