-
Notifications
You must be signed in to change notification settings - Fork 13.4k
feat(react-router): upgrade to react router 6 #30831
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
base: major-9.0
Are you sure you want to change the base?
Conversation
Co-authored-by: Sean Perkins <13732623+sean-perkins@users.noreply.github.com>
…ter 6 Co-authored-by: Sean Perkins <13732623+sean-perkins@users.noreply.github.com>
…ct router 6 Co-authored-by: Sean Perkins <13732623+sean-perkins@users.noreply.github.com>
… router 6 Co-authored-by: Sean Perkins <13732623+sean-perkins@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
6e780d6 to
8a2e91a
Compare
8a2e91a to
99dcb35
Compare
4d401c1 to
c60a99d
Compare
c60a99d to
a565a37
Compare
|
Took you long enough...Good job shane |
| fail-fast: false | ||
| matrix: | ||
| apps: [reactrouter5] | ||
| apps: [reactrouter6] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should either update the breaking changes file to include no longer supporting react router 5 or create a follow-up task since doing so might conflict with next: https://github.com/ionic-team/ionic-framework/blob/next/BREAKING.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I still need to document migration strategies and such. For the most part, it's a pretty easy migration (ignore the inline overlay stuff, that was all removed), but I just need to write it out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went ahead and did this in this branch, figuring it would be an easy enough merge conflict resolution. If you'd like to review in an easier to read way:
https://github.com/ionic-team/ionic-framework/blob/b2a71050344052a4858d0c8b6712f0d5bc9d7219/BREAKING.md
Issue number: resolves #24177
What is the current behavior?
Currently, Ionic Framework React Router only supports React Router 5. This has many issues and unsupported/broken features.
What is the new behavior?
With this change, Ionic Framework will support React Router 6 while still supporting transitions in the same way a native app does.
Most of what caused this change to take a long time is that React Router 5 and React Router 6 have fundamental differences in how they handle components once they're no longer part of the view. In this change, we move away from relying on React Router directly so much and have our own implementation for deciding how views get dealt with during navigation and when they're cleaned up, allowing for us to still transition between them like we need to while still using React Router as much as we possibly can.
This change will also lay the foundation for the migration to React Router 7, which will ideally be easier since most of the hard work has been dealt with here.
Does this introduce a breaking change?
Other information
Current dev build: