[Deps]: Upgraded react-router-dom #2565
Conversation
yashpandey06
force-pushed
the
upgrade-react-router-dom
branch
3 times, most recently
from
010b43e to
b863fe5
Compare
| </Switch> | ||
| </Page> | ||
| </Router> | ||
| <Page> |
There was a problem hiding this comment.
previously there was a history attribute
<Router history={history}>
what's the implication of removing it?
Disclaimer: I am not a React developer, don't know how any of this works.
There was a problem hiding this comment.
It is handled internally need v6+ version
There was a problem hiding this comment.
I see you removed the screenshot of the docs, please add a link to those docs instead
yashpandey06
force-pushed
the
upgrade-react-router-dom
branch
from
b863fe5 to
23f3396
Compare
|
|
||
| import '../common/vars.css'; | ||
| import '../common/utils.css'; | ||
| import 'antd/dist/reset.css'; | ||
| import './index.css'; | ||
| import { history, store } from '../../utils/configure-store'; | ||
| import { HistoryProvider } from '../../utils/useHistory'; |
There was a problem hiding this comment.
Use ../../utils/useHistory used anywhere else? Should it be deleted?
There was a problem hiding this comment.
@yurishkuro yeah i believe ....this is significant to notice that we can even replace useHistory hook and withProps component functionality with useNavigate .
There was a problem hiding this comment.
not sure it answers my question - since this is the primary navigation piece of the app and you're no longer importing useHistory module, should this PR not also include the deletion of said module?
| @@ -77,8 +77,7 @@ | |||
| "react-is": "^18.2.0", | |||
| "react-json-view-lite": "2.0.1", | |||
| "react-redux": "^8.1.2", | |||
| "react-router-dom": "5.3.4", | |||
| "react-router-dom-v5-compat": "^6.24.0", | |||
| "react-router-dom": "7.1.1", | |||
There was a problem hiding this comment.
I expect this needs changed to package-lock
Signed-off-by: YASH PANDEY <[email protected]>
yashpandey06
force-pushed
the
upgrade-react-router-dom
branch
from
23f3396 to
d073538
Compare
| @@ -20115,13 +20019,12 @@ | |||
| "react-icons": "^5.0.1", | |||
| "react-is": "^18.2.0", | |||
| "react-json-view-lite": "2.0.1", | |||
| "react-redux": "^9.2.0", | |||
There was a problem hiding this comment.
@hari45678 this looks like a left over from #2553?
There was a problem hiding this comment.
yes actually. i hand picked the changes from original PR. This got left out.
| @@ -20160,7 +20063,7 @@ | |||
| "jest-junit": "^16.0.0", | |||
| "less": "4.2.0", | |||
| "react-test-renderer": "^18.3.1", | |||
| "rollup-plugin-visualizer": "^5.13.1", | |||
| "rollup-plugin-visualizer": "^5.11.0", | |||
There was a problem hiding this comment.
any idea what's causing this downgrade?
| @@ -20489,7 +20438,7 @@ | |||
| "style-loader": "4.0.0", | |||
| "webpack": "^5.92.0", | |||
| "webpack-cli": "^5.1.4", | |||
| "webpack-dev-server": "^5.2.0", | |||
| "webpack-dev-server": "^5.0.4", | |||
|
Looking at the changes. Have you tested them locally? @yashpandey06 |
Which problem is this PR solving?
Description of the changes
Checklist
jaeger:make lint testjaeger-ui:npm run lintandnpm run test