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

Not possible to use Angular without Zone because of Router.setUpLocationChangeListener #20582

Closed
ArekSliwa opened this issue Nov 22, 2017 · 12 comments

Comments

@ArekSliwa
Copy link

ArekSliwa commented Nov 22, 2017

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[x ] Bug report  
[ ] 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

Current behavior

When using Angular Router it's not possible to use new Angular 5 feature ngZone: 'noop' .
In the console we can see: Zone is not defined error.
Router method setUpLocationChangeListener is using Zone.
screenshots:
zone is not defined
Router.setUpLocationChangeListener method implementation

Expected behavior

Angular Router is able to work without Zone.

Minimal reproduction of the problem with instructions

  1. Just fire angular app with
    platformBrowserDynamic()
    .bootstrapModule(AppModule, {
    ngZone: 'noop'
    });
  2. comment import 'zone.js/dist/zone'; in polyfills
  3. Try to use angular Router in some easy case.

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

Environment


Angular version: 5.0.1



Browser:
- [x ] 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
Copy link
Contributor

@mhevery , should we provide a empty Zone implementation when use NgNoopZone?

such as for this issue,

if (!global['Zone']) {
  function NoopZone() {
    wrap(func) {
      return func;
    }
    ...
  }
  
  global['Zone'] = NoopZone;
  NoopZone.current = new NoopZone();
}

or provide a auto changeDetection version?

if (!global['Zone']) {
  function NoopZone() {
    wrap(func) {
      return function() {
        const result = func.apply(this, arguments);
        changeDetectRef.detectChange();
      } ;
    }
    ...
  }
  
  global['Zone'] = NoopZone;
  NoopZone.current = new NoopZone();
}

@mhevery
Copy link
Contributor

mhevery commented Nov 27, 2017

@JiaLiPassion Instead of creating a noop mock, we should just remove that code from router. It seems like that 25e5b2f#diff-7f0963cbc503a361ded7d2bfec5f01b9R386 is there because zone.js does not properly patch the lociation.subscribe on the Rx. Given that now have a patch for Rx it does not seem like it is needed. Or rewrite the code in a way which does not need this???

@JiaLiPassion
Copy link
Contributor

@mhevery , got it, I will look into it.

JiaLiPassion added a commit to JiaLiPassion/angular that referenced this issue Nov 28, 2017
JiaLiPassion added a commit to JiaLiPassion/angular that referenced this issue Nov 28, 2017
@ArekSliwa
Copy link
Author

Thanks a lot for your attention to this issue! seems I can close it?

@JiaLiPassion
Copy link
Contributor

@ArekSliwa, if my PR is correct, after merge , this issue will be closed automatically, thank you for posting the issue.

@me-12
Copy link

me-12 commented Dec 20, 2017

@JiaLiPassion Thank you for your PR. I am currently running into the same issue and was wondering if you could give an update on this issue. It looks like Travis is complaining about errors while testing.

@JiaLiPassion
Copy link
Contributor

@me-12 , the travis is passed in the PR, do you mean some test failed in your travis build ?

@me-12
Copy link

me-12 commented Dec 20, 2017

@JiaLiPassion Sorry my bad. I was looking at your fork instead of the PR. #20640

So we are just waiting for @mhevery to review the PR again. Right?

@JiaLiPassion
Copy link
Contributor

@me-12, yeah, I think so. please wait for a while.

@glebmachine
Copy link

Any news?

@ngbot ngbot bot added this to the Backlog milestone Jan 23, 2018
JiaLiPassion added a commit to JiaLiPassion/angular that referenced this issue Jan 30, 2018
mhevery pushed a commit to JiaLiPassion/angular that referenced this issue Feb 1, 2018
JiaLiPassion added a commit to JiaLiPassion/angular that referenced this issue Feb 3, 2018
JiaLiPassion added a commit to JiaLiPassion/angular that referenced this issue Feb 3, 2018
JiaLiPassion added a commit to JiaLiPassion/angular that referenced this issue Feb 3, 2018
@alxhub alxhub closed this as completed in f791e9f Feb 5, 2018
@glebmachine
Copy link

Wow, hope finally it works)

@angular-automatic-lock-bot
Copy link

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 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants