Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[React 18] Fix remaining TypeScript errors #6988

Conversation

tkajtoch
Copy link
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 cee-chen and a team July 25, 2023 14:43
@tkajtoch tkajtoch self-assigned this Jul 25, 2023
@kibanamachine
Copy link

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

Copy link
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
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
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
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

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