search

moribvndvs / ng2-idle

Responding to idle users in Angular (not AngularJS) applications.
https://moribvndvs.github.io/ng2-idle
Apache License 2.0
315 stars 128 forks source link

keepalive interval should run outside of Angular zone #113

Closed just-jeb closed 4 years ago

just-jeb commented 5 years ago

I'm submitting a ... (check one with "x")

[x ] bug report => search github for a similar issue or PR before submitting
[ ] feature request
[ ] support request => Please do not submit support request here, instead see https://github.com/HackedByChinese/ng2-idle/blob/master/CONTRIBUTING.md#getting-help

Current behavior

Keepalive interval is run inside Angular zone, which causes Testability to be unstable.

Expected behavior

Keepalive interval should not affect Testability state, i.e. run outside of Angular zone.

Minimal reproduction of the problem with instructions

  1. Set interval: keepalive.inteval(1000)
  2. Start keepalive: keepalive.start()
  3. Check the Testability state: getAllAngularTestabilities()[0].isStable()
  4. See that it is unstable, i.e. false

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

To let the e2e tests pass in applications that use keepalive.

Please tell us about your environment:

Windows 7, IntelliJ, npm, Angular dev server

Suggested solution: Keepalive interval function should be run outside of Angular zone, while onPing emission should run inside the Angular zone.

just-jeb commented 5 years ago

I can issue a PR if needed.

flekmatik commented 5 years ago

Do I understand correctly that no one can use protractor for testing his application once this package is used? Is there any workaround?

just-jeb commented 5 years ago

There is actually. If you run keepalive.interval() outside of Angular zone this will work. Worked for me.
But I still think this behavior should come out of the box with this package.

flekmatik commented 5 years ago

Thanks!! It would be great if you made a pull request, then we can source that one directly.

just-jeb commented 5 years ago

Will do in the nearest future.

michaelmills commented 5 years ago

Submitted a pull request: https://github.com/HackedByChinese/ng2-idle/pull/122

moribvndvs commented 4 years ago

Whoops, this PR was merged and I think has been in the codebase for a while now. It's definitely in the 8.0.0 beta builds.