-
-
Notifications
You must be signed in to change notification settings - Fork 725
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
fix(react-router): navigate if redirect is thrown in component #3107
base: main
Are you sure you want to change the base?
Conversation
View your CI Pipeline Execution ↗ for commit b0ae919.
☁️ Nx Cloud last updated this comment at |
b9c2da2
to
fb85ed4
Compare
5d0ea08
to
1a4d32f
Compare
1a4d32f
to
adf8a33
Compare
_fromLocation: router.state.location, | ||
}) | ||
return ( | ||
<Navigate {...redirect} replace={true} ignoreBlocker={true} /> |
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.
What's the rationale for setting replace=true
here? Is this something that's required to make it work?
Or is it purely for allowing the user to go from 1
to -1
in a single back button click when considering the user being at position 1
in [ -1 , 0 , 1 ]
?
Asking, because if this is not something required on our side to make it functional at runtime, then perhaps, we shouldn't be enforcing replace=true
on our side forcing a history replacement. Maybe this should be something the user should opt-in to?
@@ -252,6 +252,84 @@ describe('redirect', () => { | |||
expect(await screen.findByText('Final')).toBeInTheDocument() | |||
expect(window.location.pathname).toBe('/final') | |||
}) | |||
|
|||
test('when `redirect` is thrown in route component', async () => { |
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 think we should test the back button behaviour here since it'd likely be one of the few buttons that'd be front of mind for the end-user to consider immediately hitting next.
No description provided.