Merge pull request #247 from splitio/remove_unload_event_handler

Prepare release v1.9.1, that removes 'unload' event handler.

CHANGES.txt

@@ -1,10 +1,13 @@
+1.9.1 (September 21, 2023)
+ - Updated browser listener to avoid registering a handler for 'unload' DOM events, that can prevent browsers from being able to put pages in the back/forward cache for faster back and forward loads (Related to issue https://github.com/splitio/javascript-client/issues/759).
+
 1.9.0 (July 18, 2023)
  - Updated streaming architecture implementation to apply feature flag updates from the notification received which is now enhanced, improving efficiency and reliability of the whole update system.
 
 1.8.3 (June 29, 2023)
  - Updated some transitive dependencies for vulnerability fixes.
  - Updated SDK_READY_TIMED_OUT event to be emitted immediately when a connection error occurs using pluggable storage (i.e., when the wrapper `connect` promise is rejected) in consumer and partial consumer modes.
- - Bugfix - The `destroy` method has been updated to immediately flag the SDK client as destroyed, to prevent unexpected behaviours when `getTreatment` and `track` methods are called synchronously after `destroy` method is called.
+ - Bugfixing - The `destroy` method has been updated to immediately flag the SDK client as destroyed, to prevent unexpected behaviours when `getTreatment` and `track` methods are called synchronously after `destroy` method is called.
 
 1.8.2 (May 15, 2023)
  - Updated terminology on the SDKs codebase to be more aligned with current standard without causing a breaking change. The core change is the term split for feature flag on things like logs and IntelliSense comments.
@@ -90,7 +93,7 @@
    - Integration with Auth service V2, connecting to the new channels and applying the received connection delay.
    - Implemented handling of the new MySegmentsV2 notification types (SegmentRemoval, KeyList, Bounded and Unbounded)
    - New control notification for environment scoped streaming reset.
- - Updated localhost mode to emit SDK_READY_FROM_CACHE event in Browser when using localStorage (Related to issue https://github.com/splitio/react-client/issues/34).
+ - Updated localhost mode to emit SDK_READY_FROM_CACHE event in browser when using localStorage (Related to issue https://github.com/splitio/react-client/issues/34).
  - Updated dependencies for vulnerability fixes.
 
 0.1.0 (March 30, 2021)

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@splitsoftware/splitio-commons",
-  "version": "1.9.0",
+  "version": "1.9.1",
   "description": "Split Javascript SDK common components",
   "main": "cjs/index.js",
   "module": "esm/index.js",

src/listeners/__tests__/browser.spec.ts

@@ -79,12 +79,10 @@ const fakeSplitApi = {
 
 const VISIBILITYCHANGE_EVENT = 'visibilitychange';
 const PAGEHIDE_EVENT = 'pagehide';
-const UNLOAD_EVENT = 'unload';
 
 const eventListeners: any = {
   [VISIBILITYCHANGE_EVENT]: new Set<() => any>(),
   [PAGEHIDE_EVENT]: new Set<() => any>(),
-  [UNLOAD_EVENT]: new Set<() => any>()
 };
 
 // mock window and navigator objects
@@ -144,13 +142,13 @@ function triggerEvent(event: string, visibilityState?: string) { // @ts-expect-e
 function assertStart(listener: BrowserSignalListener) {
   // Assigned right function to right signal.
   expect((global.document.addEventListener as jest.Mock).mock.calls).toEqual([[VISIBILITYCHANGE_EVENT, listener.flushDataIfHidden]]);
-  expect((global.window.addEventListener as jest.Mock).mock.calls).toEqual([[PAGEHIDE_EVENT, listener.flushData], [UNLOAD_EVENT, listener.stopSync]]);
+  expect((global.window.addEventListener as jest.Mock).mock.calls).toEqual([[PAGEHIDE_EVENT, listener.flushData]]);
 }
 
 function assertStop(listener: BrowserSignalListener) {
   // removed correct listener from correct signal on stop.
   expect((global.document.removeEventListener as jest.Mock).mock.calls).toEqual([[VISIBILITYCHANGE_EVENT, listener.flushDataIfHidden]]);
-  expect((global.window.removeEventListener as jest.Mock).mock.calls).toEqual([[PAGEHIDE_EVENT, listener.flushData], [UNLOAD_EVENT, listener.stopSync]]);
+  expect((global.window.removeEventListener as jest.Mock).mock.calls).toEqual([[PAGEHIDE_EVENT, listener.flushData]]);
 }
 
 /* Mocks end */
@@ -164,7 +162,6 @@ test('Browser JS listener / consumer mode', () => {
 
   triggerEvent(VISIBILITYCHANGE_EVENT, 'hidden');
   triggerEvent(PAGEHIDE_EVENT);
-  triggerEvent(UNLOAD_EVENT);
 
   // Unload event was triggered, but sendBeacon and post services should not be called
   expect(global.window.navigator.sendBeacon).toBeCalledTimes(0);
@@ -205,31 +202,23 @@ test('Browser JS listener / standalone mode / Impressions optimized mode with te
 });
 
 test('Browser JS listener / standalone mode / Impressions debug mode', () => {
-  const syncManagerMockWithPushManager = { pushManager: { stop: jest.fn() } };
+  const syncManagerMock = {};
 
   // @ts-expect-error
-  const listener = new BrowserSignalListener(syncManagerMockWithPushManager, fullSettings, fakeStorageDebug as IStorageSync, fakeSplitApi);
+  const listener = new BrowserSignalListener(syncManagerMock, fullSettings, fakeStorageDebug as IStorageSync, fakeSplitApi);
 
   listener.start();
   assertStart(listener);
 
   triggerEvent(VISIBILITYCHANGE_EVENT, 'visible');
 
   // If visibility changes to visible, do nothing
-  expect(syncManagerMockWithPushManager.pushManager.stop).not.toBeCalled();
   expect(global.window.navigator.sendBeacon).not.toBeCalled();
 
   triggerEvent(PAGEHIDE_EVENT);
 
   // Pagehide event was triggered. Thus sendBeacon method should be called twice.
   expect(global.window.navigator.sendBeacon).toBeCalledTimes(2);
-  expect(syncManagerMockWithPushManager.pushManager.stop).not.toBeCalled();
-
-  triggerEvent(UNLOAD_EVENT);
-
-  // Unload event was triggered and pushManager is defined, so it must be stopped.
-  expect(syncManagerMockWithPushManager.pushManager.stop).toBeCalledTimes(1);
-  expect(global.window.navigator.sendBeacon).toBeCalledTimes(2);
 
   // Http post services should have not been called
   expect(fakeSplitApi.postTestImpressionsBulk).not.toBeCalled();

src/listeners/browser.ts

@@ -15,8 +15,7 @@ import { isConsentGranted } from '../consent';
 
 const VISIBILITYCHANGE_EVENT = 'visibilitychange';
 const PAGEHIDE_EVENT = 'pagehide';
-const UNLOAD_EVENT = 'unload';
-const EVENT_NAME = 'for unload page event.';
+const EVENT_NAME = 'for visibilitychange and pagehide events.';
 
 /**
  * We'll listen for events over the window object.
@@ -33,7 +32,6 @@ export class BrowserSignalListener implements ISignalListener {
   ) {
     this.flushData = this.flushData.bind(this);
     this.flushDataIfHidden = this.flushDataIfHidden.bind(this);
-    this.stopSync = this.stopSync.bind(this);
     this.fromImpressionsCollector = fromImpressionsCollector.bind(undefined, settings.core.labelsEnabled);
   }
 
@@ -48,11 +46,9 @@ export class BrowserSignalListener implements ISignalListener {
       document.addEventListener(VISIBILITYCHANGE_EVENT, this.flushDataIfHidden);
     }
     if (typeof window !== 'undefined' && window.addEventListener) {
-      // Some browsers like Safari does not fire the `visibilitychange` event when the page is being unloaded. So we also flush data in the `pagehide` event.
-      // If both events are triggered, the last one will find the storage empty, so no duplicated data will be submitted.
+      // Some browsers, like Safari, does not fire the `visibilitychange` event when the page is being unloaded. Therefore, we also flush data in the `pagehide` event.
+      // If both events are triggered, the latter will find the storage empty, so no duplicate data will be submitted.
       window.addEventListener(PAGEHIDE_EVENT, this.flushData);
-      // Stop streaming on 'unload' event. Used instead of 'beforeunload', because 'unload' is not a cancelable event, so no other listeners can stop the event from occurring.
-      window.addEventListener(UNLOAD_EVENT, this.stopSync);
     }
   }
 
@@ -67,15 +63,9 @@ export class BrowserSignalListener implements ISignalListener {
     }
     if (typeof window !== 'undefined' && window.removeEventListener) {
       window.removeEventListener(PAGEHIDE_EVENT, this.flushData);
-      window.removeEventListener(UNLOAD_EVENT, this.stopSync);
     }
   }
 
-  stopSync() {
-    // Close streaming connection
-    if (this.syncManager && this.syncManager.pushManager) this.syncManager.pushManager.stop();
-  }
-
   /**
    * flushData method.
    * Called when pagehide event is triggered. It flushed remaining impressions and events to the backend,