fix(angular): provide a single compilation unit for the trace directive (#3617)

Co-authored-by: Kamil Ogórek <[email protected]>

Author: arturovt

packages/angular/README.md

@@ -155,11 +155,11 @@ To track Angular components as part of your transactions, you have 3 options.
 _TraceDirective:_ used to track a duration between `OnInit` and `AfterViewInit` lifecycle hooks in template:
 
 ```javascript
-import { TraceDirective } from '@sentry/angular';
+import { TraceModule } from '@sentry/angular';
 
 @NgModule({
   // ...
-  declarations: [TraceDirective],
+  imports: [TraceModule],
   // ...
 })
 export class AppModule {}
@@ -168,9 +168,9 @@ export class AppModule {}
 Then inside your components template (keep in mind that directive name attribute is required):
 
 ```html
-<app-header [trace]="'header'"></app-header>
-<articles-list [trace]="'articles-list'"></articles-list>
-<app-footer [trace]="'footer'"></app-footer>
+<app-header trace="header"></app-header>
+<articles-list trace="articles-list"></articles-list>
+<app-footer trace="footer"></app-footer>
 ```
 
 _TraceClassDecorator:_ used to track a duration between `OnInit` and `AfterViewInit` lifecycle hooks in components:

packages/angular/src/errorhandler.ts

@@ -2,18 +2,7 @@ import { HttpErrorResponse } from '@angular/common/http';
 import { ErrorHandler as AngularErrorHandler, Injectable } from '@angular/core';
 import * as Sentry from '@sentry/browser';
 
-// That's the `global.Zone` exposed when the `zone.js` package is used.
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-declare const Zone: any;
-
-// There're 2 types of Angular applications:
-// 1) zone-full (by default)
-// 2) zone-less
-// The developer can avoid importing the `zone.js` package and tells Angular that
-// he is responsible for running the change detection by himself. This is done by
-// "nooping" the zone through `CompilerOptions` when bootstrapping the root module.
-// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
-const isNgZoneEnabled = typeof Zone !== 'undefined' && !!Zone.current;
+import { runOutsideAngular } from './zone';
 
 /**
  * Options used to configure the behavior of the Angular ErrorHandler.
@@ -51,16 +40,7 @@ class SentryErrorHandler implements AngularErrorHandler {
     const extractedError = this._extractError(error) || 'Handled unknown error';
 
     // Capture handled exception and send it to Sentry.
-    const eventId = isNgZoneEnabled
-      ? // The `Zone.root.run` basically will capture the exception in the most parent zone.
-        // The Angular's zone is forked from the `Zone.root`. In this case, `zone.js` won't
-        // trigger change detection, and `ApplicationRef.tick()` will not be run.
-        // Caretaker note: we're using `Zone.root` except `NgZone.runOutsideAngular` since this
-        // will require injecting the `NgZone` facade. That will create a breaking change for
-        // projects already using the `SentryErrorHandler`.
-        // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
-        Zone.root.run(() => Sentry.captureException(extractedError))
-      : Sentry.captureException(extractedError);
+    const eventId = runOutsideAngular(() => Sentry.captureException(extractedError));
 
     // When in development mode, log the error to console for immediate feedback.
     if (this._options.logErrors) {

packages/angular/src/index.ts

@@ -10,5 +10,6 @@ export {
   TraceClassDecorator,
   TraceMethodDecorator,
   TraceDirective,
+  TraceModule,
   TraceService,
 } from './tracing';

packages/angular/src/tracing.ts

@@ -1,23 +1,12 @@
-import { AfterViewInit, Directive, Injectable, Input, OnDestroy, OnInit } from '@angular/core';
+import { AfterViewInit, Directive, Injectable, Input, NgModule, OnDestroy, OnInit } from '@angular/core';
 import { Event, NavigationEnd, NavigationStart, Router } from '@angular/router';
 import { getCurrentHub } from '@sentry/browser';
 import { Span, Transaction, TransactionContext } from '@sentry/types';
 import { logger, stripUrlQueryAndFragment, timestampWithMs } from '@sentry/utils';
 import { Observable, Subscription } from 'rxjs';
 import { filter, tap } from 'rxjs/operators';
 
-// That's the `global.Zone` exposed when the `zone.js` package is used.
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-declare const Zone: any;
-
-// There're 2 types of Angular applications:
-// 1) zone-full (by default)
-// 2) zone-less
-// The developer can avoid importing the `zone.js` package and tells Angular that
-// he is responsible for running the change detection by himself. This is done by
-// "nooping" the zone through `CompilerOptions` when bootstrapping the root module.
-// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
-const isNgZoneEnabled = typeof Zone !== 'undefined' && !!Zone.current;
+import { runOutsideAngular } from './zone';
 
 let instrumentationInitialized: boolean;
 let stashedStartTransaction: (context: TransactionContext) => Transaction | undefined;
@@ -106,21 +95,10 @@ export class TraceService implements OnDestroy {
     filter(event => event instanceof NavigationEnd),
     tap(() => {
       if (this._routingSpan) {
-        if (isNgZoneEnabled) {
-          // The `Zone.root.run` basically will finish the transaction in the most parent zone.
-          // The Angular's zone is forked from the `Zone.root`. In this case, `zone.js` won't
-          // trigger change detection, and `ApplicationRef.tick()` will not be run.
-          // Caretaker note: we're using `Zone.root` except `NgZone.runOutsideAngular` since this
-          // will require injecting the `NgZone` facade. That will create a breaking change for
-          // projects already using the `TraceService`.
-          // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
-          Zone.root.run(() => {
-            // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-            this._routingSpan!.finish();
-          });
-        } else {
-          this._routingSpan.finish();
-        }
+        runOutsideAngular(() => {
+          // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+          this._routingSpan!.finish();
+        });
         this._routingSpan = null;
       }
     }),
@@ -136,7 +114,7 @@ export class TraceService implements OnDestroy {
 
   /**
    * This is used to prevent memory leaks when the root view is created and destroyed multiple times,
-   * since `subscribe` callbacks captures `this` and prevent many resources from being GC'd.
+   * since `subscribe` callbacks capture `this` and prevent many resources from being GC'd.
    */
   public ngOnDestroy(): void {
     this._subscription.unsubscribe();
@@ -179,6 +157,15 @@ export class TraceDirective implements OnInit, AfterViewInit {
   }
 }
 
+/**
+ * A module serves as a single compilation unit for the `TraceDirective` and can be re-used by any other module.
+ */
+@NgModule({
+  declarations: [TraceDirective],
+  exports: [TraceDirective],
+})
+export class TraceModule {}
+
 /**
  * Decorator function that can be used to capture initialization lifecycle of the whole component.
  */

packages/angular/src/zone.ts

@@ -0,0 +1,28 @@
+// That's the `global.Zone` exposed when the `zone.js` package is used.
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+declare const Zone: any;
+
+// There're 2 types of Angular applications:
+// 1) zone-full (by default)
+// 2) zone-less
+// The developer can avoid importing the `zone.js` package and tells Angular that
+// he is responsible for running the change detection by himself. This is done by
+// "nooping" the zone through `CompilerOptions` when bootstrapping the root module.
+// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
+const isNgZoneEnabled = typeof Zone !== 'undefined' && !!Zone.current;
+
+/**
+ * The function that does the same job as `NgZone.runOutsideAngular`.
+ */
+export function runOutsideAngular<T>(callback: () => T): T {
+  // The `Zone.root.run` basically will run the `callback` in the most parent zone.
+  // Any asynchronous API used inside the `callback` won't catch Angular's zone
+  // since `Zone.current` will reference `Zone.root`.
+  // The Angular's zone is forked from the `Zone.root`. In this case, `zone.js` won't
+  // trigger change detection, and `ApplicationRef.tick()` will not be run.
+  // Caretaker note: we're using `Zone.root` except `NgZone.runOutsideAngular` since this
+  // will require injecting the `NgZone` facade. That will create a breaking change for
+  // projects already using the `@sentry/angular`.
+  // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
+  return isNgZoneEnabled ? Zone.root.run(callback) : callback();
+}