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

Upgrade react-router-dom to latest version #1825

Open
1 of 7 tasks
anshgoyalevil opened this issue Sep 26, 2023 · 4 comments
Open
1 of 7 tasks

Upgrade react-router-dom to latest version #1825

anshgoyalevil opened this issue Sep 26, 2023 · 4 comments
Labels
changelog:dependencies Update to dependencies

Comments

@anshgoyalevil
Copy link
Contributor

anshgoyalevil commented Sep 26, 2023

This is a Tracker Issue for all react-router-dom related upgrades.

Useful resources:

@anshgoyalevil anshgoyalevil changed the title [Tracker]: Upgrade react-router-dom to latest version Upgrade react-router-dom to latest version Sep 26, 2023
@priyanshu-kun
Copy link
Contributor

rrdv6 has plenty of breaking changes compare to v4 that currently in use, I think we have to migrate it gradually.

@anshgoyalevil
Copy link
Contributor Author

Yes, we would have to upgrade it sequentially version by version, keeping the project intact

@priyanshu-kun
Copy link
Contributor

@anshgoyalevil do we have to migrate it to v5 before v6?

@anshgoyalevil
Copy link
Contributor Author

Yes. First v5, then 5.1, and then v6. (There can be more versions between 5.1 and 6, to which we would have to upgrade first).

Though it is a part of my LFX Mentorship, if you want to help out, you may DM me on Slack, and we can collaborate on another dependency upgrade, so that we both don't have to create the same type of PRs simultaneously.

yurishkuro pushed a commit that referenced this issue Sep 27, 2023
## Which problem is this PR solving?
- related to: #1825
- The `react-router-redux` is a dead project and is not compatible with
`react-router-dom` `v5` and `v6`.
- The whole react-router community suggested using `redux-first-history`
which is compatible with `v4`, `v5` and `v6` both.

## How was this change tested?
- Manually along with redux dev toolkit and automated testing

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [x] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

Signed-off-by: Ansh Goyal <anshgoyal1704@gmail.com>
yurishkuro pushed a commit that referenced this issue Sep 27, 2023
## Which problem is this PR solving?
- part of: #1825
- Upgrades react-router-dom to v5.2.0

## Description of the changes
- This PR upgrades the rrd to v5.2.0
- This upgrade was previously attempted with PR
#727 but at that time, an
issue (#803) was
reported because `react-router-redux` v5.x was not compatible with rrd
v5.2.0, so it was reverted with PR:
#837
- Now, since we have `redux-first-history` instead of
`react-router-redux`, we can upgrade to rrd v5.2.0 safely now.

## How was this change tested?
- The reported issue with v5.2.0
(#803) is not being
reproduced now.


![image](https://github.com/jaegertracing/jaeger-ui/assets/94157520/43c11ec6-02e6-4ede-855b-22822f77d4ae)


## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [x] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

Signed-off-by: Ansh Goyal <anshgoyal1704@gmail.com>
yurishkuro pushed a commit that referenced this issue Nov 11, 2023
…1970)

## Which problem is this PR solving?
- part of: #1825

## Description of the changes
- This PR adds the react-router compatibility package to merge v5 and v6
APIs according to the official migration guide:
remix-run/react-router#8753 and
https://reactrouter.com/en/6.16.0/upgrading/v5
- After this PR, we need to change `<Route>` to `<CompatRoute>` one at a
time, to minimize the impact area per patch.

## How was this change tested?
- `yarn start` + `yarn test`

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [x] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

Signed-off-by: Ansh Goyal <anshgoyal1704@gmail.com>
@yurishkuro yurishkuro added changelog:dependencies Update to dependencies and removed bug labels Nov 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:dependencies Update to dependencies
Projects
None yet
Development

No branches or pull requests

3 participants