feat: upgrade react router to v6 #542
Conversation
|
Hi @Syed-Ali-Abbas-Zaidi, Just to confirm. Are you still working on this PR? We are planning the board cleanup. Should I close this PR? |
Hi, This PR is currently blocked due to this issue in react-router v6. This issue was resolved last week but it will be released in a week or two and after that, we will start working on this PR. |
|
@Syed-Ali-Abbas-Zaidi We are doing some optimization work and decomposition of tightly coupled components to prevent unwanted re-rendering. This PR will be affected by that work. We need to add a little bit of delay for this upgrade to prevent rework. what is your thought on this? |
|
@Syed-Ali-Abbas-Zaidi You can start working on this PR. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #542 +/- ##
==========================================
- Coverage 92.42% 92.35% -0.07%
==========================================
Files 169 169
Lines 3446 3468 +22
Branches 897 900 +3
==========================================
+ Hits 3185 3203 +18
- Misses 241 245 +4
Partials 20 20 ☔ View full report in Codecov by Sentry. |
Syed-Ali-Abbas-Zaidi
force-pushed
the
Ali-Abbas/react-router-upgrade
branch
4 times, most recently
from
7865d7b
to
a0f2d5d
Compare
Syed-Ali-Abbas-Zaidi
force-pushed
the
Ali-Abbas/react-router-upgrade
branch
from
a0f2d5d
to
bf76d80
Compare
There was a problem hiding this comment.
These routes display a blank content area in cases where the topic or category does not have any units.
These routes display a blank post content area when topic or category have units.
http://localhost:2002/course-v1:edX+DemoX+Demo_Course/topics/i4x-edx-eiorguegnru-course-foobarbaz/posts/
src/discussions/navigation/breadcrumb-menu/LegacyBreadcrumbMenu.test.jsx
Outdated
Show resolved
Hide resolved
Syed-Ali-Abbas-Zaidi
force-pushed
the
Ali-Abbas/react-router-upgrade
branch
from
e35400e
to
1996a10
Compare
There was a problem hiding this comment.
The route is crashing; it should redirect to All Posts for both legacy and new edX.
Fixed. 🤞 |
…into Ali-Abbas/react-router-upgrade
There was a problem hiding this comment.
Some routes are breaking and unexpected behavior. I have listed down all the router bugs and expected behavior in this doc.
…into Ali-Abbas/react-router-upgrade
Syed-Ali-Abbas-Zaidi
force-pushed
the
Ali-Abbas/react-router-upgrade
branch
from
69e3830
to
004360e
Compare
I have fixed the issues, and as per our discussion all routes that are not supported by this MFE will redirect to |
* feat: upgrade react router to v6 * fix: routing issues * fix: category route should redirect to all posts * fix: path error on routes
* feat: upgrade react router to v6 (openedx#542) * feat: upgrade react router to v6 * fix: routing issues * fix: category route should redirect to all posts * fix: path error on routes * perf: add support of css-variables to quince --------- Co-authored-by: Syed Ali Abbas Zaidi <88369802+Syed-Ali-Abbas-Zaidi@users.noreply.github.com> Co-authored-by: Diana Catalina Olarte <diana.olarte@edunext.co>
Ticket
React Router Upgrade to v6.
Description
This PR upgrades React Router from
v5tov6.