italia / design-react-kit

Il toolkit React conforme alle linee guida di design per i siti internet e i servizi digitali della PA.
https://italia.github.io/design-react-kit/
BSD 3-Clause "New" or "Revised" License
152 stars 80 forks source link

feat(deps): update to react 18 #1049

Closed Nick87 closed 1 week ago

Nick87 commented 3 months ago

Fixes #963

PR Checklist

Short description of what this resolves:

Upgrade dependencies, react included

vercel[bot] commented 3 months ago

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 commented 3 months ago

npmi 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 commented 3 months ago

npmi 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 commented 2 months ago

npmi 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 commented 2 months ago

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

image

sabato-galasso commented 2 months ago

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

Virtute90 commented 2 months ago

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 commented 2 months ago

@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

Nick87 commented 2 months ago

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.

I have replaced ReactChild with ReactNode, and updated the isReactChild utility function in src\Notification\core.tsx. I can't find the babel plugin you were talking about: are there other deprecated types that need updating?

Virtute90 commented 2 months ago

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.

I have replaced ReactChild with ReactNode, and updated the isReactChild utility function in src\Notification\core.tsx. I can't find the babel plugin you were talking about: are there other deprecated types that need updating?

The plugin should be this: https://github.com/gund/eslint-plugin-deprecation

Nick87 commented 2 months ago

@Virtute90 I can't seem to find any other deprecated types being used in the codebase: I do find deprecated props, wrong assignment operators, wrong useCallback declarations and incomplete dependencies array, but that's another story for another PR maybe.

image

Virtute90 commented 2 months ago

To update react-select I believe that it is not enough to change the version in the package.json, this guide is shown on the site:

https://react-select.com/upgrade#from-v4-to-v5

I also suggest reviewing the component given the latest issues that have been added (#1041 #1035)

Nick87 commented 2 months ago

To update react-select I believe that it is not enough to change the version in the package.json, this guide is shown on the site:

https://react-select.com/upgrade#from-v4-to-v5

I also suggest reviewing the component given the latest issues that have been added (#1041 #1035)

I'll take a look at the react-select migration guide and at the Select component, but I do not think that the issued you mentioned (one of which was opened by me) have nothing to do with the react version: it was a matter a props that were not forwarded or incorrectly forwarded to the underlying input, regardless of the react version.

sabato-galasso commented 2 months ago

To update react-select I believe that it is not enough to change the version in the package.json, this guide is shown on the site: https://react-select.com/upgrade#from-v4-to-v5 I also suggest reviewing the component given the latest issues that have been added (#1041 #1035)

I'll take a look at the react-select migration guide and at the Select component, but I do not think that the issued you mentioned (one of which was opened by me) have nothing to do with the react version: it was a matter a props that were not forwarded or incorrectly forwarded to the underlying input, regardless of the react version.

we no longer use react-select it has been replaced with accessible-autocomplete, you can also remove it

Nick87 commented 2 months ago

I have removed react-select altogether. What do you think is this PR missing to call it complete?

sabato-galasso commented 2 months ago

I have removed react-select altogether. What do you think is this PR missing to call it complete?

there is no backward compatibility test with react 17, so install the kit in a project that uses react 17, checking that no components break.

Nick87 commented 2 months ago

I have removed react-select altogether. What do you think is this PR missing to call it complete?

there is no backward compatibility test with react 17, so install the kit in a project that uses react 17, checking that no components break.

This PR is not supposed to be backward-compatible with React 17, at least not officially, and you can tell from the peerDependencies:

image

I beliebe that this PR should cause a major version bump, hence having 6.X.X for react 18 and 5.X.X for react 17: after all, was not the goal of this pr to avoid becoming obsolete in the first place? :)

sabato-galasso commented 1 month ago

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

Nick87 commented 1 month ago

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

Virtute90 commented 1 month ago

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 commented 1 month ago

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 commented 2 weeks ago

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

astagi commented 2 weeks ago

@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 commented 2 weeks ago

@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 commented 2 weeks ago

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

cc @sabato-galasso @Nick87 @Virtute90