diff --git a/README.md b/README.md index 1c8589443..6f498757a 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ This README is intended for developing the library itself. ## Testing Unit tests: run `yarn test`. -Cypress: run `yarn serve` to have a test server running and separately `yarn cypress` to launch Cypress test engine. +Cypress: run `yarn start` to have a test server running and separately `yarn cypress` to launch Cypress test engine. ### Running TestCafe E2E tests with BrowserStack diff --git a/functional_tests/feature-flags.test.ts b/functional_tests/feature-flags.test.ts index a9bf93bc6..3b2cc6861 100644 --- a/functional_tests/feature-flags.test.ts +++ b/functional_tests/feature-flags.test.ts @@ -107,10 +107,7 @@ test('person properties set in identify() with the same distinct_id are sent to }) }) -test('identify() doesnt trigger new request automatically if first request takes too long', async () => { - // TODO: Make this experience nicer, and queue requests, rather than leave - // it upto the user to call reloadFeatureFlags() manually. - +test('identify() triggers new request in queue after first request', async () => { const token = v4() const posthog = await createPosthogInstance(token, { advanced_disable_decide: false }) @@ -130,12 +127,60 @@ test('identify() doesnt trigger new request automatically if first request takes resetRequests(token) // don't wait for decide callback - posthog.identify('test-id', { - email: 'test@email.com', + email: 'test2@email.com', }) await waitFor(() => { expect(getRequests(token)['/decide/']).toEqual([]) }) + + // wait for decide callback + // eslint-disable-next-line compat/compat + await new Promise((res) => setTimeout(res, 500)) + + // now second call should've fired + await waitFor(() => { + expect(getRequests(token)['/decide/']).toEqual([ + { + $anon_distinct_id: anonymousId, + distinct_id: 'test-id', + groups: {}, + person_properties: { email: 'test2@email.com' }, + token, + }, + ]) + }) +}) + +test('identify() does not trigger new request in queue after first request for loaded callback', async () => { + const token = v4() + await createPosthogInstance(token, { + advanced_disable_decide: false, + bootstrap: { distinctID: 'anon-id' }, + loaded: (ph) => { + ph.identify('test-id', { email: 'test3@email.com' }) + ph.group('playlist', 'id:77', { length: 8 }) + }, + }) + + await waitFor(() => { + expect(getRequests(token)['/decide/']).toEqual([ + // This is the initial call to the decide endpoint on PostHog init, with all info added from `loaded`. + { + // $anon_distinct_id: 'anon-id', // no anonymous ID is sent because this was overridden on load + distinct_id: 'test-id', + groups: { playlist: 'id:77' }, + person_properties: { + email: 'test3@email.com', + }, + group_properties: { + playlist: { + length: 8, + }, + }, + token, + }, + ]) + }) }) diff --git a/src/__tests__/compression.js b/src/__tests__/compression.js index fc0ee5572..2904fe5bd 100644 --- a/src/__tests__/compression.js +++ b/src/__tests__/compression.js @@ -92,8 +92,8 @@ describe('Payload Compression', () => { }, featureFlags: { receivedFeatureFlags: jest.fn(), - resetRequestQueue: jest.fn(), setReloadingPaused: jest.fn(), + _startReloadTimer: jest.fn(), }, _hasBootstrappedFeatureFlags: jest.fn(), get_property: (property_key) => diff --git a/src/__tests__/decide.js b/src/__tests__/decide.js index 8ee7078b9..20122a342 100644 --- a/src/__tests__/decide.js +++ b/src/__tests__/decide.js @@ -27,8 +27,8 @@ describe('Decide', () => { }, featureFlags: { receivedFeatureFlags: jest.fn(), - resetRequestQueue: jest.fn(), setReloadingPaused: jest.fn(), + _startReloadTimer: jest.fn(), }, _hasBootstrappedFeatureFlags: jest.fn(), getGroups: () => ({ organization: '5' }), diff --git a/src/__tests__/posthog-core.js b/src/__tests__/posthog-core.js index 1474c6c12..87a2a2ac2 100644 --- a/src/__tests__/posthog-core.js +++ b/src/__tests__/posthog-core.js @@ -893,6 +893,7 @@ describe('_loaded()', () => { featureFlags: { setReloadingPaused: jest.fn(), resetRequestQueue: jest.fn(), + _startReloadTimer: jest.fn(), }, _start_queue_if_opted_in: jest.fn(), })) diff --git a/src/__tests__/posthog-core.loaded.js b/src/__tests__/posthog-core.loaded.js index 0afbd28c0..4f7ce5e8b 100644 --- a/src/__tests__/posthog-core.loaded.js +++ b/src/__tests__/posthog-core.loaded.js @@ -18,6 +18,7 @@ describe('loaded() with flags', () => { featureFlags: { setReloadingPaused: jest.fn(), resetRequestQueue: jest.fn(), + _startReloadTimer: jest.fn(), receivedFeatureFlags: jest.fn(), }, _start_queue_if_opted_in: jest.fn(), @@ -48,6 +49,7 @@ describe('loaded() with flags', () => { beforeEach(() => { jest.spyOn(given.lib.featureFlags, 'setGroupPropertiesForFlags') jest.spyOn(given.lib.featureFlags, 'setReloadingPaused') + jest.spyOn(given.lib.featureFlags, '_startReloadTimer') jest.spyOn(given.lib.featureFlags, 'resetRequestQueue') jest.spyOn(given.lib.featureFlags, '_reloadFeatureFlagsRequest') }) @@ -55,17 +57,22 @@ describe('loaded() with flags', () => { it('doesnt call flags while initial load is happening', () => { given.subject() - jest.runAllTimers() + jest.runOnlyPendingTimers() expect(given.lib.featureFlags.setGroupPropertiesForFlags).toHaveBeenCalled() // loaded ph.group() calls setGroupPropertiesForFlags expect(given.lib.featureFlags.setReloadingPaused).toHaveBeenCalledWith(true) expect(given.lib.featureFlags.resetRequestQueue).toHaveBeenCalledTimes(1) + expect(given.lib.featureFlags._startReloadTimer).toHaveBeenCalled() expect(given.lib.featureFlags.setReloadingPaused).toHaveBeenCalledWith(false) - // even if the decide request returned late, we should not call _reloadFeatureFlagsRequest + // we should call _reloadFeatureFlagsRequest for `group` only after the initial load // because it ought to be paused until decide returns expect(given.overrides._send_request).toHaveBeenCalledTimes(1) expect(given.lib.featureFlags._reloadFeatureFlagsRequest).toHaveBeenCalledTimes(0) + + jest.runOnlyPendingTimers() + expect(given.overrides._send_request).toHaveBeenCalledTimes(2) + expect(given.lib.featureFlags._reloadFeatureFlagsRequest).toHaveBeenCalledTimes(1) }) }) @@ -74,7 +81,7 @@ describe('loaded() with flags', () => { expect(given.overrides.featureFlags.setReloadingPaused).toHaveBeenCalledWith(true) expect(given.overrides.featureFlags.setReloadingPaused).toHaveBeenCalledWith(false) - expect(given.overrides.featureFlags.resetRequestQueue).toHaveBeenCalledTimes(1) + expect(given.overrides.featureFlags._startReloadTimer).toHaveBeenCalled() expect(given.overrides.featureFlags.receivedFeatureFlags).toHaveBeenCalledTimes(1) }) }) diff --git a/src/decide.ts b/src/decide.ts index 1a79dee96..bf92c8fe1 100644 --- a/src/decide.ts +++ b/src/decide.ts @@ -39,8 +39,9 @@ export class Decide { } parseDecideResponse(response: DecideResponse): void { - this.instance.featureFlags.resetRequestQueue() this.instance.featureFlags.setReloadingPaused(false) + // :TRICKY: Reload - start another request if queued! + this.instance.featureFlags._startReloadTimer() if (response?.status === 0) { console.error('Failed to fetch feature flags from PostHog.') diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 2635f8d03..e52a055cd 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -548,6 +548,10 @@ export class PostHog { // and compression will not be available. if (!disableDecide) { new Decide(this).call() + + // TRICKY: Reset any decide reloads queued during config.loaded because they'll be + // covered by the decide call right above. + this.featureFlags.resetRequestQueue() } }