ref(replay): Avoid duplicate debounce timers #6863
Conversation
size-limit report 📦
|
There was a problem hiding this comment.
Nice improvement. I think we already test equal max/min waits so I think we're good test-wise.
In the future we might be able to simplify the debounce function further, if we decide that maxwait should always equal wait. But let's leave it for the moment as-is
Jup, my thoughts as well! |
mydea
force-pushed
the
fn/avoid-debounce-duplicate-timers
branch
from
015ea3c
to
84f889e
Compare
| @@ -146,13 +148,13 @@ describe('Integration | sendReplayEvent', () => { | |||
| expect(replay.eventBuffer?.pendingLength).toBe(0); | |||
| }); | |||
|
|
|||
| it('uploads a replay event if 15 seconds have elapsed since the last replay upload', async () => { | |||
| it('uploads a replay event if maxFlushDelay is set 15 seconds have elapsed since the last replay upload', async () => { | |||
There was a problem hiding this comment.
Hmm, any clue why this test wasn't failing before? 😅 I suspect there is actually a bug (??) that is fixed here, but not sure - we do have a test for the wait time being equal, but maybe it doesn't catch some aspect...?!
| @@ -247,60 +249,6 @@ describe('Integration | sendReplayEvent', () => { | |||
| expect(replay.eventBuffer?.pendingLength).toBe(0); | |||
| }); | |||
|
|
|||
| it('uploads a replay event if 5 seconds have elapsed since the last replay event occurred', async () => { | |||
There was a problem hiding this comment.
this test was actually here twice!
| expect(replay.eventBuffer?.pendingLength).toBe(0); | ||
| }); | ||
|
|
||
| it('uploads a replay event if 15 seconds have elapsed since the last replay upload', async () => { |
There was a problem hiding this comment.
Same for this test, exact duplicate 😅
Since we now have a default flush time === max flush time in replay, we can skip the duplicate timer in that case.