EmmanuelRoux / ngx-matomo-client

Matomo analytics client for Angular applications
https://matomo.org/
MIT License
74 stars 16 forks source link

fix(tracker): add the option to run tracking outside of angular zone #62

Closed Arrakis-Lux closed 2 years ago

Arrakis-Lux commented 2 years ago

Fixes #60

Tests are broken though I have very little experience with those and the fix did not appeared to be obvious.

EmmanuelRoux commented 2 years ago

Thanks for the PR @Arrakis-Lux

You can add some tests like that:

Git Patch ```diff Index: projects/tracker/src/lib/matomo-tracker.service.spec.ts <+>UTF-8 =================================================================== diff --git a/projects/tracker/src/lib/matomo-tracker.service.spec.ts b/projects/tracker/src/lib/matomo-tracker.service.spec.ts --- a/projects/tracker/src/lib/matomo-tracker.service.spec.ts (revision 736dd3637ced5f88bd2451d648dee85ed3b18e45) +++ b/projects/tracker/src/lib/matomo-tracker.service.spec.ts (date 1669319256568) @@ -1,3 +1,4 @@ +import { NgZone } from '@angular/core'; import { InternalMatomoConfiguration } from './configuration'; import { MatomoHolder } from './holder'; import { @@ -15,8 +16,16 @@ const PLATFORM_SERVER_ID = 'server'; describe('MatomoTracker', () => { - function createTracker(disabled = false, platform: Object = PLATFORM_BROWSER_ID): MatomoTracker { - return createMatomoTracker({ disabled } as InternalMatomoConfiguration, platform); + function createMockZone(): jasmine.SpyObj { + return jasmine.createSpyObj(['runOutsideAngular']); + } + + function createTracker( + config: Partial = { disabled: false }, + platform: Object = PLATFORM_BROWSER_ID, + ngZone: NgZone = createMockZone() + ): MatomoTracker { + return createMatomoTracker(config as InternalMatomoConfiguration, platform, ngZone); } beforeEach(() => { @@ -632,7 +641,7 @@ it('should ignore calls when disabled', () => { // Given - const tracker = createTracker(true); + const tracker = createTracker({ disabled: true }); (window as any)._paq = undefined; // Then @@ -642,7 +651,7 @@ it('should reject all promises when disabled', done => { // Given - const tracker = createTracker(true); + const tracker = createTracker({ disabled: true }); // Then tracker @@ -656,7 +665,7 @@ it('should ignore calls when platform is not browser', () => { // Given - const tracker = createTracker(false, PLATFORM_SERVER_ID); + const tracker = createTracker({ disabled: false }, PLATFORM_SERVER_ID); (window as any)._paq = undefined; // Then @@ -666,7 +675,7 @@ it('should reject all promises when platform is not browser', done => { // Given - const tracker = createTracker(false, PLATFORM_SERVER_ID); + const tracker = createTracker({ disabled: false }, PLATFORM_SERVER_ID); // Then tracker @@ -677,4 +686,23 @@ done(); }); }); + + it('should run commands outside Angular Zone', () => { + // Given + const zone = createMockZone(); + const tracker = createTracker({ runOutsideAngularZone: true }, PLATFORM_BROWSER_ID, zone); + let runOutside = false; + + zone.runOutsideAngular.and.callFake(fn => { + runOutside = true; + return fn(); + }); + + // When + tracker.ping(); + + // Then + expect(runOutside).toBeTrue(); + expect(window._paq).toEqual([['ping']]); + }); }); ```
Arrakis-Lux commented 2 years ago

Hey there !

Hope you had a nice evening !

Just added the suggered tests (thanks for that :p I really need to work on this aspect ^-^)

Also updated the configuration documentation. I am not sure about the "Available in manual mode" part though.

EmmanuelRoux commented 2 years ago

@Arrakis-Lux Thank you, looks good to me!

Maybe we can do some optimization in the future, but we can keep it as-is for a first opt-in fix. This fix might become a default in a future version of the lib. For that reason, please let us know any feedback in this PR or in a new issue!

EmmanuelRoux commented 2 years ago

:tada: This PR is included in version 4.0.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

EmmanuelRoux commented 1 year ago

:tada: This PR is included in version 4.0.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket: