From 6455d95957ddfe2804a939b17c809f5709830eb6 Mon Sep 17 00:00:00 2001 From: MoumitaM Date: Fri, 18 Oct 2024 16:16:43 +0530 Subject: [PATCH] chore: addressed review comments --- .../src/types/ApplicationState.ts | 2 ++ .../__tests__/app/RudderAnalytics.test.ts | 32 ++++++++++--------- packages/analytics-js/public/index.html | 5 ++- .../analytics-js/src/app/RudderAnalytics.ts | 32 ++++++++++++------- .../src/state/slices/autoTrack.ts | 2 ++ .../src/state/slices/loadOptions.ts | 7 ---- 6 files changed, 45 insertions(+), 35 deletions(-) diff --git a/packages/analytics-js-common/src/types/ApplicationState.ts b/packages/analytics-js-common/src/types/ApplicationState.ts index 123c032d6..f1b0181ab 100644 --- a/packages/analytics-js-common/src/types/ApplicationState.ts +++ b/packages/analytics-js-common/src/types/ApplicationState.ts @@ -84,10 +84,12 @@ export type LifecycleState = { }; export type AutoTrackState = { + enabled: Signal; pageLifecycle: PageLifecycleState; }; export type PageLifecycleState = { + enabled: Signal; visitId: Signal; pageLoadedTimestamp: Signal; }; diff --git a/packages/analytics-js/__tests__/app/RudderAnalytics.test.ts b/packages/analytics-js/__tests__/app/RudderAnalytics.test.ts index 4247309b5..462b001e7 100644 --- a/packages/analytics-js/__tests__/app/RudderAnalytics.test.ts +++ b/packages/analytics-js/__tests__/app/RudderAnalytics.test.ts @@ -1,4 +1,5 @@ import type { LoadOptions } from '@rudderstack/analytics-js-common/types/LoadOptions'; +import type { PreloadedEventCall } from '../../src/components/preloadBuffer/types'; import { state } from '../../src/state'; import { RudderAnalytics } from '../../src/app/RudderAnalytics'; import { Analytics } from '../../src/components/core/Analytics'; @@ -322,20 +323,22 @@ describe('trackPageLifecycleEvents', () => { }); it('should not add pageLifecycleEvents in the buffer when the tracking is not enabled through load options', () => { - rudderAnalyticsInstance.trackPageLifecycleEvents({}); + const bufferedEvents: PreloadedEventCall[] = []; + rudderAnalyticsInstance.trackPageLifecycleEvents(bufferedEvents, {}); - expect(window.RudderStackGlobals?.app?.preloadedEventsBuffer).toEqual([]); + expect(bufferedEvents).toEqual([]); }); it('should inherit enabled and options properties from autoTrack load option', () => { - rudderAnalyticsInstance.trackPageLifecycleEvents({ + const bufferedEvents: PreloadedEventCall[] = []; + rudderAnalyticsInstance.trackPageLifecycleEvents(bufferedEvents, { autoTrack: { enabled: true, options: { key: 'value' }, }, }); - expect(window.RudderStackGlobals?.app?.preloadedEventsBuffer).toEqual([ + expect(bufferedEvents).toEqual([ [ 'track', 'Page Loaded', @@ -346,7 +349,8 @@ describe('trackPageLifecycleEvents', () => { }); it('should override enabled and options properties of autoTrack if provided in load option', () => { - rudderAnalyticsInstance.trackPageLifecycleEvents({ + const bufferedEvents: PreloadedEventCall[] = []; + rudderAnalyticsInstance.trackPageLifecycleEvents(bufferedEvents, { autoTrack: { enabled: true, options: { key: 'value' }, @@ -356,11 +360,12 @@ describe('trackPageLifecycleEvents', () => { }, }); - expect(window.RudderStackGlobals?.app?.preloadedEventsBuffer).toEqual([]); + expect(bufferedEvents).toEqual([]); }); it('should track Page Loaded event irrespective of useBeacon load option', () => { - rudderAnalyticsInstance.trackPageLifecycleEvents({ + const bufferedEvents: PreloadedEventCall[] = []; + rudderAnalyticsInstance.trackPageLifecycleEvents(bufferedEvents, { useBeacon: false, autoTrack: { pageLifecycle: { @@ -369,7 +374,7 @@ describe('trackPageLifecycleEvents', () => { }, }); - expect(window.RudderStackGlobals?.app?.preloadedEventsBuffer).toEqual([ + expect(bufferedEvents).toEqual([ [ 'track', 'Page Loaded', @@ -380,8 +385,9 @@ describe('trackPageLifecycleEvents', () => { }); it('should track Page Unloaded event if useBeacon is set to true and trackPageLifecycle feature is enabled', () => { + const bufferedEvents: PreloadedEventCall[] = []; rudderAnalyticsInstance.track = jest.fn(); - rudderAnalyticsInstance.trackPageLifecycleEvents({ + rudderAnalyticsInstance.trackPageLifecycleEvents(bufferedEvents, { useBeacon: true, autoTrack: { pageLifecycle: { @@ -402,17 +408,13 @@ describe('trackPageLifecycleEvents', () => { }); it('should invoke trackPageLifecycleEvents method when load API is called', () => { - const trackPageLifecycleEventsSpy = jest.spyOn( - rudderAnalyticsInstance, - 'trackPageLifecycleEvents', - ); - + rudderAnalyticsInstance.trackPageLifecycleEvents = jest.fn(); rudderAnalyticsInstance.load('writeKey', 'data-plane-url', { autoTrack: { enabled: true, }, }); - expect(trackPageLifecycleEventsSpy).toHaveBeenCalledWith({ + expect(rudderAnalyticsInstance.trackPageLifecycleEvents).toHaveBeenCalledWith([], { autoTrack: { enabled: true, }, diff --git a/packages/analytics-js/public/index.html b/packages/analytics-js/public/index.html index caf0b9610..fb35e033a 100644 --- a/packages/analytics-js/public/index.html +++ b/packages/analytics-js/public/index.html @@ -160,8 +160,11 @@ // 'StorageMigrator', // 'XhrQueue' // ] - // trackPageLifecycle:{ + // autoTrack:{ // enabled: true, + // pageLifecycle:{ + // enabled:true + // } // } }; diff --git a/packages/analytics-js/src/app/RudderAnalytics.ts b/packages/analytics-js/src/app/RudderAnalytics.ts index d71d1a1b6..7c055a44a 100644 --- a/packages/analytics-js/src/app/RudderAnalytics.ts +++ b/packages/analytics-js/src/app/RudderAnalytics.ts @@ -140,8 +140,15 @@ class RudderAnalytics implements IRudderAnalytics { } this.setDefaultInstanceKey(writeKey); + const preloadedEventsArray = this.getPreloadedEvents(); + // Track page loaded lifecycle event if enabled - this.trackPageLifecycleEvents(loadOptions); + this.trackPageLifecycleEvents(preloadedEventsArray, loadOptions); + + // The array will be mutated in the below method + promotePreloadedConsentEventsToTop(preloadedEventsArray); + + setExposedGlobal(GLOBAL_PRELOAD_BUFFER, clone(preloadedEventsArray)); this.analyticsInstances[writeKey] = new Analytics(); this.getAnalyticsInstance(writeKey).load(writeKey, dataPlaneUrl, loadOptions); @@ -164,7 +171,10 @@ class RudderAnalytics implements IRudderAnalytics { * @param loadOptions * @returns */ - trackPageLifecycleEvents(loadOptions?: Partial) { + trackPageLifecycleEvents( + preloadedEventsArray: PreloadedEventCall[], + loadOptions?: Partial, + ) { const { autoTrack, useBeacon } = loadOptions ?? {}; const { enabled: autoTrackEnabled = false, @@ -177,17 +187,14 @@ class RudderAnalytics implements IRudderAnalytics { options = autoTrackOptions, } = pageLifecycle ?? {}; - const preloadedEventsArray = this.getPreloadedEvents(); + state.autoTrack.enabled.value = autoTrackEnabled; + state.autoTrack.pageLifecycle.enabled.value = enabled; - if (enabled) { - this.trackPageLoadedEvent(events, options, preloadedEventsArray); - this.setupPageUnloadTracking(events, useBeacon, options); + if (!enabled) { + return; } - - // The array will be mutated in the below method - promotePreloadedConsentEventsToTop(preloadedEventsArray); - - setExposedGlobal(GLOBAL_PRELOAD_BUFFER, clone(preloadedEventsArray)); + this.trackPageLoadedEvent(events, options, preloadedEventsArray); + this.setupPageUnloadTracking(events, useBeacon, options); } /** @@ -196,6 +203,7 @@ class RudderAnalytics implements IRudderAnalytics { * @param options * @param preloadedEventsArray */ + // eslint-disable-next-line class-methods-use-this private trackPageLoadedEvent( events: PageLifecycleEvents[], options: ApiOptions, @@ -241,7 +249,7 @@ class RudderAnalytics implements IRudderAnalytics { * Register page unload listener to track page unload event * @param options */ - private registerPageUnloadListener(options: object) { + private registerPageUnloadListener(options: ApiOptions) { onPageLeave((isAccessible: boolean) => { if (isAccessible === false && state.lifecycle.loaded.value) { const pageUnloadedTimestamp = Date.now(); diff --git a/packages/analytics-js/src/state/slices/autoTrack.ts b/packages/analytics-js/src/state/slices/autoTrack.ts index 965a6c015..302f2c818 100644 --- a/packages/analytics-js/src/state/slices/autoTrack.ts +++ b/packages/analytics-js/src/state/slices/autoTrack.ts @@ -2,7 +2,9 @@ import { signal } from '@preact/signals-core'; import type { AutoTrackState } from '@rudderstack/analytics-js-common/types/ApplicationState'; const autoTrackState: AutoTrackState = { + enabled: signal(false), pageLifecycle: { + enabled: signal(false), visitId: signal(undefined), pageLoadedTimestamp: signal(undefined), }, diff --git a/packages/analytics-js/src/state/slices/loadOptions.ts b/packages/analytics-js/src/state/slices/loadOptions.ts index 53d3aaa70..aae972fd6 100644 --- a/packages/analytics-js/src/state/slices/loadOptions.ts +++ b/packages/analytics-js/src/state/slices/loadOptions.ts @@ -44,13 +44,6 @@ const defaultLoadOptions: LoadOptions = { }, sendAdblockPageOptions: {}, useServerSideCookies: false, - autoTrack: { - enabled: false, - pageLifecycle: { - enabled: false, - events: [PageLifecycleEvents.LOADED, PageLifecycleEvents.UNLOADED], - }, - }, }; const loadOptionsState: LoadOptionsState = signal(clone(defaultLoadOptions));