improve doc / descriptions

packages/core/src/analytics/__tests__/dispatch.test.ts

@@ -16,7 +16,7 @@ jest.mock('../../callback', () => ({
 
 import { EventQueue } from '../../queue/event-queue'
 import { Emitter } from '../../emitter'
-import { dispatch, getDelayTimeout } from '../dispatch'
+import { dispatch, getDelay } from '../dispatch'
 import { PriorityQueue } from '../../priority-queue'
 
 let emitter!: Emitter
@@ -34,7 +34,7 @@ afterEach(() => {
 })
 
 describe('Dispatch', () => {
-  it('should return ctx if offline and retryQueue is false', async () => {
+  it('should not dispatch if client is currently offline and retries are *disabled* for the main event queue', async () => {
     isOnline.mockReturnValue(false)
     isOffline.mockReturnValue(true)
 
@@ -48,7 +48,7 @@ describe('Dispatch', () => {
     expect(called).toBeFalsy()
   })
 
-  it('should not dispatch if offline and retryQueue is true', async () => {
+  it('should be allowed to dispatch if client is currently offline and retries are *enabled* for the main event queue', async () => {
     isOnline.mockReturnValue(false)
     isOffline.mockReturnValue(true)
 
@@ -61,7 +61,7 @@ describe('Dispatch', () => {
     expect(called).toBeTruthy()
   })
 
-  it('should call dispatchSingle with ctx if queue is empty', async () => {
+  it('should call dispatchSingle correctly if queue is empty', async () => {
     queue.isEmpty = jest.fn().mockReturnValue(true)
     await dispatch({ type: 'screen' }, queue, emitter)
     expect(dispatchSingleSpy).toBeCalledWith(
@@ -70,7 +70,7 @@ describe('Dispatch', () => {
     expect(dispatchSpy).not.toBeCalled()
   })
 
-  it('should call dispatch with ctx if queue is empty', async () => {
+  it('should call dispatch correctly if queue has items', async () => {
     queue.isEmpty = jest.fn().mockReturnValue(false)
     await dispatch({ type: 'screen' }, queue, emitter)
     expect(dispatchSpy).toBeCalledWith(
@@ -90,14 +90,15 @@ describe('Dispatch', () => {
   })
 })
 
-describe('getDelayTimeout', () => {
-  it('should work as expected', () => {
+describe(getDelay, () => {
+  it('should calculate the amount of time to delay before invoking the callback', () => {
     const aShortTimeAgo = Date.now() - 200
-    expect(Math.round(getDelayTimeout(aShortTimeAgo, 500))).toBe(300)
+    const timeout = 5000
+    expect(Math.round(getDelay(aShortTimeAgo, timeout))).toBe(4800)
   })
 
   it('should have a sensible default', () => {
     const aShortTimeAgo = Date.now() - 200
-    expect(Math.round(getDelayTimeout(aShortTimeAgo))).toBe(100)
+    expect(Math.round(getDelay(aShortTimeAgo))).toBe(100)
   })
 })

packages/core/src/analytics/dispatch.ts

@@ -13,11 +13,9 @@ type DispatchOptions = {
 }
 
 /* The amount of time in ms to wait before invoking the callback. */
-export const getDelayTimeout = (
-  startTimeInEpochMS: number,
-  timeoutInMS?: number
-) => {
+export const getDelay = (startTimeInEpochMS: number, timeoutInMS?: number) => {
   const elapsedTime = Date.now() - startTimeInEpochMS
+  // increasing the timeout increases the delay by almost the same amount -- this is weird legacy behavior.
   return Math.max((timeoutInMS ?? 300) - elapsedTime, 0)
 }
 /**
@@ -53,7 +51,7 @@ export async function dispatch(
     dispatched = await invokeCallback(
       dispatched,
       options.callback,
-      getDelayTimeout(startTime, options.timeout),
+      getDelay(startTime, options.timeout), //  TODO: I have no idea why this delay is based on the timeout -- so if you set a timeout of 5seconds, it will (in addition) actually delay ~5seconds before invoking.
       options.timeout
     )
   }

packages/core/src/callback/index.ts

@@ -22,13 +22,15 @@ function sleep(timeoutInMs: number): Promise<void> {
 }
 
 /**
- * @param delayTimeout - The amount of time in ms to wait before invoking the callback.
+ * @param delay - The amount of time in ms to wait before invoking the callback.
+ * Ajs 1.0 worked differently so the delay was meant to give ajs a chance to send the event before invoking the callback.
+ * Customers used that behavior to delay navigating to a new page. However, AJS 2.0 resolves a promise once the event has already been sent... so this is likely used to give third party destinations time to flush
  * @param timeout - The maximum amount of time in ms to allow the callback to run for.
  */
 export function invokeCallback(
   ctx: CoreContext,
   callback: Callback,
-  delayTimeout: number,
+  delay: number,
   timeout?: number
 ): Promise<CoreContext> {
   const cb = () => {
@@ -40,7 +42,7 @@ export function invokeCallback(
   }
 
   return (
-    sleep(delayTimeout)
+    sleep(delay)
       // pTimeout ensures that the callback can't cause the context to hang
       .then(() => pTimeout(cb(), timeout ?? 1000))
       .catch((err) => {