-
Notifications
You must be signed in to change notification settings - Fork 814
React 18 support #1458
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 support #1458
Conversation
…React 18 by default
This reverts commit 109ee01.
✅ Deploy Preview for evergreen-storybook ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
Drafting this - until we update the way we're rendering rendering the Tooltip w/ the new recommended |
| test('<Text /> has undefined behavior when trying to set arbitrary sizes', () => { | ||
| render(<Text size={800} />) | ||
| expect(mockFn.mock.calls.length).toEqual(1) | ||
| expect(mockFn.mock.calls.length).toBeGreaterThanOrEqual(1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The warning from ReactDOM.render added an additional call here 🤦
…rgreen into react-18-support
Overview
Resolves #1453
I wanted to move the two
@typespackages that are in our production dependency list, but I'm honestly not sure how to test to see if they are required for ourindex.d.tsyet (i.e. if anything breaks for an end-user if we don't ship with those types). I imagine if those types are needed because we're referencing them in some of our own type definitions, we could specify@types/reactas a peer dependency instead, but that might be a problem for another day.I pulled in a local version of the package in
appand it seems to run fine (no console warning since we're on an older version of React).In a React v18 app, it will still produce the warning for now.
Screenshots (if applicable)
Documentation