ref(replay): Replace lodash.debounce with custom debounce implementation

[Lms24]

We've been using lodash.debounce to debounce the flushing of replay events. This has a number of drawbacks:

• It caused bugs and build errors for our users ([Replay] Uncaught TypeError: debounce.default is not a function #6474, fix(replay): Ensure lodash.debounce does not trigger next.js warning #6551)
• It's a third-party dependency and we generally want to use as few of them as possible
• lodash's implementation includes features we don't use, making it's bundle size footprint bigger than it needs to be
• lodash's implementation is in CJS, so we need to upcomiple it to ESM when building the replay package and CDN bundles

This PR replaces lodash's implementation with a heavily simplified version of debounce which just does what we need it for:

• Delay the function invocation by a wait time and gate it with a maxWait value
• Provide the return value of the invocation for subsequent debounced function calls
• Provide a flush and cancel function on the debounced function (analogously to lodash).
• Invoke the function after the wait/maxwait time triggered (i.e. on the trailing edge). Lodash allows to choose between the leading and trailing edge, which makes the implementation much more complicated than for us necessary
• This works for functions without parameters. By not supporting args, we can further cut down bundle size. (We might want to revisit this in the future but for now, this should do).

With this change, we can also get rid of the package patch we introduced in #6551 and of the commonjs() plugin in our build process.

closes #6553

[github-actions]

size-limit report 📦

Path	Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified)	19.79 KB (+0.01% 🔺)
@sentry/browser - ES5 CDN Bundle (minified)	61.3 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped + minified)	18.57 KB (-0.01% 🔽)
@sentry/browser - ES6 CDN Bundle (minified)	54.84 KB (0%)
@sentry/browser - Webpack (gzipped + minified)	20.34 KB (0%)
@sentry/browser - Webpack (minified)	66.48 KB (0%)
@sentry/react - Webpack (gzipped + minified)	20.36 KB (0%)
@sentry/nextjs Client - Webpack (gzipped + minified)	47.56 KB (0%)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified)	26.75 KB (-0.02% 🔽)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified)	25.19 KB (+0.02% 🔺)
@sentry/replay ES6 CDN Bundle (gzipped + minified)	41.59 KB (-1.18% 🔽)
@sentry/replay - Webpack (gzipped + minified)	37.6 KB (-1.49% 🔽)

[lforst]

Just a thought: Intuitively I would have implemented the debounce + maxWait by starting two timeouts. The first one is for maxWait. That timeout invokes the function if it hasn't been invoked yet and cancels the other timeout. The other timeout is for the actual debounce functionality.

With that implementation, I think we wouldn't have to track the remaining time and things like that. Was there a particular reason why you went this route? I might be missing something.

[Lms24]

This was also my first approach and I already implemented it that way. However, I got replay test errors I couldn't explain, which made me look at the lodash implementation more carefully and I decided to give the one-timeout approach a try. Looking back though, I think I might have missed some details in my first implementation so I'll give the 2-timeouts approach another go. In case I don't get anywhere with it, I'll come back to this version.

[Lms24]

Ok, I've rewritten the debounce implementation with two timers, which should bring down bundle size more than the first version. This required some test adjustments (see my comment)

Also, I moved the function back to the Replay package because we can still pull it into utils if other packages need it.

[Lms24]

I had to replace a couple of runAllTimers calls with advanceTimersByTime because apparently, two timers don't seem to work with runAllTimers. I'm honestly not entirely sure why and I don't fully understand what might be causing these errors but I think correctness-wise, the debounce function isn't responsible for the test fails.

WDYT, does this change make sense? @billyvg you know these tests best. Is there something I'm missing?

[billyvg]

Not sure, I would think runAllTimers should work -- in any case I think replacing it with advanceTimersByTime is fine.

Should we replace 5000 (and the default for flushMinDelay) with a constant?

[Lms24]

My theory is that the cancellation of the two timers when one of them is invoked might somehow interfere with whatever runAllTimers is doing but tbh this is a guess at best.

Should we replace 5000 (and the default for flushMinDelay) with a constant?

Sure can do

[Lms24]

Sure can do

On second thought, let's do this in a separate PR, just to not clutter this one. I've checked and we're using 5000 quite a lot in tests.

Edit: #6612

[Lms24]

Since build changes are always risky, I double checked NPM packages and CDN bundles and they still work with my test apps.

[mydea]

Great work! 🚀