Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(replay): Fix debounce when maxWait == wait #7208

Merged
merged 3 commits into from Feb 17, 2023

Conversation

billyvg
Copy link
Member

@billyvg billyvg commented Feb 16, 2023

Update and fix tests for debounce when maxWait == wait.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2023

size-limit report 📦

Path Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified) 20.05 KB (-0.01% 🔽)
@sentry/browser - ES5 CDN Bundle (minified) 62.14 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped + minified) 18.68 KB (-0.01% 🔽)
@sentry/browser - ES6 CDN Bundle (minified) 55.29 KB (0%)
@sentry/browser - Webpack (gzipped + minified) 20.41 KB (0%)
@sentry/browser - Webpack (minified) 66.73 KB (0%)
@sentry/react - Webpack (gzipped + minified) 20.44 KB (0%)
@sentry/nextjs Client - Webpack (gzipped + minified) 47.78 KB (0%)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) 26.93 KB (-0.01% 🔽)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified) 25.2 KB (-0.01% 🔽)
@sentry/replay ES6 CDN Bundle (gzipped + minified) 42.57 KB (+0.11% 🔺)
@sentry/replay - Webpack (gzipped + minified) 36.78 KB (+0.16% 🔺)
@sentry/browser + @sentry/tracing + @sentry/replay - ES6 CDN Bundle (gzipped + minified) 60.2 KB (+0.09% 🔺)
@sentry/browser + @sentry/replay - ES6 CDN Bundle (gzipped + minified) 53.8 KB (+0.1% 🔺)

@github-actions
Copy link
Contributor

Replay SDK metrics 🚀

    Plain +Sentry +Replay
Revision Value Value Diff Ratio Value Diff Ratio
LCP This PR 8305b94 73.10 ms 100.46 ms +27.36 ms +37.43 % 143.07 ms +69.97 ms +95.72 %
Previous 1cf8988 70.81 ms 94.27 ms +23.46 ms +33.13 % 124.62 ms +53.81 ms +75.99 %
CLS This PR 8305b94 0.06 ms 0.06 ms -0.00 ms -0.43 % 0.06 ms -0.00 ms -0.85 %
Previous 1cf8988 0.06 ms 0.06 ms -0.00 ms -0.39 % 0.06 ms -0.00 ms -0.46 %
CPU This PR 8305b94 16.63 % 17.27 % +0.65 pp +3.90 % 22.25 % +5.63 pp +33.83 %
Previous 1cf8988 12.30 % 12.01 % -0.29 pp -2.37 % 17.10 % +4.80 pp +39.02 %
JS heap avg This PR 8305b94 1.94 MB 2.01 MB +64.89 kB +3.34 % 2.87 MB +926.13 kB +47.68 %
Previous 1cf8988 1.94 MB 1.99 MB +48.93 kB +2.52 % 2.87 MB +929.88 kB +47.85 %
JS heap max This PR 8305b94 2.3 MB 2.55 MB +246.42 kB +10.70 % 3.36 MB +1.05 MB +45.72 %
Previous 1cf8988 2.3 MB 2.57 MB +267.17 kB +11.60 % 3.35 MB +1.05 MB +45.56 %
netTx This PR 8305b94 0 B 0 B 0 B n/a 2.2 kB +2.2 kB n/a
Previous 1cf8988 0 B 0 B 0 B n/a 2.22 kB +2.22 kB n/a
netRx This PR 8305b94 0 B 0 B 0 B n/a 41 B +41 B n/a
Previous 1cf8988 0 B 0 B 0 B n/a 41 B +41 B n/a
netCount This PR 8305b94 0 0 0 n/a 1 +1 n/a
Previous 1cf8988 0 0 0 n/a 1 +1 n/a
netTime This PR 8305b94 0.00 ms 0.00 ms 0.00 ms n/a 71.00 ms +71.00 ms n/a
Previous 1cf8988 0.00 ms 0.00 ms 0.00 ms n/a 91.07 ms +91.07 ms n/a

Previous results on branch: develop

RevisionLCPCLSCPUJS heap avgJS heap maxnetTxnetRxnetCountnetTime
1cf8988+53.81 ms-0.00 ms+4.80 pp+929.88 kB+1.05 MB+2.22 kB+41 B+1+91.07 ms
68655e3+72.60 ms+0.00 ms+7.90 pp+922.72 kB+1.04 MB+2.22 kB+41 B+1+109.40 ms
a8449de+58.27 ms-0.00 ms+7.12 pp+927.42 kB+1.05 MB+2.2 kB+41 B+1+98.31 ms
79babe9+58.69 ms-0.00 ms+4.40 pp+927.46 kB+1.06 MB+2.23 kB+41 B+1+103.20 ms
5359ba9+55.62 ms-0.00 ms+4.29 pp+935.26 kB+1.05 MB+2.2 kB+41 B+1+79.05 ms

*) pp - percentage points - an absolute difference between two percentages.
Last updated: Thu, 16 Feb 2023 20:44:16 GMT

@@ -57,8 +57,13 @@ export function debounce(func: CallbackFunction, wait: number, options?: Debounc
}
timerId = setTimeout(invokeFunc, wait);

if (maxWait && maxTimerId === undefined && maxWait !== wait) {
maxTimerId = setTimeout(invokeFunc, maxWait);
if (maxWait && maxTimerId === undefined) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, invokeFunc already calls clearTimeout, so we can streamlime this slightly - IMHO we only need to remove the maxWait !== wait part, which was an incorrect "streamlining" that we added.

I'll adjust this and merge this and will actually release this fix today as well!

billyvg and others added 3 commits February 17, 2023 09:13
@mydea mydea marked this pull request as ready for review February 17, 2023 08:15
@mydea mydea force-pushed the fix-replay-fix-debounce-maxwait branch from 48483f6 to cb71021 Compare February 17, 2023 08:15
Copy link
Member

@mydea mydea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good! I'll leave the default max wait time at 5500 for now, we can then later think about reverting this back to 5000 - IMHO it's also OK to leave it at 5500, I guess.

@mydea mydea merged commit aacdf2b into develop Feb 17, 2023
@mydea mydea deleted the fix-replay-fix-debounce-maxwait branch February 17, 2023 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants