From 4ed0d3371572601b4f37e4d1f943ef5ea561aa04 Mon Sep 17 00:00:00 2001 From: Daniel Jackins Date: Mon, 31 Jul 2023 21:00:08 -0600 Subject: [PATCH 1/5] track navigation status properly --- ...e-leave.test.ts => on-page-change.test.ts} | 31 ++++++++++++------- .../{on-page-leave.ts => on-page-change.ts} | 6 ++-- .../plugins/segmentio/batched-dispatcher.ts | 8 ++--- 3 files changed, 27 insertions(+), 18 deletions(-) rename packages/browser/src/lib/__tests__/{on-page-leave.test.ts => on-page-change.test.ts} (68%) rename packages/browser/src/lib/{on-page-leave.ts => on-page-change.ts} (89%) diff --git a/packages/browser/src/lib/__tests__/on-page-leave.test.ts b/packages/browser/src/lib/__tests__/on-page-change.test.ts similarity index 68% rename from packages/browser/src/lib/__tests__/on-page-leave.test.ts rename to packages/browser/src/lib/__tests__/on-page-change.test.ts index f2fb026f7..171f4c191 100644 --- a/packages/browser/src/lib/__tests__/on-page-leave.test.ts +++ b/packages/browser/src/lib/__tests__/on-page-change.test.ts @@ -1,4 +1,4 @@ -import { onPageLeave } from '../on-page-leave' +import { onPageChange } from '../on-page-change' const helpers = { dispatchEvent(event: 'pagehide' | 'visibilitychange') { @@ -24,42 +24,51 @@ beforeEach(() => { helpers.setVisibilityState('visible') }) -describe('onPageLeave', () => { +describe('onPageChange', () => { test('callback should fire on pagehide', () => { const cb = jest.fn() - onPageLeave(cb) + onPageChange(cb) helpers.dispatchPageHideEvent() expect(cb).toBeCalledTimes(1) }) test('callback should fire if document becomes hidden', () => { const cb = jest.fn() - onPageLeave(cb) + onPageChange(cb) helpers.dispatchVisChangeEvent('hidden') expect(cb).toBeCalledTimes(1) }) - test('callback should *not* fire if document becomes visible', () => { + test('callback should fire if document becomes visible', () => { const cb = jest.fn() - onPageLeave(cb) + onPageChange(cb) helpers.dispatchVisChangeEvent('visible') - expect(cb).not.toBeCalled() + expect(cb).toBeCalledTimes(1) }) test('if both event handlers fire, callback should still fire only once', () => { const cb = jest.fn() - onPageLeave(cb) + onPageChange(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', () => { + test('if user leaves a tab, returns, and leaves again, callback should be called on each navigation', () => { const cb = jest.fn() - onPageLeave(cb) + onPageChange(cb) helpers.dispatchVisChangeEvent('hidden') helpers.dispatchVisChangeEvent('visible') helpers.dispatchVisChangeEvent('hidden') - expect(cb).toBeCalledTimes(2) + expect(cb).toBeCalledTimes(3) + }) + + test('if user navigates, callback should be passed appropriate "unloaded" value', () => { + const cb = jest.fn() + onPageChange(cb) + helpers.dispatchVisChangeEvent('hidden') + expect(cb).toBeCalledWith(true) + helpers.dispatchVisChangeEvent('visible') + expect(cb).toBeCalledWith(false) }) }) diff --git a/packages/browser/src/lib/on-page-leave.ts b/packages/browser/src/lib/on-page-change.ts similarity index 89% rename from packages/browser/src/lib/on-page-leave.ts rename to packages/browser/src/lib/on-page-change.ts index 1c12eb70f..7610e9cbe 100644 --- a/packages/browser/src/lib/on-page-leave.ts +++ b/packages/browser/src/lib/on-page-change.ts @@ -7,13 +7,13 @@ * * adapted from https://stackoverflow.com/questions/3239834/window-onbeforeunload-not-working-on-the-ipad/52864508#52864508, */ -export const onPageLeave = (cb: (...args: unknown[]) => void) => { +export const onPageChange = (cb: (...args: boolean[]) => void) => { let unloaded = false // prevents double firing if both are supported window.addEventListener('pagehide', () => { if (unloaded) return unloaded = true - cb() + cb(unloaded) }) // using document instead of window because of bug affecting browsers before safari 14 (detail in footnotes https://caniuse.com/?search=visibilitychange) @@ -21,9 +21,9 @@ export const onPageLeave = (cb: (...args: unknown[]) => void) => { if (document.visibilityState == 'hidden') { if (unloaded) return unloaded = true - cb() } else { unloaded = false } + cb(unloaded) }) } diff --git a/packages/browser/src/plugins/segmentio/batched-dispatcher.ts b/packages/browser/src/plugins/segmentio/batched-dispatcher.ts index 4a0193642..541d8c91a 100644 --- a/packages/browser/src/plugins/segmentio/batched-dispatcher.ts +++ b/packages/browser/src/plugins/segmentio/batched-dispatcher.ts @@ -1,6 +1,6 @@ import { SegmentEvent } from '../../core/events' import { fetch } from '../../lib/fetch' -import { onPageLeave } from '../../lib/on-page-leave' +import { onPageChange } from '../../lib/on-page-change' export type BatchingDispatchConfig = { size?: number @@ -91,10 +91,10 @@ export default function batch( }, timeout) } - onPageLeave(() => { - pageUnloaded = true + onPageChange((unloaded: boolean) => { + pageUnloaded = unloaded - if (buffer.length) { + if (pageUnloaded && buffer.length) { const reqs = chunks(buffer).map(sendBatch) Promise.all(reqs).catch(console.error) } From 63c548e5d8dfb90e1721a9dd797687ac5134d218 Mon Sep 17 00:00:00 2001 From: Daniel Jackins Date: Mon, 31 Jul 2023 21:09:12 -0600 Subject: [PATCH 2/5] changeset --- .changeset/orange-actors-hug.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/orange-actors-hug.md diff --git a/.changeset/orange-actors-hug.md b/.changeset/orange-actors-hug.md new file mode 100644 index 000000000..2d444d9c6 --- /dev/null +++ b/.changeset/orange-actors-hug.md @@ -0,0 +1,5 @@ +--- +'@segment/analytics-next': patch +--- + +Fix batching after page navigation From 33b11fb90f4666c1e5e9ed98ef0ded6af30d73eb Mon Sep 17 00:00:00 2001 From: Daniel Jackins Date: Mon, 31 Jul 2023 21:10:59 -0600 Subject: [PATCH 3/5] fix checkmark formatting --- .github/PULL_REQUEST_TEMPLATE | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE b/.github/PULL_REQUEST_TEMPLATE index 601e5799f..855edcdf0 100644 --- a/.github/PULL_REQUEST_TEMPLATE +++ b/.github/PULL_REQUEST_TEMPLATE @@ -8,5 +8,4 @@ Hello! And thanks for contributing to the Analytics-Next 🎉 Also make sure to describe how you tested this change, include any gifs or screenshots you find necessary. ---> - -[] I've included a changeset (psst. run `yarn changeset`. Read about changesets [here](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md)). \ No newline at end of file +[ ] I've included a changeset (psst. run `yarn changeset`. Read about changesets [here](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md)). From d4dca8d1d3b5992c772f99851d01605162cb333e Mon Sep 17 00:00:00 2001 From: Daniel Jackins Date: Mon, 31 Jul 2023 21:12:16 -0600 Subject: [PATCH 4/5] try again --- .github/PULL_REQUEST_TEMPLATE | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/PULL_REQUEST_TEMPLATE b/.github/PULL_REQUEST_TEMPLATE index 855edcdf0..d82b264b9 100644 --- a/.github/PULL_REQUEST_TEMPLATE +++ b/.github/PULL_REQUEST_TEMPLATE @@ -8,4 +8,4 @@ Hello! And thanks for contributing to the Analytics-Next 🎉 Also make sure to describe how you tested this change, include any gifs or screenshots you find necessary. ---> -[ ] I've included a changeset (psst. run `yarn changeset`. Read about changesets [here](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md)). +- [ ] I've included a changeset (psst. run `yarn changeset`. Read about changesets [here](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md)). From 92e118c5e8056a02f7bfca7aa126aa9d86f5f552 Mon Sep 17 00:00:00 2001 From: Daniel Jackins Date: Tue, 1 Aug 2023 11:09:28 -0600 Subject: [PATCH 5/5] fix types --- packages/browser/src/lib/on-page-change.ts | 2 +- packages/browser/src/plugins/segmentio/batched-dispatcher.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/browser/src/lib/on-page-change.ts b/packages/browser/src/lib/on-page-change.ts index 7610e9cbe..da4194c6f 100644 --- a/packages/browser/src/lib/on-page-change.ts +++ b/packages/browser/src/lib/on-page-change.ts @@ -7,7 +7,7 @@ * * adapted from https://stackoverflow.com/questions/3239834/window-onbeforeunload-not-working-on-the-ipad/52864508#52864508, */ -export const onPageChange = (cb: (...args: boolean[]) => void) => { +export const onPageChange = (cb: (unloaded: boolean) => void) => { let unloaded = false // prevents double firing if both are supported window.addEventListener('pagehide', () => { diff --git a/packages/browser/src/plugins/segmentio/batched-dispatcher.ts b/packages/browser/src/plugins/segmentio/batched-dispatcher.ts index 541d8c91a..35995d115 100644 --- a/packages/browser/src/plugins/segmentio/batched-dispatcher.ts +++ b/packages/browser/src/plugins/segmentio/batched-dispatcher.ts @@ -91,7 +91,7 @@ export default function batch( }, timeout) } - onPageChange((unloaded: boolean) => { + onPageChange((unloaded) => { pageUnloaded = unloaded if (pageUnloaded && buffer.length) {