From d9b47c43e5e08efce14fe4150536ff60b8df91e0 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Wed, 15 Nov 2023 15:07:34 -0600 Subject: [PATCH] Add emitter to generic utils (#993) --- .changeset/healthy-toes-confess.md | 4 +++ .changeset/stale-seals-impress.md | 5 +++ packages/browser/package.json | 3 +- packages/browser/src/browser/index.ts | 2 +- packages/browser/src/core/analytics/index.ts | 3 +- packages/browser/src/lib/create-deferred.ts | 16 --------- .../src/plugins/ajs-destination/types.ts | 2 +- packages/core/package.json | 1 + .../src/analytics/__tests__/dispatch.test.ts | 2 +- packages/core/src/analytics/dispatch.ts | 2 +- packages/core/src/index.ts | 1 - packages/core/src/priority-queue/index.ts | 2 +- packages/core/src/queue/event-queue.ts | 2 +- .../src/emitter/__tests__/emitter.test.ts | 34 ++++++++++++++++++- .../src/emitter/emitter.ts} | 33 ++++++++++++++++++ packages/generic-utils/src/emitter/index.ts | 1 + packages/generic-utils/src/index.ts | 1 + packages/node/src/app/emitter.ts | 3 +- packages/node/src/lib/abort.ts | 2 +- .../segmentio/__tests__/methods.test.ts | 2 +- .../segmentio/__tests__/publisher.test.ts | 2 +- yarn.lock | 2 ++ 22 files changed, 95 insertions(+), 30 deletions(-) create mode 100644 .changeset/healthy-toes-confess.md create mode 100644 .changeset/stale-seals-impress.md delete mode 100644 packages/browser/src/lib/create-deferred.ts rename packages/{core => generic-utils}/src/emitter/__tests__/emitter.test.ts (60%) rename packages/{core/src/emitter/index.ts => generic-utils/src/emitter/emitter.ts} (68%) create mode 100644 packages/generic-utils/src/emitter/index.ts diff --git a/.changeset/healthy-toes-confess.md b/.changeset/healthy-toes-confess.md new file mode 100644 index 000000000..1c6eb9ac1 --- /dev/null +++ b/.changeset/healthy-toes-confess.md @@ -0,0 +1,4 @@ +--- +'@segment/analytics-core': minor +--- +Consume Emitter module from `@segment/analytics-generic-utils` diff --git a/.changeset/stale-seals-impress.md b/.changeset/stale-seals-impress.md new file mode 100644 index 000000000..608a3a92b --- /dev/null +++ b/.changeset/stale-seals-impress.md @@ -0,0 +1,5 @@ +--- +'@segment/analytics-generic-utils': minor +--- + +Add Emitter library. Log default warning if a listeners exceeds 10 for a specific event type (configurable) diff --git a/packages/browser/package.json b/packages/browser/package.json index 2ffed79f9..ad673ca04 100644 --- a/packages/browser/package.json +++ b/packages/browser/package.json @@ -44,12 +44,13 @@ "size-limit": [ { "path": "dist/umd/index.js", - "limit": "29.1 KB" + "limit": "29.2 KB" } ], "dependencies": { "@lukeed/uuid": "^2.0.0", "@segment/analytics-core": "1.3.2", + "@segment/analytics-generic-utils": "1.0.0", "@segment/analytics.js-video-plugins": "^0.2.1", "@segment/facade": "^3.4.9", "@segment/tsub": "^2.0.0", diff --git a/packages/browser/src/browser/index.ts b/packages/browser/src/browser/index.ts index 39ac2ee83..2c9da9128 100644 --- a/packages/browser/src/browser/index.ts +++ b/packages/browser/src/browser/index.ts @@ -8,7 +8,7 @@ import { Plan } from '../core/events' import { Plugin } from '../core/plugin' import { MetricsOptions } from '../core/stats/remote-metrics' import { mergedOptions } from '../lib/merged-options' -import { createDeferred } from '../lib/create-deferred' +import { createDeferred } from '@segment/analytics-generic-utils' import { envEnrichment } from '../plugins/env-enrichment' import { PluginFactory, diff --git a/packages/browser/src/core/analytics/index.ts b/packages/browser/src/core/analytics/index.ts index 876b0eb81..db2f13d18 100644 --- a/packages/browser/src/core/analytics/index.ts +++ b/packages/browser/src/core/analytics/index.ts @@ -13,7 +13,8 @@ import { import type { FormArgs, LinkArgs } from '../auto-track' import { isOffline } from '../connection' import { Context } from '../context' -import { dispatch, Emitter } from '@segment/analytics-core' +import { dispatch } from '@segment/analytics-core' +import { Emitter } from '@segment/analytics-generic-utils' import { Callback, EventFactory, diff --git a/packages/browser/src/lib/create-deferred.ts b/packages/browser/src/lib/create-deferred.ts deleted file mode 100644 index 66c3b5d7a..000000000 --- a/packages/browser/src/lib/create-deferred.ts +++ /dev/null @@ -1,16 +0,0 @@ -/** - * Return a promise that can be externally resolved - */ -export const createDeferred = () => { - let resolve!: (value: T | PromiseLike) => void - let reject!: (reason: any) => void - const promise = new Promise((_resolve, _reject) => { - resolve = _resolve - reject = _reject - }) - return { - resolve, - reject, - promise, - } -} diff --git a/packages/browser/src/plugins/ajs-destination/types.ts b/packages/browser/src/plugins/ajs-destination/types.ts index 659f6c41f..a3b39d6f9 100644 --- a/packages/browser/src/plugins/ajs-destination/types.ts +++ b/packages/browser/src/plugins/ajs-destination/types.ts @@ -1,6 +1,6 @@ import { Group, Identify, Track, Page, Alias } from '@segment/facade' import { Analytics } from '../../core/analytics' -import { Emitter } from '@segment/analytics-core' +import { Emitter } from '@segment/analytics-generic-utils' import { User } from '../../core/user' export interface LegacyIntegration extends Emitter { diff --git a/packages/core/package.json b/packages/core/package.json index 31f164f60..08707c883 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -34,6 +34,7 @@ "packageManager": "yarn@3.4.1", "dependencies": { "@lukeed/uuid": "^2.0.0", + "@segment/analytics-generic-utils": "1.0.0", "dset": "^3.1.2", "tslib": "^2.4.1" } diff --git a/packages/core/src/analytics/__tests__/dispatch.test.ts b/packages/core/src/analytics/__tests__/dispatch.test.ts index bcef422d2..829facfb4 100644 --- a/packages/core/src/analytics/__tests__/dispatch.test.ts +++ b/packages/core/src/analytics/__tests__/dispatch.test.ts @@ -16,7 +16,7 @@ jest.mock('../../callback', () => ({ })) import { CoreEventQueue } from '../../queue/event-queue' -import { Emitter } from '../../emitter' +import { Emitter } from '@segment/analytics-generic-utils' import { dispatch, getDelay } from '../dispatch' import { CoreContext } from '../../context' import { TestCtx, TestEventQueue } from '../../../test-helpers' diff --git a/packages/core/src/analytics/dispatch.ts b/packages/core/src/analytics/dispatch.ts index 182af351f..ac17bfab3 100644 --- a/packages/core/src/analytics/dispatch.ts +++ b/packages/core/src/analytics/dispatch.ts @@ -2,7 +2,7 @@ import { CoreContext } from '../context' import { Callback } from '../events/interfaces' import { CoreEventQueue } from '../queue/event-queue' import { invokeCallback } from '../callback' -import { Emitter } from '../emitter' +import { Emitter } from '@segment/analytics-generic-utils' export type DispatchOptions = { timeout?: number diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 64195c34c..3193a5629 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -1,4 +1,3 @@ -export * from './emitter' export * from './emitter/interface' export * from './plugins' export * from './events/interfaces' diff --git a/packages/core/src/priority-queue/index.ts b/packages/core/src/priority-queue/index.ts index 49932ea11..3be5eccf2 100644 --- a/packages/core/src/priority-queue/index.ts +++ b/packages/core/src/priority-queue/index.ts @@ -1,4 +1,4 @@ -import { Emitter } from '../emitter' +import { Emitter } from '@segment/analytics-generic-utils' import { backoff } from './backoff' /** diff --git a/packages/core/src/queue/event-queue.ts b/packages/core/src/queue/event-queue.ts index 95e43ba08..81af660bc 100644 --- a/packages/core/src/queue/event-queue.ts +++ b/packages/core/src/queue/event-queue.ts @@ -3,7 +3,7 @@ import { groupBy } from '../utils/group-by' import { ON_REMOVE_FROM_FUTURE, PriorityQueue } from '../priority-queue' import { CoreContext, ContextCancelation } from '../context' -import { Emitter } from '../emitter' +import { Emitter } from '@segment/analytics-generic-utils' import { Integrations, JSONObject } from '../events/interfaces' import { CorePlugin } from '../plugins' import { createTaskGroup, TaskGroup } from '../task/task-group' diff --git a/packages/core/src/emitter/__tests__/emitter.test.ts b/packages/generic-utils/src/emitter/__tests__/emitter.test.ts similarity index 60% rename from packages/core/src/emitter/__tests__/emitter.test.ts rename to packages/generic-utils/src/emitter/__tests__/emitter.test.ts index b73855792..7edcb283c 100644 --- a/packages/core/src/emitter/__tests__/emitter.test.ts +++ b/packages/generic-utils/src/emitter/__tests__/emitter.test.ts @@ -1,4 +1,4 @@ -import { Emitter } from '../' +import { Emitter } from '../emitter' describe(Emitter, () => { it('emits events', () => { @@ -71,4 +71,36 @@ describe(Emitter, () => { expect(fn).toHaveBeenCalledTimes(2) }) + + it('has a default max listeners of 10', () => { + const em = new Emitter() + expect(em.maxListeners).toBe(10) + }) + + it('should warn if possible memory leak', () => { + const fn = jest.fn() + const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}) + const em = new Emitter({ maxListeners: 3 }) + em.on('test', fn) + em.on('test', fn) + em.on('test', fn) + expect(warnSpy).not.toHaveBeenCalled() + // call on 4th + em.on('test', fn) + expect(warnSpy).toHaveBeenCalledTimes(1) + // do not call additional times + em.on('test', fn) + expect(warnSpy).toHaveBeenCalledTimes(1) + }) + + it('has no warning if listener limit is set to 0', () => { + const fn = jest.fn() + const warnSpy = jest.spyOn(console, 'warn') + const em = new Emitter({ maxListeners: 0 }) + expect(em.maxListeners).toBe(0) + for (let i = 0; i++; i < 20) { + em.on('test', fn) + } + expect(warnSpy).not.toHaveBeenCalled() + }) }) diff --git a/packages/core/src/emitter/index.ts b/packages/generic-utils/src/emitter/emitter.ts similarity index 68% rename from packages/core/src/emitter/index.ts rename to packages/generic-utils/src/emitter/emitter.ts index d681b20f4..f5b331be9 100644 --- a/packages/core/src/emitter/index.ts +++ b/packages/generic-utils/src/emitter/emitter.ts @@ -2,6 +2,13 @@ type EventName = string type EventFnArgs = any[] type EmitterContract = Record +export interface EmitterOptions { + /** How many event listeners for a particular event before emitting a warning (0 = disabled) + * @default 10 + **/ + maxListeners?: number +} + /** * Event Emitter that takes the expected contract as a generic * @example @@ -16,7 +23,32 @@ type EmitterContract = Record * ``` */ export class Emitter { + maxListeners: number + constructor(options?: EmitterOptions) { + this.maxListeners = options?.maxListeners ?? 10 + } private callbacks: Partial = {} + private warned = false + + private warnIfPossibleMemoryLeak( + event: EventName + ) { + if (this.warned) { + return + } + if ( + this.maxListeners && + this.callbacks[event]!.length > this.maxListeners + ) { + console.warn( + `Event Emitter: Possible memory leak detected; ${String( + event + )} has exceeded ${this.maxListeners} listeners.` + ) + this.warned = true + } + } + on( event: EventName, callback: (...args: Contract[EventName]) => void @@ -25,6 +57,7 @@ export class Emitter { this.callbacks[event] = [callback] as Contract[EventName] } else { this.callbacks[event]!.push(callback) + this.warnIfPossibleMemoryLeak(event) } return this } diff --git a/packages/generic-utils/src/emitter/index.ts b/packages/generic-utils/src/emitter/index.ts new file mode 100644 index 000000000..9b03d6cc3 --- /dev/null +++ b/packages/generic-utils/src/emitter/index.ts @@ -0,0 +1 @@ +export * from './emitter' diff --git a/packages/generic-utils/src/index.ts b/packages/generic-utils/src/index.ts index 0a6560caf..8e99205a4 100644 --- a/packages/generic-utils/src/index.ts +++ b/packages/generic-utils/src/index.ts @@ -1 +1,2 @@ export * from './create-deferred' +export * from './emitter' diff --git a/packages/node/src/app/emitter.ts b/packages/node/src/app/emitter.ts index b1d9abf2f..1925d30b3 100644 --- a/packages/node/src/app/emitter.ts +++ b/packages/node/src/app/emitter.ts @@ -1,4 +1,5 @@ -import { CoreEmitterContract, Emitter } from '@segment/analytics-core' +import type { CoreEmitterContract } from '@segment/analytics-core' +import { Emitter } from '@segment/analytics-generic-utils' import { Context } from './context' import type { AnalyticsSettings } from './settings' import { SegmentEvent } from './types' diff --git a/packages/node/src/lib/abort.ts b/packages/node/src/lib/abort.ts index dae0dd341..b8d93120f 100644 --- a/packages/node/src/lib/abort.ts +++ b/packages/node/src/lib/abort.ts @@ -1,7 +1,7 @@ /** * use non-native event emitter for the benefit of non-node runtimes like CF workers. */ -import { Emitter } from '@segment/analytics-core' +import { Emitter } from '@segment/analytics-generic-utils' import { detectRuntime } from './env' /** diff --git a/packages/node/src/plugins/segmentio/__tests__/methods.test.ts b/packages/node/src/plugins/segmentio/__tests__/methods.test.ts index f1ed369b3..de07a0a3b 100644 --- a/packages/node/src/plugins/segmentio/__tests__/methods.test.ts +++ b/packages/node/src/plugins/segmentio/__tests__/methods.test.ts @@ -3,7 +3,7 @@ import { createSuccess } from '../../../__tests__/test-helpers/factories' import { createConfiguredNodePlugin } from '../index' import { PublisherProps } from '../publisher' import { Context } from '../../../app/context' -import { Emitter } from '@segment/analytics-core' +import { Emitter } from '@segment/analytics-generic-utils' import { assertHTTPRequestOptions, httpClientOptionsBodyMatcher, diff --git a/packages/node/src/plugins/segmentio/__tests__/publisher.test.ts b/packages/node/src/plugins/segmentio/__tests__/publisher.test.ts index c95462976..e1824ed5d 100644 --- a/packages/node/src/plugins/segmentio/__tests__/publisher.test.ts +++ b/packages/node/src/plugins/segmentio/__tests__/publisher.test.ts @@ -1,4 +1,4 @@ -import { Emitter } from '@segment/analytics-core' +import { Emitter } from '@segment/analytics-generic-utils' import { range } from 'lodash' import { createConfiguredNodePlugin } from '..' import { Context } from '../../../app/context' diff --git a/yarn.lock b/yarn.lock index aa0dfd376..21c452dd3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3742,6 +3742,7 @@ __metadata: resolution: "@segment/analytics-core@workspace:packages/core" dependencies: "@lukeed/uuid": ^2.0.0 + "@segment/analytics-generic-utils": 1.0.0 dset: ^3.1.2 tslib: ^2.4.1 languageName: unknown @@ -3761,6 +3762,7 @@ __metadata: "@lukeed/uuid": ^2.0.0 "@segment/analytics-browser-actions-braze": ^1.3.0 "@segment/analytics-core": 1.3.2 + "@segment/analytics-generic-utils": 1.0.0 "@segment/analytics.js-integration": ^3.3.3 "@segment/analytics.js-integration-amplitude": ^3.3.3 "@segment/analytics.js-video-plugins": ^0.2.1