-
-
Notifications
You must be signed in to change notification settings - Fork 115
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
feat(router config): ignoreUnknownRoutes option #558
Conversation
@EisenbergEffect I've done some simple testing and I've found that this doesn't seem to solve the issue it was trying to address. In any case, I think we shouldn't release this until its tested and verified, and I'm looking to get a release of the latest changes out in the next week. Shall I revert this using the GitHub revert feature, or manually in Git? |
GitHub revert is fine. |
@davismj https://github.com/aurelia/router/pull/561/files this shows how it should work although that's not a final version, as a note there are a handful of failing specs right now but they were failing before I opened that PR. |
In a nutshell if you have pushState enabled you should still be able to create an href that is not swallowed by the router (they all currently are) and instead you should be able to trigger navigation that the router ignores. I think the existing PR simply swallowed the error but we need to propagate the route up which the PR I linked above addresses but is probably a work in progress, let me know if that makes sense. Also if we are not releasing until Tuesday please do not revert a commit without giving the author a chance to fix the issue. |
@EisenbergEffect opened, I'll pull it in. @PWKad its not that I don't want him to finish it, its that I'd rather not rush it, I'd rather have the time to test these things fully and add some docs before they get pulled in. |
@davismj The changelog of v1.5.0 says it got pulled in. Just checked and it is not, can you remove the entry from the releases changelog? |
Thanks, done. |
Implements the ignoreUnknownRoutes option mentioned in issue #527.