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

Migrate react-router to v6 #4703

Merged
merged 38 commits into from
Jul 26, 2024
Merged

Migrate react-router to v6 #4703

merged 38 commits into from
Jul 26, 2024

Conversation

teodosii
Copy link
Member

@teodosii teodosii commented Jul 18, 2024

What this PR does

  • Migrate react-router from v5 to v6

Closes #4031

@teodosii teodosii added pr:no public docs Added to a PR that does not require public documentation updates release:patch PR will be added to "Other Changes" section of release notes labels Jul 19, 2024
@teodosii
Copy link
Member Author

Blocked until scenes migrates to react-router v6

@@ -199,7 +240,7 @@ const config = async (env): Promise<Configuration> => {

if (isWSL()) {
baseConfig.watchOptions = {
poll: 3000,
// poll: 3000,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on WSL this isn't really needed. I use WSL and this breaks reloading actually, doesn't seem to be configurable :(

router: Router<T>;
}

export function withRouter<X, T extends PropsWithRouter<X>>(Component: React.FC<T>): React.FC<Omit<T, 'router'>> {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Newest v6 react-router seems to no longer export a HOC for withRouter. Added this in here with all the necessary types

@@ -8,6 +8,10 @@ module.exports = {
'import/internal-regex':
'^assets|^components|^containers|^contexts|^icons|^models|^network|^pages|^services|^state|^utils|^plugin',
},
parserOptions: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it needed here since we have it in overrides already? Seems it's a duplication in this case?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say not as I undo-ed the other packages' updates. But I believe once you bump grafana/eslint, it is needed.

Copy link
Contributor

@brojd brojd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great job! let me just make sure tomorrow if this will work ok in the IRM monorepo setup (incident uses v5.3.3)

@brojd brojd self-requested a review July 26, 2024 10:17
@teodosii teodosii added this pull request to the merge queue Jul 26, 2024
Merged via the queue into dev with commit 0aa3b1d Jul 26, 2024
21 checks passed
@teodosii teodosii deleted the rares/react-router-v6 branch July 26, 2024 10:46
brojd pushed a commit that referenced this pull request Sep 18, 2024
# What this PR does

- Migrate react-router from v5 to v6

Closes #4031
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:no public docs Added to a PR that does not require public documentation updates release:patch PR will be added to "Other Changes" section of release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

App plugin: Update to react-router v6
2 participants