-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
ref(browser): Simplify TraceKit parser and reorganize tests #4514
Conversation
Guessing these code analysis results are false positives since it's highlighting regexes that I haven't even modified? |
It might be that CodeQL only scans code that was changed (and tracekit regex hasn’t been moved for a while). I’ll mark as false positive for now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to have ~0.7% difference in minified, non-gzipped bundle (which is what we are focusing on)
@sentry/browser - CDN Bundle (gzipped)
Size limit: 100 KB
Size: 19.63 KB with all dependencies, minified and gzipped
@sentry/browser - CDN Bundle (minified)
Size limit: 120 KB
Size: 62.75 KB with all dependencies and minified
@sentry/browser - Webpack
Size limit: 100 KB
Size: 22.16 KB with all dependencies, minified and gzipped
@sentry/browser - Webpack - gzip = false
Size limit: 100 KB
Size: 75.87 KB with all dependencies and minified
@sentry/react - Webpack
Size limit: 100 KB
Size: 22.19 KB with all dependencies, minified and gzipped
@sentry/nextjs Client - Webpack
Size limit: 100 KB
Size: 46.26 KB with all dependencies, minified and gzipped
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped)
Size limit: 100 KB
Size: 28.18 KB with all dependencies, minified and gzipped
I'm gonna tag @kamilogorek for a review for some added confidence. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! We have all custom made changes covered with tests, so I trust them just fine.
Refactors and reorganises typekit unit tests:
message
andname
which were previously not checkedMakes a few simplifications to tracekit parser including:
args
address at
stripping to the Chrome regexerror.columnNumber
since it's not used and even the original typekit tests expect what appears in the stack string