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

feature(router): warn if navigation triggered outside Angular zone #24728

Closed
trotyl opened this Issue Jul 2, 2018 · 5 comments

Comments

Projects
None yet
4 participants
@trotyl
Contributor

trotyl commented Jul 2, 2018

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Performance issue
[x] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question
[ ] Other... Please describe:

Current behavior

When some user performs navigation outside the Angular zone, the change detection will not be invoked.

Even it's certainly users' fault, they may still going to be confused by what the problem is:

Expected behavior

As the navigation should always happen in Angular zone, so router itself could guard against that thus produce a warning when not valid.

Minimal reproduction of the problem with instructions

N/A.

What is the motivation / use case for changing the behavior?

Reduce the invalid issue amounts.

Environment


Angular version: X.Y.Z


Browser:
- [ ] Chrome (desktop) version XX
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX
 
For Tooling issues:
- Node version: XX  
- Platform:  

Others:

@JiaLiPassion

This comment has been minimized.

Contributor

JiaLiPassion commented Jul 7, 2018

If there is no use case that user want to run navigation outside of ngZone, I vote for force inside ngZone solution.

@trotyl

This comment has been minimized.

Contributor

trotyl commented Jul 7, 2018

@JiaLiPassion But for ErrorHandler-like usages, it's switched to root zone deliberately to avoid possible infinite loops, once comes implicit switching to Angular zone in navigation, the risk of infinite loops could be back again.

@JiaLiPassion

This comment has been minimized.

Contributor

JiaLiPassion commented Jul 7, 2018

@trotyl, yeah, so the case will be.
ErrorHandler -> ngZone.runOutsideAngular -> navigation to error page -> ngZone.run -> CD cause error -> ErrorHandler.

I think maybe we can do something to let ErrorHandler know that the error is triggered by previous ErrorHandler and just stop...

@kara kara added the comp: router label Jul 10, 2018

@ngbot ngbot bot added this to the needsTriage milestone Jul 10, 2018

@jasonaden

This comment has been minimized.

Contributor

jasonaden commented Jul 17, 2018

@trotyl Is this something you want to implement?

Also, we would like to reduce the reliance on Zones to make Zones optional in the future. So whatever the warning is, it needs to be there only when Zones is being used for CD.

@trotyl

This comment has been minimized.

Contributor

trotyl commented Jul 18, 2018

@jasonaden Added a PR for warning purpose, should be OK for noop zone environments.

trotyl added a commit to trotyl/angular that referenced this issue Jul 19, 2018

trotyl added a commit to trotyl/angular that referenced this issue Aug 8, 2018

trotyl added a commit to trotyl/angular that referenced this issue Aug 9, 2018

mhevery added a commit to trotyl/angular that referenced this issue Sep 5, 2018

mhevery added a commit that referenced this issue Sep 5, 2018

@mhevery mhevery closed this in 010e35d Sep 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment