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

popTo causes an infinite loop when route is not found (as seen on #3026) #3043

Conversation

daviscabral
Copy link
Collaborator

@daviscabral daviscabral commented May 20, 2018

I am not sure about the solution here. Right now it is just assuming that it does not exist and returning to the top level route existent.

The problem with this approach is that might allow calls to inexistent routes pass unknown.

What do you think here guys?

  1. Add Keep the warning about the unknown route;
  2. Thrown an error if reaches the top level without finding a routeName that matches;
  3. Keep like it is in this PR, just redirects to the top level.

(This PR is about the problem reported in #3026)

@daviscabral daviscabral requested a review from aksonov May 20, 2018 02:56
@daviscabral daviscabral force-pushed the 3026/dc/pop-to-causes-infinite-loop-when-route-is-not-found branch from 5eec2cb to 309b55a Compare May 20, 2018 03:45
@daviscabral daviscabral requested a review from Blapi May 23, 2018 15:03
@daviscabral
Copy link
Collaborator Author

@aksonov any objections about getting this merged?

@aksonov
Copy link
Owner

aksonov commented Jun 6, 2018

Strange, I didn't received any email about that, sorry. will review now

Copy link
Owner

@aksonov aksonov left a comment

Choose a reason for hiding this comment

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

LGTM

@aksonov aksonov merged commit 080813e into aksonov:master Jun 6, 2018
@SexySix SexySix mentioned this pull request Jul 8, 2018
@daviscabral daviscabral deleted the 3026/dc/pop-to-causes-infinite-loop-when-route-is-not-found branch July 26, 2018 07:05
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.

2 participants