-
Notifications
You must be signed in to change notification settings - Fork 296
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
Conversation
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, |
There was a problem hiding this comment.
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'>> { |
There was a problem hiding this comment.
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
grafana-plugin/.eslintrc.js
Outdated
@@ -8,6 +8,10 @@ module.exports = { | |||
'import/internal-regex': | |||
'^assets|^components|^containers|^contexts|^icons|^models|^network|^pages|^services|^state|^utils|^plugin', | |||
}, | |||
parserOptions: { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
grafana-plugin/src/pages/escalation-chains/EscalationChains.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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)
# What this PR does - Migrate react-router from v5 to v6 Closes #4031
What this PR does
Closes #4031