From 8d14bfc77bb7a7c98fd82309b9cd9a4e990f80bd Mon Sep 17 00:00:00 2001 From: Xinyi Ye Date: Fri, 12 Jan 2024 15:59:07 -0800 Subject: [PATCH] feat: offline mode Move offline logic into a plugin --- .../analytics-browser/src/browser-client.ts | 23 ++----- packages/analytics-browser/src/config.ts | 2 +- .../src/plugins/network-checker-plugin.ts | 22 ------- .../plugins/network-connectivity-checker.ts | 61 +++++++++++++++++++ .../test/browser-client.test.ts | 25 ++++++-- .../network-connectivity-checker.test.ts | 46 ++++++++++++++ packages/analytics-core/src/config.ts | 4 +- packages/analytics-types/src/config/core.ts | 2 +- 8 files changed, 136 insertions(+), 49 deletions(-) delete mode 100644 packages/analytics-browser/src/plugins/network-checker-plugin.ts create mode 100644 packages/analytics-browser/src/plugins/network-connectivity-checker.ts create mode 100644 packages/analytics-browser/test/plugins/network-connectivity-checker.test.ts diff --git a/packages/analytics-browser/src/browser-client.ts b/packages/analytics-browser/src/browser-client.ts index fec3f8737..895e375fc 100644 --- a/packages/analytics-browser/src/browser-client.ts +++ b/packages/analytics-browser/src/browser-client.ts @@ -1,6 +1,5 @@ import { AmplitudeCore, Destination, Identify, returnWrapper, Revenue, UUID } from '@amplitude/analytics-core'; import { - getGlobalScope, getAnalyticsConnector, getAttributionTrackingConfig, getPageViewTrackingConfig, @@ -32,7 +31,7 @@ import { formInteractionTracking } from './plugins/form-interaction-tracking'; import { fileDownloadTracking } from './plugins/file-download-tracking'; import { DEFAULT_SESSION_END_EVENT, DEFAULT_SESSION_START_EVENT } from './constants'; import { detNotify } from './det-notification'; -import { NetworkCheckerPlugin } from './plugins/network-checker-plugin'; +import { networkConnectivityCheckerPlugin } from './plugins/network-connectivity-checker'; export class AmplitudeBrowser extends AmplitudeCore implements BrowserClient { // eslint-disable-next-line @typescript-eslint/ban-ts-comment @@ -88,7 +87,9 @@ export class AmplitudeBrowser extends AmplitudeCore implements BrowserClient { // Step 4: Install plugins // Do not track any events before this - await this.add(new NetworkCheckerPlugin()).promise; + if (this.config.offline !== null) { + await this.add(networkConnectivityCheckerPlugin()).promise; + } await this.add(new Destination()).promise; await this.add(new Context()).promise; await this.add(new IdentityEventSender()).promise; @@ -123,22 +124,6 @@ export class AmplitudeBrowser extends AmplitudeCore implements BrowserClient { connector.eventBridge.setEventReceiver((event) => { void this.track(event.eventType, event.eventProperties); }); - - // Step 8: Add network listeners for offline mode - const globalScope = getGlobalScope(); - if (globalScope) { - globalScope.addEventListener('online', () => { - this.config.offline = false; - // Flush immediately cause ERR_NETWORK_CHANGED - setTimeout(() => { - this.flush(); - }, this.config.flushIntervalMillis); - }); - - globalScope.addEventListener('offline', () => { - this.config.offline = true; - }); - } } getUserId() { diff --git a/packages/analytics-browser/src/config.ts b/packages/analytics-browser/src/config.ts index 04e05d2f8..a0b473394 100644 --- a/packages/analytics-browser/src/config.ts +++ b/packages/analytics-browser/src/config.ts @@ -60,7 +60,7 @@ export class BrowserConfig extends Config implements IBrowserConfig { public loggerProvider: ILogger = new Logger(), public logLevel: LogLevel = LogLevel.Warn, public minIdLength?: number, - public offline = false, + public offline: boolean | null = false, optOut = false, public partnerId?: string, public plan?: Plan, diff --git a/packages/analytics-browser/src/plugins/network-checker-plugin.ts b/packages/analytics-browser/src/plugins/network-checker-plugin.ts deleted file mode 100644 index a8ec5c1b7..000000000 --- a/packages/analytics-browser/src/plugins/network-checker-plugin.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { BeforePlugin, Event } from '@amplitude/analytics-types'; -import { BrowserConfig } from 'src/config'; - -export class NetworkCheckerPlugin implements BeforePlugin { - name = '@amplitude/plugin-network-checker-browser'; - type = 'before' as const; - - // this.config is defined in setup() which will always be called first - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - config: BrowserConfig; - - setup(config: BrowserConfig): Promise { - this.config = config; - return Promise.resolve(undefined); - } - - async execute(context: Event): Promise { - this.config.offline = !navigator.onLine; - return context; - } -} diff --git a/packages/analytics-browser/src/plugins/network-connectivity-checker.ts b/packages/analytics-browser/src/plugins/network-connectivity-checker.ts new file mode 100644 index 000000000..fb02c9045 --- /dev/null +++ b/packages/analytics-browser/src/plugins/network-connectivity-checker.ts @@ -0,0 +1,61 @@ +import { getGlobalScope } from '@amplitude/analytics-client-common'; +import { BeforePlugin, BrowserClient } from '@amplitude/analytics-types'; +import { BrowserConfig } from 'src/config'; + +interface EventListener { + type: 'online' | 'offline'; + handler: () => void; +} + +export const networkConnectivityCheckerPlugin = (): BeforePlugin => { + const name = '@amplitude/plugin-network-checker-browser'; + const type = 'before' as const; + const globalScope = getGlobalScope(); + let eventListeners: EventListener[] = []; + + const addNetworkListener = (type: 'online' | 'offline', handler: () => void) => { + if (globalScope) { + globalScope.addEventListener(type, handler); + eventListeners.push({ + type, + handler, + }); + } + }; + + const removeNetworkListeners = () => { + eventListeners.forEach(({ type, handler }) => { + if (globalScope) { + globalScope.removeEventListener(type, handler); + } + }); + eventListeners = []; + }; + + const setup = async (config: BrowserConfig, amplitude: BrowserClient) => { + config.offline = !navigator.onLine; + + addNetworkListener('online', () => { + config.offline = false; + // Flush immediately will cause ERR_NETWORK_CHANGED + setTimeout(() => { + amplitude.flush(); + }, config.flushIntervalMillis); + }); + + addNetworkListener('offline', () => { + config.offline = true; + }); + }; + + const teardown = async () => { + removeNetworkListeners(); + }; + + return { + name, + type, + setup, + teardown, + }; +}; diff --git a/packages/analytics-browser/test/browser-client.test.ts b/packages/analytics-browser/test/browser-client.test.ts index 538c18825..87df8ca44 100644 --- a/packages/analytics-browser/test/browser-client.test.ts +++ b/packages/analytics-browser/test/browser-client.test.ts @@ -14,6 +14,7 @@ import * as AnalyticsClientCommon from '@amplitude/analytics-client-common'; import * as fileDownloadTracking from '../src/plugins/file-download-tracking'; import * as formInteractionTracking from '../src/plugins/form-interaction-tracking'; import * as webAttributionPlugin from '@amplitude/plugin-web-attribution-browser'; +import * as networkConnectivityChecker from '../src/plugins/network-connectivity-checker'; describe('browser-client', () => { let apiKey = ''; @@ -263,10 +264,29 @@ describe('browser-client', () => { expect(webAttributionPluginPlugin).toHaveBeenCalledTimes(1); }); + test('should add network connectivity checker plugin by default', async () => { + const networkConnectivityCheckerPlugin = jest.spyOn( + networkConnectivityChecker, + 'networkConnectivityCheckerPlugin', + ); + await client.init(apiKey, userId).promise; + expect(networkConnectivityCheckerPlugin).toHaveBeenCalledTimes(1); + }); + + test('should not add network connectivity checker plugin if offline is null', async () => { + const networkConnectivityCheckerPlugin = jest.spyOn( + networkConnectivityChecker, + 'networkConnectivityCheckerPlugin', + ); + await client.init(apiKey, userId, { + offline: null, + }).promise; + expect(networkConnectivityCheckerPlugin).toHaveBeenCalledTimes(0); + }); + test('should listen for network change to online', async () => { jest.useFakeTimers(); const addEventListenerMock = jest.spyOn(window, 'addEventListener'); - // const setTimeoutSpy = jest.spyOn(window, 'setTimeout'); const flush = jest.spyOn(client, 'flush').mockReturnValue({ promise: Promise.resolve() }); await client.init(apiKey, { @@ -276,15 +296,12 @@ describe('browser-client', () => { expect(addEventListenerMock).toHaveBeenCalledWith('online', expect.any(Function)); expect(client.config.offline).toBe(false); - // expect(setTimeoutSpy).toHaveBeenCalledTimes(1); - // expect(setTimeoutSpy).toHaveBeenLastCalledWith(expect.any(Function), client.config.flushIntervalMillis); jest.advanceTimersByTime(client.config.flushIntervalMillis); expect(flush).toHaveBeenCalledTimes(1); jest.useRealTimers(); addEventListenerMock.mockRestore(); - // setTimeoutSpy.mockRestore(); flush.mockRestore(); }); diff --git a/packages/analytics-browser/test/plugins/network-connectivity-checker.test.ts b/packages/analytics-browser/test/plugins/network-connectivity-checker.test.ts new file mode 100644 index 000000000..ff0387675 --- /dev/null +++ b/packages/analytics-browser/test/plugins/network-connectivity-checker.test.ts @@ -0,0 +1,46 @@ +/* eslint-disable @typescript-eslint/unbound-method */ + +import { createAmplitudeMock, createConfigurationMock } from '../helpers/mock'; +import { networkConnectivityCheckerPlugin } from '../../src/plugins/network-connectivity-checker'; + +describe('networkConnectivityCheckerPlugin', () => { + const amplitude = createAmplitudeMock(); + const config = createConfigurationMock(); + + test('should set up correctly when online', async () => { + const plugin = networkConnectivityCheckerPlugin(); + jest.spyOn(navigator, 'onLine', 'get').mockReturnValue(true); + const addEventListenerSpy = jest.spyOn(window, 'addEventListener'); + + await plugin.setup?.(config, amplitude); + + expect(config.offline).toEqual(false); + expect(addEventListenerSpy).toHaveBeenCalledWith('online', expect.any(Function)); + expect(addEventListenerSpy).toHaveBeenCalledWith('offline', expect.any(Function)); + addEventListenerSpy.mockRestore(); + }); + + test('should set up correctly when offline', async () => { + const plugin = networkConnectivityCheckerPlugin(); + jest.spyOn(navigator, 'onLine', 'get').mockReturnValue(false); + const addEventListenerSpy = jest.spyOn(window, 'addEventListener'); + + await plugin.setup?.(config, amplitude); + + expect(config.offline).toEqual(true); + expect(addEventListenerSpy).toHaveBeenCalledWith('online', expect.any(Function)); + expect(addEventListenerSpy).toHaveBeenCalledWith('offline', expect.any(Function)); + addEventListenerSpy.mockRestore(); + }); + + test('should teardown plugin', async () => { + const plugin = networkConnectivityCheckerPlugin(); + const removeEventListenerSpy = jest.spyOn(window, 'removeEventListener'); + + await plugin.setup?.(createConfigurationMock(), amplitude); + await plugin.teardown?.(); + + expect(removeEventListenerSpy).toHaveBeenCalledWith('online', expect.any(Function)); + expect(removeEventListenerSpy).toHaveBeenCalledWith('offline', expect.any(Function)); + }); +}); diff --git a/packages/analytics-core/src/config.ts b/packages/analytics-core/src/config.ts index 2c586c78a..12cca3363 100644 --- a/packages/analytics-core/src/config.ts +++ b/packages/analytics-core/src/config.ts @@ -42,7 +42,7 @@ export class Config implements IConfig { loggerProvider: ILogger; logLevel: LogLevel; minIdLength?: number; - offline: boolean; + offline?: boolean | null; plan?: Plan; ingestionMetadata?: IngestionMetadata; serverUrl: string | undefined; @@ -71,7 +71,7 @@ export class Config implements IConfig { this.minIdLength = options.minIdLength; this.plan = options.plan; this.ingestionMetadata = options.ingestionMetadata; - this.offline = options.offline ?? defaultConfig.offline; + this.offline = options.offline !== undefined ? options.offline : defaultConfig.offline; this.optOut = options.optOut ?? defaultConfig.optOut; this.serverUrl = options.serverUrl; this.serverZone = options.serverZone || defaultConfig.serverZone; diff --git a/packages/analytics-types/src/config/core.ts b/packages/analytics-types/src/config/core.ts index 5fe79ea4f..72648d607 100644 --- a/packages/analytics-types/src/config/core.ts +++ b/packages/analytics-types/src/config/core.ts @@ -15,7 +15,7 @@ export interface Config { logLevel: LogLevel; loggerProvider: Logger; minIdLength?: number; - offline: boolean; + offline?: boolean | null; optOut: boolean; plan?: Plan; ingestionMetadata?: IngestionMetadata;