Skip to content

fix(router): NavigatonError and NavigationCancel should always be emi… - #20803

Closed
vsavkin wants to merge 1 commit into
angular:masterfrom
vsavkin:fix_error_emitted_too_early
Closed

fix(router): NavigatonError and NavigationCancel should always be emi…#20803
vsavkin wants to merge 1 commit into
angular:masterfrom
vsavkin:fix_error_emitted_too_early

Conversation

@vsavkin

@vsavkin vsavkin commented Dec 5, 2017

Copy link
Copy Markdown
Contributor

…tted after resetting the URL back

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Sometimes we emit NavigatonCancel before resetting the URL, sometimes after.

Issue Number: N/A

What is the new behavior?

We should always emit "final" events (NavigatonError, NavigationCancel, NavigationEnd) after we reset the state of the router.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@vsavkin
vsavkin force-pushed the fix_error_emitted_too_early branch from 616e7f5 to 320828b Compare December 5, 2017 16:35
jasonaden
jasonaden approved these changes Dec 5, 2017
View reviewed changes

@jasonaden jasonaden left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good. Let's see how many redirect/error handling issues this fixes!

@jasonaden jasonaden added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels Dec 5, 2017
@jasonaden jasonaden closed this in d8cc09b Dec 8, 2017
jasonaden pushed a commit that referenced this pull request Dec 8, 2017
@vsavkin @jasonaden
fix(router): NavigatonError and NavigationCancel should be emitted af…
baeec4d 
…ter resetting the URL (#20803)

PR Close #20803
This was referenced Dec 12, 2017
Closed
Closed
Open
@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented Sep 12, 2019

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@jasonaden jasonaden jasonaden approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker area: router cla: yes target: patch This PR is targeted for the next patch release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vsavkin @jasonaden @IgorMinar @googlebot