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

feat(deps): update to react 18 #1049

Merged
merged 19 commits into from
Jul 2, 2024
Merged

Conversation

Nick87
Copy link
Contributor

@Nick87 Nick87 commented Apr 9, 2024

Fixes #963

PR Checklist

  • My branch is up-to-date with the Upstream main branch.
  • The unit tests pass locally with my changes (if applicable).
  • I have added tests that prove my fix is effective or that my feature works (if applicable).
  • I have added necessary documentation (if appropriate).

Short description of what this resolves:

Upgrade dependencies, react included

Copy link

vercel bot commented Apr 9, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
design-react-kit ✅ Ready (Inspect) Visit Preview Jul 2, 2024 10:20am

@Nick87
Copy link
Contributor Author

Nick87 commented Apr 9, 2024

npm i and yarn build complete successfully.

N.B. I had to override react-stickup react dependency due to conflicting peer dependencies: it requires react >=16.3.0 <18.0.0 and it is already at its latest version available.

This is the npm run test result (same as the main branch):

image

@sabato-galasso
Copy link
Collaborator

npm i and yarn build complete successfully.

N.B. I had to override react-stickup react dependency due to conflicting peer dependencies: it requires react >=16.3.0 <18.0.0 and it is already at its latest version available.

This is the npm run test result (same as the main branch):

Can you give me the repo permission, I'll help you if I can? thanks

@Nick87
Copy link
Contributor Author

Nick87 commented Apr 11, 2024

npm i and yarn build complete successfully.
N.B. I had to override react-stickup react dependency due to conflicting peer dependencies: it requires react >=16.3.0 <18.0.0 and it is already at its latest version available.
This is the npm run test result (same as the main branch):

Can you give me the repo permission, I'll help you if I can? thanks

i should have given you access to the repo

@Nick87
Copy link
Contributor Author

Nick87 commented Apr 11, 2024

It's a little better now: I have a test failing due to a non-matching snapshot

image

@sabato-galasso
Copy link
Collaborator

With snap updated
Screenshot 2024-04-12 alle 09 54 03

@Virtute90
Copy link
Collaborator

To upgrade to React 18 you also need to check the deprecated types, one above all ReactChild:

@types/react/index.d.ts

I believe there is a babel plugin that checks all the code.

@Nick87
Copy link
Contributor Author

Nick87 commented Apr 14, 2024

@sabato-galasso please be advised, i'm rebasing my branch to rename some commit messages since i'm getting commitlint errors due to missing subject/type in the previous commit messages that cause an automatic check to fail immediately

This reverts commit 2bc3bbd, reversing
changes made to 7578576.
@sabato-galasso
Copy link
Collaborator

@Nick87 I did a wrong merge, can you revert the last commit? I don't have permission. Thank you

@Nick87
Copy link
Contributor Author

Nick87 commented Jun 5, 2024

is this PR still meaningful? i was considering closing it since it seems to be going nowhere imho :)

@Nick87 Nick87 force-pushed the feature/deps-bump-963 branch from 2bc3bbd to 7578576 Compare June 5, 2024 12:03
@Virtute90
Copy link
Collaborator

is this PR still meaningful? i was considering closing it since it seems to be going nowhere imho :)

It hasn't been merged yet :(

@sabato-galasso
Copy link
Collaborator

is this PR still meaningful? i was considering closing it since it seems to be going nowhere imho :)

you can try to rebase and resolve conflicts, let's see if we can merge it

@astagi astagi linked an issue Jun 24, 2024 that may be closed by this pull request
@astagi astagi requested a review from Lorezz June 24, 2024 14:59
@astagi
Copy link
Member

astagi commented Jun 24, 2024

@Lorezz and me are going to resolve conflicts and review this PR on Wed 26.

@astagi
Copy link
Member

astagi commented Jun 25, 2024

@sabato-galasso @Virtute90 @Nick87 I tried to realign this branch with main I have some problems with tests dependencies.. @Virtute90 changed the build system maybe we need something else to make tests working again.

cc @Lorezz

@Virtute90
Copy link
Collaborator

@sabato-galasso @Virtute90 @Nick87 I tried to realign this branch with main I have some problems with tests dependencies.. @Virtute90 changed the build system maybe we need something else to make tests working again.

cc @Lorezz

I looked at the branch of @Nick87, you need to do these operations:

and the tests should work correctly

@astagi
Copy link
Member

astagi commented Jun 26, 2024

@Lorezz just updated tests and dependencies, we're testing an unstable build before merging this PR.

cc @sabato-galasso @Nick87 @Virtute90

@astagi astagi merged commit ac0b4d5 into italia:main Jul 2, 2024
3 of 5 checks passed
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.

Fix: remove defaultProps on Alert [FEATURE] Upgrade to React 18
5 participants