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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

deps: Update to support React 18 #1483

Merged
merged 9 commits into from
May 2, 2022
Merged

deps: Update to support React 18 #1483

merged 9 commits into from
May 2, 2022

Conversation

Gaya
Copy link
Contributor

@Gaya Gaya commented Apr 28, 2022 •
edited
Loading

馃挜 Breaking Changes

  • Allows React 18 and updates visx-demo's render methods

馃殌 Enhancements

  • Allows the use of React 18

See: #1473

@Gaya Gaya changed the title Update react 18 Update to React 18 Apr 29, 2022
@Gaya Gaya marked this pull request as ready for review April 29, 2022 11:24
williaster
williaster reviewed Apr 29, 2022
View reviewed changes
Copy link
Collaborator

@williaster williaster left a comment

Choose a reason for hiding this comment

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

Overall this looks great to me! Thanks so much for going through the updates. I'm just going to pull it locally and play with it before approving 馃憤

packages/visx-demo/src/sandboxes/template/index.tsx
<ParentSize>{({ width, height }) => <Example width={width} height={height} />}</ParentSize>,
document.getElementById('root'),
);
const root = createRoot(document.getElementById('root')!);
Copy link
Collaborator

Choose a reason for hiding this comment

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

馃憤

packages/visx-text/Readme.md Show resolved Hide resolved
@williaster
Copy link
Collaborator

Ah looks like CI is failing due to prettier formatting in all the sandbox updates. you should be able to auto-fix this by running yarn format (or yarn run prettier --write) in the root directory.

@williaster williaster changed the title Update to React 18 deps: Update to support React 18 Apr 29, 2022
Gaya and others added 2 commits May 2, 2022 07:45
@Gaya @williaster
Add react@18 note to packages/visx-text/Readme.md
38ce48a 
Co-authored-by: Chris Williams <williaster@users.noreply.github.com>
@Gaya
style: fix sandbox code style
e6d1c32
@Gaya
Copy link
Contributor Author

Gaya commented May 2, 2022

@williaster thanks for taking a look. Fixed the problems you addressed. Though the README.md in root also got updated when I ran yarn format, I didn't commit this file as I think it had nothing to do with the other updates I did. Don't know if the CI will fail because of that, but the demo code has been fixed.

Let me know if you need something else.

williaster
williaster approved these changes May 2, 2022
View reviewed changes
Copy link
Collaborator

@williaster williaster left a comment

Choose a reason for hiding this comment

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

so great! thanks @Gaya 鉂わ笍

this looks good to me, I'm going to release this as a minor bump but keep it as an alpha to start. then if that looks good published we can do a non-alpha. (releases will be posted to this PR)

@williaster williaster merged commit 882ffc3 into airbnb:master May 2, 2022
@Gaya
Copy link
Contributor Author

Gaya commented May 2, 2022

Awesome! Glad to help 馃檹

@github-actions
Copy link

github-actions bot commented May 2, 2022

馃帀 This PR is included in version v2.10.0-alpha.0 of the packages modified 馃帀

@github-actions
Copy link

github-actions bot commented May 3, 2022

馃帀 This PR is included in version v2.10.0 of the packages modified 馃帀

@hshoff hshoff mentioned this pull request May 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@williaster williaster williaster approved these changes

Assignees
No one assigned
Labels
alpha-release 馃殌enhancement minor-release @visx/visx
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Gaya @williaster