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

Migrate to new react (18) #209

Closed
blcham opened this issue Jun 21, 2023 · 6 comments
Closed

Migrate to new react (18) #209

blcham opened this issue Jun 21, 2023 · 6 comments
Assignees

Comments

@blcham
Copy link
Collaborator

blcham commented Jun 21, 2023

We need to migrate to the newest react.

Consider and write info here about:

  • using Typescript when changing something
  • writing some info useful for upgrade of record-manager
  • using new features/best practices of react (i.e. design what to rewrite now or later)

A/C:

  • version of react is changed to 18
  • all github security updates are merged
  • it is tested manually
  • new minor version is released
@blcham
Copy link
Collaborator Author

blcham commented Jun 21, 2023

Btw, testing framework enzyme is not supported in react 18.

@VojtechLunak VojtechLunak self-assigned this Jun 24, 2023
@VojtechLunak
Copy link
Collaborator

VojtechLunak commented Jun 24, 2023

Summary

With React and React DOM upgraded to version 18.2.0 and other packages which were possible to upgrade to correspond to React 18.2.0 as well, only two packages have conflicts:

  1. @luigiminardim/[email protected]
    a. Have no other option compatible with React 18
  2. @wojtekmaj/[email protected]
    a. There exists enzyme adapter for react 18 (repo), and with first tests, it seems to work.

After migrating storybook to latest version (7.0.23), which brings some significant changes (binaries not used, instead used storybook cli), the stories for Answer do not work, because the intl field is undefined (probably because of not using the library 1. in the list above).

Incompatibilities

With React and React DOM upgraded to v 18.2.0, these warnings occur:


After upgrading @storybook dependencies to latest versions (7.0.23) and react-datepicker, react-intl, etc. to their latest versions, only these errors remain:

  • @luigiminardim/[email protected]
    • this peer dependency is only optional, so it might be possible to override without side effects
    • I personally do not know the reason this is used, need to consult it later
  • @wojtekmaj/[email protected]
    • there exists new adapter for React 18, but it is not as popular (repo)

Migration of storybook

docs
The base scripts changed:

  • start-storybook -> sb dev
  • build-storybook -> sb build

These scripts are from new dependency - @storybook/cli.
To migrate the main.cjs script to new, I had to use another new dependencies: @storybook/react-webpack5 and @storybook/builder-webpack5.

VojtechLunak added a commit that referenced this issue Jun 26, 2023
…torybook, react component libs, testing libs,...) synchronised. Work in progress. Library for global storybook controls has React 18 as optional peer dependency, need to examine closer.
@blcham
Copy link
Collaborator Author

blcham commented Jun 30, 2023

Current state:

  • enzyme is upgraded.

Next:

Fix NPM netlify strorybook build.

@VojtechLunak
Copy link
Collaborator

VojtechLunak commented Jul 3, 2023

Enzyme

After further investigation (1)(2) into Enzyme and React 17 / React 18 enzyme adapter, I think the tests will need migration from using Enzyme into another library (for example react-testing-library). In the current state 11 tests (only from Answer.test.js) do not pass because of the unofficial adapter for React 18, which does not work exactly as adapter for React 17.

Storybook & global controls

After further investigation, it seems the storybook stories would benefit from refactoring them into the newest scenario format. The reason is more efficient decorator application (Applying react context providers to components).

As [email protected] is not compatible with Storybook 7, I looked into handling globals and it seems that storybook has its own built-in globals and toolbars mechanism.

Netlify build fix

With Storybook upgraded to version 7, it is dependant on version of Node to be higher than 16. Netlify should use Node 18 by default, but if I understand correctly, it had some cached Node 12. To force Netlify to use Node 18, I added .node-version file to this repository and then the build succeded.

@blcham
Copy link
Collaborator Author

blcham commented Jul 3, 2023

@VojtechLunak great job so far. I agree with storybook ideas and netlify fix. W.r.t. enzyme i cannot judge what is most appropriate library switch so i guess it is fine as well.

VojtechLunak added a commit that referenced this issue Jul 3, 2023
…rybook. Decorators might need further work to make it simple and unified
@VojtechLunak
Copy link
Collaborator

Storybook scenario sections refactor.
Stories (Sforms) vs Components.

@blcham blcham closed this as completed in 73c4d70 Jul 12, 2023
blcham added a commit that referenced this issue Jul 12, 2023
[#209] React and React DOM upgraded to v18
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

No branches or pull requests

2 participants