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

Revert version update of react-router-dom and history #3140

Merged
merged 1 commit into from Sep 2, 2021

Conversation

swaterkamp
Copy link
Member

@swaterkamp swaterkamp commented Sep 2, 2021

What:
Revert react-router-dom dependency version from 5.2.1 to 5.2.0
Revert history dependency version from 5.0.1 to 4.10.1

Why:
react-router-dom breaks our login page presumably due to a change of default behavior (remix-run/react-router#5362)
history breaks navigation almost completely

How:
Login and navigation is possible again

Checklist:

  • Tests
  • CHANGELOG Entry
  • Labels for ports to other branches

@swaterkamp swaterkamp self-assigned this Sep 2, 2021
@swaterkamp swaterkamp requested a review from a team as a code owner September 2, 2021 13:25
@swaterkamp swaterkamp force-pushed the swaterkamp/revertReactRouterDomVersion branch from 949ee42 to e0e1bda Compare September 2, 2021 13:34
@swaterkamp swaterkamp changed the title Revert version update of react-router-dom Revert version update of react-router-dom and history Sep 2, 2021
@swaterkamp swaterkamp merged commit bd8e552 into master Sep 2, 2021
@swaterkamp swaterkamp deleted the swaterkamp/revertReactRouterDomVersion branch September 2, 2021 14:35
@guidobouman
Copy link

Author of the change here: The 5.2.1 should only change that a click on a Link will not push a new entry to the history stack when a duplicate entry is already there. This makes the Link component behave the same as a native HTML anchor (<a>) tag. If it breaks your login, you were leaning on a bugged behaviour in React Router. (Maybe you used a Link where you needed a button?)

What I find more weird is that DependaBot did not touch your history package, but you do downgrade it. Is that correct?

@swaterkamp
Copy link
Member Author

Thanks for pointing this out @guidobouman . Dependabot touched history a week before in #3133 and we suspected a possible interplay between these two packages. We needed a quick fix to be able to log in again, while we are aware of the fact, that we indeed possibly relied on buggy behavior in the past. This issue was on our list to be revisited, already, but I'll investigate this again now sooner than later.

@guidobouman
Copy link

Great to hear your extensive thoughts. All the best!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants