chore: addressed review comments

rudderlabs · Oct 18, 2024 · 6455d95 · 6455d95
1 parent 26a26c5
commit 6455d95
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 35 deletions.
diff --git 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<boolean>;
   pageLifecycle: PageLifecycleState;
 };
 
 export type PageLifecycleState = {
+  enabled: Signal<boolean>;
   visitId: Signal<string | undefined>;
   pageLoadedTimestamp: Signal<number | undefined>;
 };

diff --git 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
@@ -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
@@ -140,8 +140,15 @@ class RudderAnalytics implements IRudderAnalytics<IAnalytics> {
     }
 
     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<IAnalytics> {
    * @param loadOptions
    * @returns
    */
-  trackPageLifecycleEvents(loadOptions?: Partial<LoadOptions>) {
+  trackPageLifecycleEvents(
+    preloadedEventsArray: PreloadedEventCall[],
+    loadOptions?: Partial<LoadOptions>,
+  ) {
     const { autoTrack, useBeacon } = loadOptions ?? {};
     const {
       enabled: autoTrackEnabled = false,
@@ -177,17 +187,14 @@ class RudderAnalytics implements IRudderAnalytics<IAnalytics> {
       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<IAnalytics> {
    * @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<IAnalytics> {
    * 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
@@ -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
@@ -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));