Conversation
add waiting plugin interface. any plugin implement this interface will pause event processing when it's added to analytics any plugin implement this interface can pause and resume event processing as needed
| this.store = store; | ||
| this.timeline = new Timeline(); | ||
|
|
||
| // // create and add waiting plugin immediately so early events get buffered. |
There was a problem hiding this comment.
nitpick: it is a bit easier to read multiline comments if you put them in a comment block like this:
/*
my comment
my comment2
*/
| return; | ||
| } | ||
|
|
||
| this.isReady.value = false; |
There was a problem hiding this comment.
we should use our own value instead of re-using isReady, since isReady tracks initialization state and is referenced elsewhere
There was a problem hiding this comment.
for example its used in the init function to ensure init isn't called twice on the client
/**
* Initializes the client plugins, settings and subscribers.
* Can only be called once.
*/
async init() {
try {
if (this.isReady.value) {
this.logger.warn('SegmentClient already initialized');
return;
}
if we init the client, add a waiting plugin, then init the client again we would end up bypassing the safety check and running init twice
There was a problem hiding this comment.
my suggestion would be to make a new variable in the state called "running" to match the kotlin sdk, then update QueueFlushingPlugin.ts to only remove events from the queue and add them to the state only when state.running === true.
then we can pause/unpause event flushing in analytics.ts
|
|
||
| return totalEventsCount; | ||
| } | ||
| private resumeTimeoutId?: ReturnType<typeof setTimeout>; |
There was a problem hiding this comment.
if multiple waiting plugins are running concurrently would they both set the resumeTimeoutId and overwrite the other's?
| } | ||
|
|
||
| if (!this.isReady.value) { | ||
| console.log('!this.isReady.value', !this.isReady.value); |
There was a problem hiding this comment.
we should clean up & remove debug console.logs
| store, | ||
| }); | ||
|
|
||
| client.isReady.value = true; |
There was a problem hiding this comment.
client.isReady shouldn't manually be set, it should be set automatically after analytics.init is called on the client, or does the testing harness skip the client initialization?
|
|
||
| if (!this.isReady.value) { | ||
| console.log('!this.isReady.value', !this.isReady.value); | ||
| if (!this.isReady.value && !(plugin instanceof WaitingPlugin)) { |
There was a problem hiding this comment.
we shouldn't prevent waiting plugins from being queued to be added before the client is done initializing
2 participants
add waiting plugin interface.