Skip to content

Commit bfab496

Browse files
Remove whenInit callback mechanism from trackers. It was originally added to handle React SDK side effects, but it’s unnecessary if the user calls client.track inside an effect.
1 parent 9cb3ccf commit bfab496

5 files changed

Lines changed: 27 additions & 46 deletions

File tree

src/sdkFactory/index.ts

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,6 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ISDK | SplitIO.IA
3535

3636
// initialization
3737
let hasInit = false;
38-
const initCallbacks: (() => void)[] = [];
39-
40-
function whenInit(cb: () => void) {
41-
if (hasInit) cb();
42-
else initCallbacks.push(cb);
43-
}
4438

4539
const sdkReadinessManager = sdkReadinessManagerFactory(platform.EventEmitter, settings);
4640
const readiness = sdkReadinessManager.readinessManager;
@@ -82,8 +76,8 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ISDK | SplitIO.IA
8276
strategyDebugFactory(observer) :
8377
noneStrategy;
8478

85-
const impressionsTracker = impressionsTrackerFactory(settings, storage.impressions, noneStrategy, strategy, whenInit, integrationsManager, storage.telemetry);
86-
const eventTracker = eventTrackerFactory(settings, storage.events, whenInit, integrationsManager, storage.telemetry);
79+
const impressionsTracker = impressionsTrackerFactory(settings, storage.impressions, noneStrategy, strategy, integrationsManager, storage.telemetry);
80+
const eventTracker = eventTrackerFactory(settings, storage.events, integrationsManager, storage.telemetry);
8781

8882
// splitApi is used by SyncManager and Browser signal listener
8983
const splitApi = splitApiFactory && splitApiFactory(settings, platform, telemetryTracker);
@@ -111,9 +105,6 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ISDK | SplitIO.IA
111105
uniqueKeysTracker.start();
112106
syncManager && syncManager.start();
113107
signalListener && signalListener.start();
114-
115-
initCallbacks.forEach((cb) => cb());
116-
initCallbacks.length = 0;
117108
}
118109

119110
log.info(NEW_FACTORY, [settings.version]);

src/trackers/__tests__/eventTracker.spec.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,13 @@ const fakeEvent = {
2929
}
3030
};
3131

32-
const fakeWhenInit = (cb: () => void) => cb();
33-
3432
/* Tests */
3533
describe('Event Tracker', () => {
3634

3735
test('Tracker API', () => {
3836
expect(typeof eventTrackerFactory).toBe('function'); // The module should return a function which acts as a factory.
3937

40-
const instance = eventTrackerFactory(fullSettings, fakeEventsCache, fakeWhenInit, fakeIntegrationsManager);
38+
const instance = eventTrackerFactory(fullSettings, fakeEventsCache, fakeIntegrationsManager);
4139

4240
expect(typeof instance.track).toBe('function'); // The instance should implement the track method.
4341
});
@@ -53,7 +51,7 @@ describe('Event Tracker', () => {
5351
}
5452
});
5553
// @ts-ignore
56-
const tracker = eventTrackerFactory(fullSettings, fakeEventsCache, fakeWhenInit, fakeIntegrationsManager, fakeTelemetryCache);
54+
const tracker = eventTrackerFactory(fullSettings, fakeEventsCache, fakeIntegrationsManager, fakeTelemetryCache);
5755
const result1 = tracker.track(fakeEvent, 1);
5856

5957
expect(fakeEventsCache.track.mock.calls[0]).toEqual([fakeEvent, 1]); // Should be present in the event cache.
@@ -94,7 +92,7 @@ describe('Event Tracker', () => {
9492
const settings = { ...fullSettings };
9593
const fakeEventsCache = { track: jest.fn(() => true) };
9694

97-
const tracker = eventTrackerFactory(settings, fakeEventsCache, fakeWhenInit);
95+
const tracker = eventTrackerFactory(settings, fakeEventsCache);
9896

9997
expect(tracker.track(fakeEvent)).toBe(true);
10098
expect(fakeEventsCache.track).toBeCalledTimes(1); // event should be tracked if userConsent is undefined

src/trackers/__tests__/impressionsTracker.spec.ts

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,6 @@ const fakeSettingsWithListener = {
3434
...fakeSettings,
3535
impressionListener: fakeListener
3636
};
37-
const fakeWhenInit = (cb: () => void) => cb();
38-
3937
const fakeNoneStrategy = {
4038
process: jest.fn(() => false)
4139
};
@@ -53,7 +51,7 @@ describe('Impressions Tracker', () => {
5351
const strategy = strategyDebugFactory(impressionObserverCSFactory());
5452

5553
test('Should be able to track impressions (in DEBUG mode without Previous Time).', () => {
56-
const tracker = impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategy, fakeWhenInit);
54+
const tracker = impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategy);
5755

5856
const imp1 = {
5957
feature: '10',
@@ -73,7 +71,7 @@ describe('Impressions Tracker', () => {
7371
});
7472

7573
test('Tracked impressions should be sent to impression listener and integration manager when we invoke .track()', (done) => {
76-
const tracker = impressionsTrackerFactory(fakeSettingsWithListener, fakeImpressionsCache, fakeNoneStrategy, strategy, fakeWhenInit, fakeIntegrationsManager);
74+
const tracker = impressionsTrackerFactory(fakeSettingsWithListener, fakeImpressionsCache, fakeNoneStrategy, strategy, fakeIntegrationsManager);
7775

7876
const fakeImpression = {
7977
feature: 'impression'
@@ -147,8 +145,8 @@ describe('Impressions Tracker', () => {
147145
impression3.time = 1234567891;
148146

149147
const trackers = [
150-
impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategyDebugFactory(impressionObserverSSFactory()), fakeWhenInit, undefined),
151-
impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategyDebugFactory(impressionObserverCSFactory()), fakeWhenInit, undefined)
148+
impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategyDebugFactory(impressionObserverSSFactory()), undefined),
149+
impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategyDebugFactory(impressionObserverCSFactory()), undefined)
152150
];
153151

154152
expect(fakeImpressionsCache.track).not.toBeCalled(); // storage method should not be called until impressions are tracked.
@@ -175,7 +173,7 @@ describe('Impressions Tracker', () => {
175173
impression3.time = Date.now();
176174

177175
const impressionCountsCache = new ImpressionCountsCacheInMemory();
178-
const tracker = impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategyOptimizedFactory(impressionObserverCSFactory(), impressionCountsCache), fakeWhenInit, undefined, fakeTelemetryCache as any);
176+
const tracker = impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategyOptimizedFactory(impressionObserverCSFactory(), impressionCountsCache), undefined, fakeTelemetryCache as any);
179177

180178
expect(fakeImpressionsCache.track).not.toBeCalled(); // cache method should not be called by just creating a tracker
181179

@@ -198,7 +196,7 @@ describe('Impressions Tracker', () => {
198196
test('Should track or not impressions depending on user consent status', () => {
199197
const settings = { ...fullSettings };
200198

201-
const tracker = impressionsTrackerFactory(settings, fakeImpressionsCache, fakeNoneStrategy, strategy, fakeWhenInit);
199+
const tracker = impressionsTrackerFactory(settings, fakeImpressionsCache, fakeNoneStrategy, strategy);
202200

203201
tracker.track([{ imp: impression }]);
204202
expect(fakeImpressionsCache.track).toBeCalledTimes(1); // impression should be tracked if userConsent is undefined

src/trackers/eventTracker.ts

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import { isConsumerMode } from '../utils/settingsValidation/mode';
1717
export function eventTrackerFactory(
1818
settings: ISettings,
1919
eventsCache: IEventsCacheBase,
20-
whenInit: (cb: () => void) => void,
2120
integrationsManager?: IEventsHandler,
2221
telemetryCache?: ITelemetryCacheSync | ITelemetryCacheAsync
2322
): IEventTracker {
@@ -33,15 +32,13 @@ export function eventTrackerFactory(
3332
if (tracked) {
3433
log.info(EVENTS_TRACKER_SUCCESS, [msg]);
3534
if (integrationsManager) {
36-
whenInit(() => {
37-
// Wrap in a timeout because we don't want it to be blocking.
38-
setTimeout(() => {
39-
// copy of event, to avoid unexpected behavior if modified by integrations
40-
const eventDataCopy = objectAssign({}, eventData);
41-
if (properties) eventDataCopy.properties = objectAssign({}, properties);
42-
// integrationsManager does not throw errors (they are internally handled by each integration module)
43-
integrationsManager.handleEvent(eventDataCopy);
44-
});
35+
// Wrap in a timeout because we don't want it to be blocking.
36+
setTimeout(() => {
37+
// copy of event, to avoid unexpected behavior if modified by integrations
38+
const eventDataCopy = objectAssign({}, eventData);
39+
if (properties) eventDataCopy.properties = objectAssign({}, properties);
40+
// integrationsManager does not throw errors (they are internally handled by each integration module)
41+
integrationsManager.handleEvent(eventDataCopy);
4542
});
4643
}
4744
} else {

src/trackers/impressionsTracker.ts

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ export function impressionsTrackerFactory(
1515
impressionsCache: IImpressionsCacheBase,
1616
noneStrategy: IStrategy,
1717
strategy: IStrategy,
18-
whenInit: (cb: () => void) => void,
1918
integrationsManager?: IImpressionsHandler,
2019
telemetryCache?: ITelemetryCacheSync | ITelemetryCacheAsync,
2120
): IImpressionsTracker {
@@ -67,18 +66,16 @@ export function impressionsTrackerFactory(
6766
sdkLanguageVersion: version
6867
};
6968

70-
whenInit(() => {
71-
// Wrap in a timeout because we don't want it to be blocking.
72-
setTimeout(() => {
73-
// integrationsManager.handleImpression does not throw errors
74-
if (integrationsManager) integrationsManager.handleImpression(impressionData);
69+
// Wrap in a timeout because we don't want it to be blocking.
70+
setTimeout(() => {
71+
// integrationsManager.handleImpression does not throw errors
72+
if (integrationsManager) integrationsManager.handleImpression(impressionData);
7573

76-
try { // @ts-ignore. An exception on the listeners should not break the SDK.
77-
if (impressionListener) impressionListener.logImpression(impressionData);
78-
} catch (err) {
79-
log.error(ERROR_IMPRESSIONS_LISTENER, [err]);
80-
}
81-
});
74+
try { // @ts-ignore. An exception on the listeners should not break the SDK.
75+
if (impressionListener) impressionListener.logImpression(impressionData);
76+
} catch (err) {
77+
log.error(ERROR_IMPRESSIONS_LISTENER, [err]);
78+
}
8279
});
8380
}
8481
}

0 commit comments

Comments
 (0)