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

fix: react component type errors #2440

Merged
merged 2 commits into from
May 28, 2024

Conversation

nickofthyme
Copy link
Collaborator

@nickofthyme nickofthyme commented May 21, 2024

Summary

Fixes type errors related to react types.

Details

I think the root issue here is that react now has a peerDependency on @types/react as * which would be fine if only one react was used in the project. But we are using react@18 in docs/ and react@16 everywhere else and typescript does not like this dual usage.

Since we use yarn workspaces, the react types are hoisted to the top. This fix adds the explicit dependencies of @types/react and @types/react-dom in packages/charts/package.json. Additionally, VSCode will still not like this unless there is a local tsconfig.json file at the base of the packages/charts/ directory, which affectively points to the local packages/charts/node_modules/ before the top node_modules/.

This is a temporary fix, I will clean up the react types when upgrading to react@18.

Issues

fixes #2398

@nickofthyme nickofthyme added dependencies Pull requests that update a dependency file :build Build tools / dependencies labels May 21, 2024
package.json Show resolved Hide resolved
@nickofthyme nickofthyme requested a review from markov00 May 22, 2024 14:15
Copy link
Contributor

@mbondyra mbondyra left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for taking care of it 🥇

Copy link
Member

@markov00 markov00 left a comment

Choose a reason for hiding this comment

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

Let's merge it like that but I don't like this solution so much because now the API docs is a mix of 2 defined React versions.
We can definitely find a solution when we are going to move to react 18 everywhere

@nickofthyme nickofthyme merged commit f0b3a00 into elastic:main May 28, 2024
14 checks passed
@nickofthyme nickofthyme deleted the fix-react-errors branch May 28, 2024 16:04
nickofthyme pushed a commit that referenced this pull request May 28, 2024
# [65.1.0](v65.0.0...v65.1.0) (2024-05-28)

### Bug Fixes

* **deps:** update dependency @elastic/eui to ^94.5.0 ([#2433](#2433)) ([b13ded9](b13ded9))
* **deps:** update dependency @playwright/test to ^1.44.0 ([#2434](#2434)) ([faf36aa](faf36aa))
* **Metric:** should only show one focus halo on `::focus` event ([#2441](#2441)) ([96b0779](96b0779))
* react component type errors ([#2440](#2440)) ([f0b3a00](f0b3a00))

### Features

* **legend:** add legend stats (table view) ([#2426](#2426)) ([c22f767](c22f767))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:build Build tools / dependencies dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

React type errors rendering ComponentType
3 participants