Skip to content

add ability to capture customer events before library init #436

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 46 commits into from
May 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
f294708
add ability to capture customer events before library init
silesky May 8, 2022
3f05742
add promise compat type definitions and tests
silesky May 8, 2022
594fce2
rename files
silesky May 8, 2022
abc80d4
add filter method, remove callAnalyticsMethod from class
silesky May 8, 2022
1df199c
refactor flush
silesky May 8, 2022
b663137
add allSettled
silesky May 8, 2022
b979687
add allSettled test
silesky May 9, 2022
9944ee0
wip
silesky May 10, 2022
8507f59
move load out of AnalyticsBrowser class
silesky May 10, 2022
584528d
fix type error in pages
silesky May 10, 2022
f774b4b
don't bother returning allSettled result
silesky May 10, 2022
a864b66
remove export
silesky May 10, 2022
ab080df
preserve existing logic regarding parallel and serial execution
silesky May 10, 2022
01e7f95
rename fn
silesky May 10, 2022
1a351b6
add comments
silesky May 10, 2022
04f16ca
use map
silesky May 10, 2022
67e693e
wip
silesky May 11, 2022
4ca0665
add chainable method
silesky May 11, 2022
1e37957
add more tests
silesky May 11, 2022
ed2e060
Update comment
silesky May 12, 2022
1849c0a
Update comment
silesky May 12, 2022
fd654bd
differentiate between sync and async calls
silesky May 13, 2022
b326714
remove allSettled
silesky May 13, 2022
6d84a76
move more shared logic outside
silesky May 13, 2022
e9e3680
tweak async execution
silesky May 13, 2022
011e3bb
tweak type
silesky May 13, 2022
044de6c
add multi instance smoke test
silesky May 13, 2022
1b3cec2
add more tests
silesky May 13, 2022
f7fa8d3
remove no-longer-needed ignore from jest config
silesky May 14, 2022
3ecde68
improve async test
silesky May 16, 2022
9b4c1c9
more tests
silesky May 16, 2022
9c02f90
refactor typedef tests
silesky May 16, 2022
f1042bc
test naming clean up
silesky May 16, 2022
7a064c4
remove resetGlobalState
silesky May 17, 2022
cd278d6
clarify test
silesky May 17, 2022
e77db68
add "this" test to event emitter
silesky May 17, 2022
f785c64
update pre-init buffer, add tests
silesky May 17, 2022
6e94ff5
check thenable instead of instanceof promise
silesky May 17, 2022
39f0f27
remove async
silesky May 17, 2022
4d2bddf
update thenable
silesky May 18, 2022
95ef70c
add more "this" tests
silesky May 18, 2022
4c2dccb
delete clearAllMocks since clearMocks option is true in jest config
silesky May 18, 2022
cdea3d7
make push a bit clearer
silesky May 18, 2022
4401356
get rid of .call since we have a test around it
silesky May 18, 2022
37cd27a
rename and move files into core
silesky May 18, 2022
9a534db
encapsulate snippet logic
silesky May 18, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,12 @@
"@typescript-eslint/no-unused-vars": "off",
"@typescript-eslint/no-explicit-any": "off",
"@typescript-eslint/no-non-null-assertion": "off",
"@typescript-eslint/no-floating-promises": "error",
"@typescript-eslint/no-floating-promises": [
"error",
{
"ignoreVoid": true
}
],
"@typescript-eslint/require-await": "off",
"@typescript-eslint/no-empty-function": "off",
"@typescript-eslint/ban-types": "off" // TODO: turn on
Expand Down
3 changes: 1 addition & 2 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@ module.exports = {
'<rootDir>/dist/',
'<rootDir>/e2e-tests',
'<rootDir>/qa',
'<rootDir>/src/__tests__/test-writekeys',
'<rootDir>/src/__tests__/stats-writekey',
],
testMatch: ["**/?(*.)+(test).[jt]s?(x)"],
clearMocks: true,
testEnvironmentOptions: {
resources: 'usable',
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@
"cjs": "tsc -p tsconfig.cjs.json",
"run-example": "cd example && yarn && yarn dev",
"clean": "rm -rf dist",
"lint": "eslint '**/*.{js,jsx,ts,tsx}'",
"lint": "tsc --noEmit && eslint '**/*.{js,jsx,ts,tsx}'",
"prepare": "yarn pkg && husky install",
"test": "jest"
},
"size-limit": [
{
"path": "dist/umd/index.js",
"limit": "25.13 KB"
"limit": "25.9 KB"
}
],
"lint-staged": {
Expand Down
356 changes: 356 additions & 0 deletions src/__tests__/analytics-pre-init.integration.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,356 @@
import { AnalyticsBrowser } from '..'
import unfetch from 'unfetch'
import { mocked } from 'ts-jest/utils'
import { Analytics } from '../analytics'
import { AnalyticsBuffered } from '../core/buffer'
import { Context } from '../core/context'
import * as Factory from '../test-helpers/factories'
import { sleep } from '../test-helpers/sleep'
import { setGlobalCDNUrl } from '../lib/parse-cdn'

jest.mock('unfetch')

const mockFetchSettingsResponse = () => {
mocked(unfetch).mockImplementation(() =>
Factory.createSuccess({ integrations: {} })
)
}

const writeKey = 'foo'

const errMsg = 'errMsg'

describe('Pre-initialization', () => {
const trackSpy = jest.spyOn(Analytics.prototype, 'track')
const identifySpy = jest.spyOn(Analytics.prototype, 'identify')
const onSpy = jest.spyOn(Analytics.prototype, 'on')
const readySpy = jest.spyOn(Analytics.prototype, 'ready')
const browserLoadSpy = jest.spyOn(AnalyticsBrowser, 'load')
const consoleErrorSpy = jest.spyOn(console, 'error')

beforeEach(() => {
setGlobalCDNUrl(undefined as any)
mockFetchSettingsResponse()
;(window as any).analytics = undefined
})

describe('Smoke', () => {
test('load should instantiate an ajsBuffered object that resolves into an Analytics object', async () => {
const ajsBuffered = AnalyticsBrowser.load({ writeKey })
expect(ajsBuffered).toBeInstanceOf<typeof AnalyticsBuffered>(
AnalyticsBuffered
)
expect(ajsBuffered.instance).toBeUndefined()
const [ajs, ctx] = await ajsBuffered
expect(ajsBuffered.instance).toBeInstanceOf<typeof Analytics>(Analytics)
expect(ajsBuffered.ctx).toBeInstanceOf<typeof Context>(Context)
expect(ajs).toBeInstanceOf<typeof Analytics>(Analytics)
expect(ctx).toBeInstanceOf<typeof Context>(Context)
})

test('If a user sends a single pre-initialized track event, that event gets flushed', async () => {
const ajsBuffered = AnalyticsBrowser.load({ writeKey })
const trackCtxPromise = ajsBuffered.track('foo', { name: 'john' })
const result = await trackCtxPromise
expect(result).toBeInstanceOf(Context)
expect(trackSpy).toBeCalledWith('foo', { name: 'john' })
expect(trackSpy).toBeCalledTimes(1)
})

test('"return types should not change over the lifecycle for ordinary methods', async () => {
const ajsBuffered = AnalyticsBrowser.load({ writeKey })

const trackCtxPromise1 = ajsBuffered.track('foo', { name: 'john' })
expect(trackCtxPromise1).toBeInstanceOf(Promise)
const ctx1 = await trackCtxPromise1
expect(ctx1).toBeInstanceOf(Context)

// loaded
const trackCtxPromise2 = ajsBuffered.track('foo', { name: 'john' })
expect(trackCtxPromise2).toBeInstanceOf(Promise)
const ctx2 = await trackCtxPromise2
expect(ctx2).toBeInstanceOf(Context)
})

test('If a user sends multiple events, all of those event gets flushed', async () => {
const ajsBuffered = AnalyticsBrowser.load({ writeKey })
const trackCtxPromise = ajsBuffered.track('foo', { name: 'john' })
const trackCtxPromise2 = ajsBuffered.track('bar', { age: 123 })
const identifyCtxPromise = ajsBuffered.identify('hello')

await Promise.all([trackCtxPromise, trackCtxPromise2, identifyCtxPromise])

expect(trackSpy).toBeCalledWith('foo', { name: 'john' })
expect(trackSpy).toBeCalledWith('bar', { age: 123 })
expect(trackSpy).toBeCalledTimes(2)

expect(identifySpy).toBeCalledWith('hello')
expect(identifySpy).toBeCalledTimes(1)
})
})

describe('Promise API', () => {
describe('.then', () => {
test('.then should be called on success', (done) => {
const ajsBuffered = AnalyticsBrowser.load({ writeKey: 'abc' })
const newPromise = ajsBuffered.then(([analytics, context]) => {
expect(analytics).toBeInstanceOf<typeof Analytics>(Analytics)
expect(context).toBeInstanceOf<typeof Context>(Context)
done()
})
expect(newPromise).toBeInstanceOf<typeof Promise>(Promise)
})

it('.then should pass to the next .then', async () => {
const ajsBuffered = AnalyticsBrowser.load({ writeKey: 'abc' })
const obj = ajsBuffered.then(() => ({ foo: 123 } as const))
expect(obj).toBeInstanceOf(Promise)
await obj.then((el) => expect(el.foo).toBe(123))
})
})

describe('.catch', () => {
it('should be capable of handling errors if using promise syntax', () => {
browserLoadSpy.mockImplementationOnce((): any => Promise.reject(errMsg))

const ajsBuffered = AnalyticsBrowser.load({ writeKey: 'abc' })
const newPromise = ajsBuffered.catch((reason) => {
expect(reason).toBe(errMsg)
})
Comment on lines +117 to +119
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the same work if we're using await e.g. await AnalyticsBrowser.load({ writeKey: 'invalid' }) - does this throw an error?

Copy link
Contributor Author

@silesky silesky May 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. For example, that case is also covered here: https://github.com/segmentio/analytics-next/pull/436/files/f1042bc17a4f13b7335433390d1892e77b630aed#diff-a3756d7d2cc9639c2f00f5871656d78a5aa10fddd6ba6bdae04447a577140d4aR165 (and probably some other places)

I think I only preferred to test promise syntax here in order to be explicit that we are talking about thenable methods, since try/catch can be used in other contexts. Wasn't necessary to do it one way or another... Technically, if an object is asserted to be a thenable as it is here (it has a .then and optional .catch method), I wouldn't necessarily try to test both syntax forms, as that's part of the spec.

expect(newPromise).toBeInstanceOf(Promise)
expect.assertions(2)
})
})

describe('.finally', () => {
test('success', async () => {
const ajsBuffered = AnalyticsBrowser.load({ writeKey: 'abc' })
const thenCb = jest.fn()
const finallyCb = jest.fn()
const catchCb = jest.fn()
await ajsBuffered.then(thenCb).catch(catchCb).finally(finallyCb)
expect(catchCb).not.toBeCalled()
expect(finallyCb).toBeCalledTimes(1)
expect(thenCb).toBeCalledTimes(1)
})
test('rejection', async () => {
browserLoadSpy.mockImplementationOnce((): any => Promise.reject(errMsg))
const ajsBuffered = AnalyticsBrowser.load({ writeKey: 'abc' })
const onFinallyCb = jest.fn()
await ajsBuffered
.catch((reason) => {
expect(reason).toBe(errMsg)
})
.finally(() => {
onFinallyCb()
})
expect(onFinallyCb).toBeCalledTimes(1)
expect.assertions(2)
})
})
})

describe('Load failures', () => {
test('rejected promise should work as expected for buffered analytics instances', async () => {
trackSpy.mockImplementationOnce(() => Promise.reject(errMsg))
const ajsBuffered = AnalyticsBrowser.load({ writeKey })
try {
await ajsBuffered.track('foo', { name: 'john' })
} catch (err) {
expect(err).toBe(errMsg)
}
expect.assertions(1)
})

test('rejected promise should work as expected for initialized analytics instances', async () => {
trackSpy.mockImplementationOnce(() => Promise.reject(errMsg))
const [analytics] = await AnalyticsBrowser.load({ writeKey })
try {
await analytics.track('foo', { name: 'john' })
} catch (err) {
expect(err).toBe(errMsg)
}
expect.assertions(1)
})
})

describe('Snippet / standalone', () => {
test('If a snippet user sends multiple events, all of those event gets flushed', async () => {
const onTrackCb = jest.fn()
const onTrack = ['on', 'track', onTrackCb]
const track = ['track', 'foo']
const track2 = ['track', 'bar']
const identify = ['identify']

;(window as any).analytics = [onTrack, track, track2, identify]

await AnalyticsBrowser.standalone(writeKey)

await sleep(100) // the snippet does not return a promise (pre-initialization) ... it sometimes has a callback as the third argument.
expect(trackSpy).toBeCalledWith('foo')
expect(trackSpy).toBeCalledWith('bar')
expect(trackSpy).toBeCalledTimes(2)

expect(identifySpy).toBeCalledWith()
expect(identifySpy).toBeCalledTimes(1)

expect(onSpy).toBeCalledTimes(1)

expect(onTrackCb).toBeCalledTimes(2) // gets called once for each track event
expect(onTrackCb).toBeCalledWith('foo', {}, undefined)
expect(onTrackCb).toBeCalledWith('bar', {}, undefined)
})
test('If a snippet user has an event "fail", it will not create a promise rejection or effect other method calls', async () => {
identifySpy.mockImplementationOnce(() => {
return Promise.reject('identity rejection')
})
consoleErrorSpy.mockImplementationOnce(() => null)

const onTrackCb = jest.fn()
const onTrack = ['on', 'track', onTrackCb]
const track = ['track', 'foo']
const track2 = ['track', 'bar']
const identify = ['identify']

;(window as any).analytics = [identify, onTrack, track, track2]

await AnalyticsBrowser.standalone(writeKey)

await sleep(100) // the snippet does not return a promise (pre-initialization) ... it sometimes has a callback as the third argument.
expect(trackSpy).toBeCalledWith('foo')
expect(trackSpy).toBeCalledWith('bar')
expect(trackSpy).toBeCalledTimes(2)

expect(identifySpy).toBeCalledWith()
expect(identifySpy).toBeCalledTimes(1)
expect(consoleErrorSpy).toBeCalledTimes(1)
expect(consoleErrorSpy).toBeCalledWith('identity rejection')

expect(onSpy).toBeCalledTimes(1)

expect(onTrackCb).toBeCalledTimes(2) // gets called once for each track event
expect(onTrackCb).toBeCalledWith('foo', {}, undefined)
expect(onTrackCb).toBeCalledWith('bar', {}, undefined)
})
})

describe('Emitter methods', () => {
test('If, before initialization, .on("track") is called, the .on method should be called after analytics load', async () => {
const ajsBuffered = AnalyticsBrowser.load({ writeKey })
const args = ['track', jest.fn()] as const
ajsBuffered.on(...args)
expect(onSpy).not.toHaveBeenCalledWith(...args)

await ajsBuffered
expect(onSpy).toBeCalledWith(...args)
expect(onSpy).toHaveBeenCalledTimes(1)
})

test('If, before initialization .on("track") is called and then .track is called, the callback method should be called after analytics loads', async () => {
const onFnCb = jest.fn()
const analytics = AnalyticsBrowser.load({ writeKey })
analytics.on('track', onFnCb)
const trackCtxPromise = analytics.track('foo', { name: 123 })

expect(onFnCb).not.toHaveBeenCalled()

await Promise.all([analytics, trackCtxPromise])

expect(onSpy).toBeCalledWith('track', onFnCb)
expect(onSpy).toHaveBeenCalledTimes(1)

expect(onFnCb).toHaveBeenCalledWith('foo', { name: 123 }, undefined)
expect(onFnCb).toHaveBeenCalledTimes(1)
})

test('If, before initialization, .ready is called, the callback method should be called after analytics loads', async () => {
const onReadyCb = jest.fn()
const analytics = AnalyticsBrowser.load({ writeKey })
const onReadyPromise = analytics.ready(onReadyCb)
expect(onReadyCb).not.toHaveBeenCalled()
await onReadyPromise
expect(readySpy).toHaveBeenCalledTimes(1)
expect(onReadyCb).toHaveBeenCalledTimes(1)
expect(readySpy).toHaveBeenCalledWith(expect.any(Function))
})

test('Should work with "on" events if a track event is called after load is complete', async () => {
const onTrackCb = jest.fn()
const ajsBuffered = AnalyticsBrowser.load({ writeKey })
ajsBuffered.on('track', onTrackCb)
await ajsBuffered
await ajsBuffered.track('foo', { name: 123 })

expect(onTrackCb).toHaveBeenCalledTimes(1)
expect(onTrackCb).toHaveBeenCalledWith('foo', { name: 123 }, undefined)
})
test('"on, off, once" should return ajsBuffered', () => {
const analytics = AnalyticsBrowser.load({ writeKey })
expect(
[
analytics.on('track', jest.fn),
analytics.off('track', jest.fn),
analytics.once('track', jest.fn),
].map((el) => el instanceof AnalyticsBuffered)
).toEqual([true, true, true])
})

test('"emitted" events should be chainable', async () => {
const onTrackCb = jest.fn()
const onIdentifyCb = jest.fn()
const ajsBuffered = AnalyticsBrowser.load({ writeKey })
const identifyResult = ajsBuffered.identify('bar')
const result = ajsBuffered
.on('track', onTrackCb)
.on('identify', onIdentifyCb)
.once('group', jest.fn)
.off('alias', jest.fn)

expect(result instanceof AnalyticsBuffered).toBeTruthy()
await ajsBuffered.track('foo', { name: 123 })
expect(onTrackCb).toHaveBeenCalledTimes(1)
expect(onTrackCb).toHaveBeenCalledWith('foo', { name: 123 }, undefined)

await identifyResult
expect(onIdentifyCb).toHaveBeenCalledTimes(1)
expect(onIdentifyCb).toHaveBeenCalledWith('bar', {}, undefined)
})

test('the "this" value of "emitted" event callbacks should be Analytics', async () => {
const ajsBuffered = AnalyticsBrowser.load({ writeKey })
ajsBuffered.on('track', function onTrackCb(this: any) {
expect(this).toBeInstanceOf(Analytics)
})
ajsBuffered.once('group', function trackOnceCb(this: any) {
expect(this).toBeInstanceOf(Analytics)
})

await Promise.all([
ajsBuffered.track('foo', { name: 123 }),
ajsBuffered.group('foo'),
])
})

test('"return types should not change over the lifecycle for chainable methods', async () => {
const ajsBuffered = AnalyticsBrowser.load({ writeKey })

const result1 = ajsBuffered.on('track', jest.fn)
expect(result1).toBeInstanceOf(AnalyticsBuffered)
await result1
// loaded
const result2 = ajsBuffered.on('track', jest.fn)
expect(result2).toBeInstanceOf(AnalyticsBuffered)
})
})

describe('Multi-instance', () => {
it('should not throw an error', async () => {
const ajsBuffered1 = AnalyticsBrowser.load({ writeKey: 'foo' })
const ajsBuffered2 = AnalyticsBrowser.load({ writeKey: 'abc' })
expect(ajsBuffered1).toBeInstanceOf(AnalyticsBuffered)
expect(ajsBuffered2).toBeInstanceOf(AnalyticsBuffered)
await ajsBuffered1
await ajsBuffered2
})
})
})
Loading