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.

Sign up for GitHub

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

Jump to bottom

Run async stuff outside Angular #196

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Run async stuff outside Angular #196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions ...cationinsights-angularplugin-js/src/lib/applicationinsights-angularplugin-js.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { Subscription } from "rxjs";
import { AnalyticsPlugin } from "@microsoft/applicationinsights-analytics-js";
import {objDeepFreeze} from "@nevware21/ts-utils";
import { PropertiesPlugin } from "@microsoft/applicationinsights-properties-js";
import { runOutsideAngular } from "./run-outside-angular";

interface IAngularExtensionConfig {
/**
Expand Down Expand Up @@ -113,7 +114,7 @@ export class AngularPlugin extends BaseTelemetryPlugin {
const pageViewTelemetry: IPageViewTelemetry = {
uri: _angularCfg.router.url
};
_self.trackPageView(pageViewTelemetry);
runOutsideAngular(() => _self.trackPageView(pageViewTelemetry));
}

// subscribe to new router events
Expand All @@ -129,7 +130,7 @@ export class AngularPlugin extends BaseTelemetryPlugin {
uri: _angularCfg.router.url,
properties: { duration: 0 } // SPA route change loading durations are undefined, so send 0
};
_self.trackPageView(pvt);
runOutsideAngular(() => _self.trackPageView(pvt));
}
}
});
Expand All @@ -150,7 +151,7 @@ export class AngularPlugin extends BaseTelemetryPlugin {
_propertiesPlugin.context.telemetryTrace.traceID = generateW3CId();
_propertiesPlugin.context.telemetryTrace.name = location && location.pathname || "_unknown_";
}
_analyticsPlugin.trackPageView(pageView);
runOutsideAngular(() => _analyticsPlugin.trackPageView(pageView));
} else {
_throwInternal(_self.diagLog(),
// eslint-disable-next-line max-len
Expand Down Expand Up @@ -186,7 +187,7 @@ export class AngularPlugin extends BaseTelemetryPlugin {
* @param event The event that needs to be processed
*/
processTelemetry(event: ITelemetryItem, itemCtx?: IProcessTelemetryContext) {
this.processNext(event, itemCtx);
runOutsideAngular(() => this.processNext(event, itemCtx));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is likely not needed as this is called internally from all of the trackXXX calls -- so (if I'm following your comments correctly) this should already be getting run in the "outside" context...

Although, I do concede that if something does end up calling a track call within the Angular context (like any of the automatic events (JS errors, URL change etc)) then this would be the point at which it would switch to the different async context.

My only concern with this (which is less of an issue with any SPA framework) is that once the page starts to unload all operation "should" become synchronous to avoid the loss of events. And I don't think we current expose the internal flags indicating that we have received any unload signal (unload, beforeunload, pagehide and visibilitychange events)

}


Expand Down
24 changes: 24 additions & 0 deletions projects/applicationinsights-angularplugin-js/src/lib/run-outside-angular.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// This would be exposed on the `globalThis` whenever `zone.js` is
// included in the `polyfills` configuration property. Starting from Angular 17,
// users can opt-in to use zoneless change detection.
// eslint-disable-next-line @typescript-eslint/naming-convention
declare const Zone: any;

/**
* The function that does the same job as `NgZone.runOutsideAngular`.
* It doesn't require an injection context to be specified.
*
* ⚠️ Note: Most of the Application Insights functionality called from
* inside the Angular execution context must be wrapped in this function.
* Angular's rendering relies on asynchronous tasks being scheduled within
* its execution context.
* Since the plugin schedules tasks that do not interact with Angular's rendering,
* it may prevent Angular from functioning reliably. Consequently, it may disrupt
* processes such as server-side rendering or client-side hydration.
*/
export const runOutsideAngular = <T>(callback: () => T): T =>
// Running the `callback` within the root execution context enables Angular
// processes (such as SSR and hydration) to continue functioning normally without
// timeouts and delays that could affect the user experience.
typeof Zone !== "undefined" ? Zone.root.run(callback) : callback()
;
Loading