fix unload detection (#613)

Co-authored-by: @agouil

Author: silesky

.changeset/lucky-camels-bathe.md

@@ -0,0 +1,5 @@
+---
+'@segment/analytics-next': patch
+---
+
+fix change detection bug and add ability to detect tab focus loss events

packages/browser/package.json

@@ -43,7 +43,7 @@
   "size-limit": [
     {
       "path": "dist/umd/index.js",
-      "limit": "25.95 KB"
+      "limit": "26.02 KB"
     }
   ],
   "dependencies": {

packages/browser/src/lib/__tests__/on-page-leave.test.ts

@@ -0,0 +1,64 @@
+import { onPageLeave } from '../on-page-leave'
+
+const helpers = {
+  dispatchEvent(event: 'pagehide' | 'visibilitychange') {
+    document.dispatchEvent(new Event(event))
+  },
+  setVisibilityState(state: DocumentVisibilityState) {
+    Object.defineProperty(document, 'visibilityState', {
+      value: state,
+      writable: true,
+    })
+  },
+  dispatchVisChangeEvent(state: DocumentVisibilityState) {
+    helpers.setVisibilityState(state)
+    helpers.dispatchEvent('visibilitychange')
+  },
+  dispatchPageHideEvent() {
+    helpers.dispatchEvent('pagehide')
+  },
+}
+
+beforeEach(() => {
+  helpers.setVisibilityState('visible')
+})
+
+describe('onPageLeave', () => {
+  test('callback should fire on pagehide', () => {
+    const cb = jest.fn()
+    onPageLeave(cb)
+    helpers.dispatchPageHideEvent()
+    expect(cb).toBeCalledTimes(1)
+  })
+
+  test('callback should fire if document becomes hidden', () => {
+    const cb = jest.fn()
+    onPageLeave(cb)
+    helpers.dispatchVisChangeEvent('hidden')
+    expect(cb).toBeCalledTimes(1)
+  })
+
+  test('callback should *not* fire if document becomes visible', () => {
+    const cb = jest.fn()
+    onPageLeave(cb)
+    helpers.dispatchVisChangeEvent('visible')
+    expect(cb).not.toBeCalled()
+  })
+
+  test('if both event handlers fire, callback should still fire only once', () => {
+    const cb = jest.fn()
+    onPageLeave(cb)
+    helpers.dispatchVisChangeEvent('hidden')
+    helpers.dispatchPageHideEvent()
+    expect(cb).toBeCalledTimes(1)
+  })
+
+  test('if user leaves a tab, returns, and leaves again, callback should be called on each departure', () => {
+    const cb = jest.fn()
+    onPageLeave(cb)
+    helpers.dispatchVisChangeEvent('hidden')
+    helpers.dispatchVisChangeEvent('visible')
+    helpers.dispatchVisChangeEvent('hidden')
+    expect(cb).toBeCalledTimes(2)
+  })
+})

packages/browser/src/lib/on-page-leave.ts

@@ -0,0 +1,29 @@
+/**
+ * Register event listener on document that fires when:
+ * * tab change or tab close (in mobile or desktop)
+ * * click back / forward button
+ * * click any link or perform any other navigation action
+ * * soft refresh / hard refresh
+ *
+ * adapted from https://stackoverflow.com/questions/3239834/window-onbeforeunload-not-working-on-the-ipad/52864508#52864508,
+ */
+export const onPageLeave = (cb: (...args: unknown[]) => void) => {
+  let unloaded = false // prevents double firing if both are supported
+
+  // using document instead of window because of bug affecting browsers before safari 14 (detail in footnotes https://caniuse.com/?search=visibilitychange)
+  document.addEventListener('pagehide', () => {
+    if (unloaded) return
+    unloaded = true
+    cb()
+  })
+
+  document.addEventListener('visibilitychange', () => {
+    if (document.visibilityState == 'hidden') {
+      if (unloaded) return
+      unloaded = true
+      cb()
+    } else {
+      unloaded = false
+    }
+  })
+}

packages/browser/src/lib/priority-queue/__tests__/persisted.test.ts

@@ -41,9 +41,9 @@ describe('Persisted Priority Queue', () => {
     let onUnload: any = jest.fn()
 
     jest
-      .spyOn(window, 'addEventListener')
+      .spyOn(document, 'addEventListener')
       .mockImplementation((evt, handler) => {
-        if (evt === 'beforeunload') {
+        if (evt === 'pagehide') {
           onUnload = handler
         }
       })
@@ -102,9 +102,9 @@ describe('Persisted Priority Queue', () => {
       let onUnload: any = jest.fn()
 
       jest
-        .spyOn(window, 'addEventListener')
+        .spyOn(document, 'addEventListener')
         .mockImplementation((evt, handler) => {
-          if (evt === 'beforeunload') {
+          if (evt === 'pagehide') {
             onUnload = handler
           }
         })
@@ -200,9 +200,9 @@ describe('Persisted Priority Queue', () => {
       const onUnloadFunctions: any[] = []
 
       jest
-        .spyOn(window, 'addEventListener')
+        .spyOn(document, 'addEventListener')
         .mockImplementation((evt, handler) => {
-          if (evt === 'beforeunload') {
+          if (evt === 'pagehide') {
             onUnloadFunctions.push(handler)
           }
         })

packages/browser/src/lib/priority-queue/persisted.ts

@@ -109,7 +109,8 @@ export class PersistedPriorityQueue extends PriorityQueue<Context> {
       }
     })
 
-    window.addEventListener('beforeunload', () => {
+    document.addEventListener('pagehide', () => {
+      // we deliberately want to use the less powerful 'pagehide' API to only persist on events where the analytics instance gets destroyed, and not on tab away.
       if (this.todo > 0) {
         const items = [...this.queue, ...this.future]
         try {

packages/browser/src/plugins/segmentio/__tests__/batched-dispatcher.test.ts

@@ -224,7 +224,7 @@ describe('Batching', () => {
 
       expect(fetch).not.toHaveBeenCalled()
 
-      window.dispatchEvent(new Event('beforeunload'))
+      document.dispatchEvent(new Event('pagehide'))
 
       expect(fetch).toHaveBeenCalledTimes(1)
 
@@ -253,7 +253,7 @@ describe('Batching', () => {
 
       expect(fetch).not.toHaveBeenCalled()
 
-      window.dispatchEvent(new Event('beforeunload'))
+      document.dispatchEvent(new Event('pagehide'))
 
       expect(fetch).toHaveBeenCalledTimes(2)
     })

packages/browser/src/plugins/segmentio/batched-dispatcher.ts

@@ -1,5 +1,6 @@
 import unfetch from 'unfetch'
 import { SegmentEvent } from '../../core/events'
+import { onPageLeave } from '../../lib/on-page-leave'
 
 let fetch = unfetch
 if (typeof window !== 'undefined') {
@@ -92,7 +93,7 @@ export default function batch(apiHost: string, config?: BatchingConfig) {
     }, timeout)
   }
 
-  window.addEventListener('beforeunload', () => {
+  onPageLeave(() => {
     pageUnloaded = true
 
     if (buffer.length) {

packages/core/src/priority-queue/persisted.ts

@@ -1,6 +1,7 @@
 import { PriorityQueue } from '.'
 import { CoreContext, SerializedContext } from '../context'
 
+// TODO: Move this back into browser
 const nullStorage = (): Storage => ({
   getItem: () => null,
   setItem: () => null,
@@ -118,8 +119,7 @@ export class PersistedPriorityQueue extends PriorityQueue<CoreContext> {
         console.error(err)
       }
     })
-
-    window.addEventListener('beforeunload', () => {
+    window.addEventListener('pagehide', () => {
       if (this.todo > 0) {
         const items = [...this.queue, ...this.future]
         try {