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

Update to React 18.2 #99

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

AllTerrainDeveloper
Copy link
Contributor

This change just updates react to the latest version 18.2

Some manual changes had to be done, but just import PropTypes from a different place rather than from the react namespace.

Testing instructions:

  • Test that the whole application keeps working

Copy link
Member

@dmsnell dmsnell left a comment

Choose a reason for hiding this comment

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

After spending some time trying to figure out a way to work around them I think updating to React 18 introduces a number of bugs that will be best dealt with by updating our application code before updating to React 18. For example, when interacting with the JSON tree after updating we get console errors. Also this doesn't move us into React 18 mode because we're not using createRoot() from react-dom/client (and it's not trivial to switch because of the other code issues).

How would you feel about limiting this PR to the PropType updates and then start tackling the other issues that hold us back from jumping into React 18?

  • componentWillReceiveProps
  • componentWillUpdate
  • whatever is causing JSONTree crashes
  • createRoot()
  • whatever other issues appear in testing

I had a commit for the lifecycle method updates but I backed it out; preferring instead to handle those in separate PRs where we can isolate them.

Some of our other dependencies are much older and I think we would have a smoother path updating React if we could update those deps first. The redux + react-redux + redux-thunk update seemed to go well when I tried, but react-json-tree didn't. Also I'd love to continue removing custom functionality from our build script so we can eject from react-scripts without needing to maintain any configuration.

@AllTerrainDeveloper
Copy link
Contributor Author

Hello! We are a little bit out of time and I did not have time to address your suggestions. I'll comeback to this PR anytime soon :)

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.

2 participants