Bump react-router #52445
Bump react-router #52445
Conversation
patrykkopycinski
added
chore
release_note:skip
patrykkopycinski
added
the
Team:Infra Monitoring UI - DEPRECATED
|
Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui) |
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
patrykkopycinski
added
the
Team:Kibana Management
|
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
|
|
||
| constructor(...args) { | ||
| super(...args); | ||
| constructor(props) { |
There was a problem hiding this comment.
Does this need to be changed? If possible, I'd prefer to leave this alone to reduce the amount of changes in this PR and lessen the scope of code review.
There was a problem hiding this comment.
sure, reverted 👍
| } | ||
| const ShareRouterComponent = React.memo(({ children, history }) => { | ||
| useEffect(() => { | ||
| registerRouter({ history }); |
There was a problem hiding this comment.
When I navigate to Management > Rollup Jobs there's a Console error that indicates that registerRouter never gets called here. Perhaps we don't need to use the hook?
const ShareRouterComponent = React.memo(({ children, history }) => {
registerRouter({ history });
return children;
});I tried this and it seems to work.
There was a problem hiding this comment.
that would cause to call registerRouter on each re-render, changed back to the Component
| super(...args); | ||
| this.registerRouter(); | ||
| } | ||
| const ShareRouterComponent = React.memo(({ children, history }) => { |
There was a problem hiding this comment.
CC @jloleysens Just giving you a head's up about the changes being made to this file because I know you've been working on it too. Any chance there might be merge conflicts?
There was a problem hiding this comment.
I think there will be, but they should be minor :)
There was a problem hiding this comment.
for sake of consistancy changed it back to the Component
There was a problem hiding this comment.
Thank you @cjcenizal @jloleysens for the review and comments, I hope I have fixed all of them. Can I ask you for another check?
…outer-5 # Conflicts: # x-pack/legacy/plugins/watcher/public/np_ready/application/app.tsx
|
@elasticmachine merge upstream |
|
That flaky failure should be fixed by #52601 |
|
@elasticmachine merge upstream |
|
Pinging @elastic/kibana-stack-services (Team:Stack Services) |
|
@elasticmachine merge upstream |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
This only contains a few commits but was not a straightforward merge. I initially got a warning about too many renames and suggesting I increase `merge.renameLimit` There were also ~100 files with merge conflicts. In general, I accepted incoming changes but I did go through each file and try to make sure what was being staged was desired. I started by clearing the conficts in both `package.json`s (new react-router version) then ran `yarn kbn bootstrap` and went back to merging. There were some files like i18n.json and security where accepting incoming would have erased a fleet/ingest/epm setting so I chose both there. There were aslso some *really* weird merge conflict markers in some files. Things I've not seent before. Roughly: ``` <<<<<<< HEAD path/to/file/we/are/viewing code ======= entirely different code >>>>>>> 802423d:path/to/VERY/different/place ``` In those cases, I opened the file on master in github to check the correct code and either took the code between the markers or copied/pasted from GitHub. Many of them were from this specific SHA. I also updated these files based on the changes in elastic#52445 x-pack/legacy/plugins/fleet/public/components/navigation/connected_link.tsx x-pack/legacy/plugins/fleet/public/components/navigation/connected_link.tsx x-pack/legacy/plugins/fleet/public/hooks/with_url_state.tsx
Summary
Bump
react-router-domto the newest versionChecklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.For maintainers