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 diff --git a/.github/PULL_REQUEST_TEMPLATE b/.github/PULL_REQUEST_TEMPLATE index 601e5799f..d82b264b9 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)). 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 90% rename from packages/browser/src/lib/on-page-leave.ts rename to packages/browser/src/lib/on-page-change.ts index 1c12eb70f..da4194c6f 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: (unloaded: 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..35995d115 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) => { + pageUnloaded = unloaded - if (buffer.length) { + if (pageUnloaded && buffer.length) { const reqs = chunks(buffer).map(sendBatch) Promise.all(reqs).catch(console.error) }