dbfannin / ngx-logger

Angular logger
MIT License
428 stars 110 forks source link

Run server logging outside angular zone #324

Closed yohny closed 1 year ago

yohny commented 1 year ago

Currently ngx-logger runs server logging calls in the angular zone (NgZone), which has a consequence that whenever server logging happens, it triggers change detection (as all network calls do under default change detection strategy). This has not only a performance impact, but in certain scenarios can cause an infinite loop to occur when logging. See stackblitz repro here (warning - as this repro causes infinite loop it will kill your browser after a while, so I recommend opening it in a different browser, so it can be killed without disturbing your current browser session).

The explanation of the linked repro is quite simple - there is a template, that renders a property of component:

<p>
  {{ infoMessage }}
</p>

the infoMessage property looks like this:

get infoMessage() {
  let age = this.person?.age;
  if (!age) {
    this._logger.warn('missing age');
    return;
  }
  if (age > 21) {
    return 'Person age is above 21';
  }
  return 'Person age is not above 21';
}

and there is the problem - inside the property the logging happens (as the person age is not set in this repro case), which results in log being send to server, which in turn triggers change detection and thus reevaluation of the property, which triggers logging again... resulting in infinite loop.

This can be avoided by running logging outside of angular zone, aka. injecting NgZone into component, and logging like this:

this._zone.runOutsideAngular(() => { // this._zone is the injected NgZone
 this._logger.warn('mising age');
});

this way logging does not trigger change detection, eliminating the infinite loop issue.

It would be helpful if out of the box server logging was performed outside angular zone by the library itself, so the library users dont have to wrap their logging calls as shown above. Its also quite unexpected that logging triggers change detection, as its the case now. There are also performance benefits when running outside angular zone. Of course if someone wants the logging call to trigger change detection (although I can not imagine why) there could be a possibility to opt-in to original behavior.

bmtheo commented 1 year ago

I added serverCallsOutsideNgZone in the config to perform server calls outside of angular zone. This is not published yet, if you are interested you can test it by using this branch https://github.com/dbfannin/ngx-logger/tree/324-server-calls-outside-ngzone

yohny commented 1 year ago

your fix seems to work, thx it would be better to make running outside NqZone the dafault as without it users might run into infinite loop and it provides also performance benefits

bmtheo commented 1 year ago

I understand your point but this would be a breaking change for those that don't use zone. It is probably not common but possible according to angular docs https://angular.io/guide/zone

Feature available in 5.0.12