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

ref(browser): Skip browser extension warning in non-debug builds #15310

Merged
merged 3 commits into from
Feb 6, 2025

Conversation

mydea
Copy link
Member

@mydea mydea commented Feb 5, 2025

I think it is fair to exclude this here, saving a few bytes...?

@mydea mydea requested review from Lms24 and s1gr1d February 5, 2025 14:39
@mydea mydea self-assigned this Feb 5, 2025
Copy link
Contributor

github-actions bot commented Feb 5, 2025

size-limit report 📦

Path Size % Change Change
@sentry/browser 22.9 KB -0.01% -1 B 🔽
@sentry/browser - with treeshaking flags 22.69 KB -0.37% -84 B 🔽
@sentry/browser (incl. Tracing) 35.77 KB +0.01% +1 B 🔺
@sentry/browser (incl. Tracing, Replay) 72.58 KB +0.01% +2 B 🔺
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 66.05 KB -0.12% -80 B 🔽
@sentry/browser (incl. Tracing, Replay with Canvas) 76.82 KB +0.01% +1 B 🔺
@sentry/browser (incl. Tracing, Replay, Feedback) 89.56 KB +0.01% +2 B 🔺
@sentry/browser (incl. Feedback) 39.84 KB +0.01% +2 B 🔺
@sentry/browser (incl. sendFeedback) 27.53 KB +0.01% +1 B 🔺
@sentry/browser (incl. FeedbackAsync) 32.31 KB +0.01% +3 B 🔺
@sentry/react 24.73 KB -0.01% -2 B 🔽
@sentry/react (incl. Tracing) 37.66 KB -0.02% -4 B 🔽
@sentry/vue 27.09 KB +0.01% +1 B 🔺
@sentry/vue (incl. Tracing) 37.47 KB -0.01% -1 B 🔽
@sentry/svelte 22.93 KB -0.01% -1 B 🔽
CDN Bundle 24.13 KB -0.31% -76 B 🔽
CDN Bundle (incl. Tracing) 35.84 KB -0.22% -80 B 🔽
CDN Bundle (incl. Tracing, Replay) 70.48 KB -0.12% -85 B 🔽
CDN Bundle (incl. Tracing, Replay, Feedback) 75.62 KB -0.12% -89 B 🔽
CDN Bundle - uncompressed 70.58 KB -0.26% -183 B 🔽
CDN Bundle (incl. Tracing) - uncompressed 106.46 KB -0.17% -183 B 🔽
CDN Bundle (incl. Tracing, Replay) - uncompressed 217.31 KB -0.09% -183 B 🔽
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 229.88 KB -0.08% -183 B 🔽
@sentry/nextjs (client) 38.63 KB -0.01% -1 B 🔽
@sentry/sveltekit (client) 36.2 KB +0.01% +1 B 🔺
@sentry/node 156.51 KB - -
@sentry/node - without tracing 97.52 KB -0.01% -1 B 🔽
@sentry/aws-serverless 106.99 KB -0.01% -1 B 🔽

View base workflow run

Copy link

codecov bot commented Feb 5, 2025

❌ 2 Tests Failed:

Tests completed Failed Passed Skipped
633 2 631 300
View the top 2 failed tests by shortest run time
manual-client/skip-init-chrome-extension/test.ts should not initialize when inside a Chrome browser extension
Stack Traces | 0.142s run time
test.ts:4:11 should not initialize when inside a Chrome browser extension
manual-client/skip-init-browser-extension/test.ts should not initialize when inside a Firefox/Safari browser extension
Stack Traces | 0.144s run time
test.ts:4:11 should not initialize when inside a Firefox/Safari browser extension

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

@@ -172,7 +172,7 @@ declare const __SENTRY_RELEASE__: string | undefined;
export function init(browserOptions: BrowserOptions = {}): Client | undefined {
const options = applyDefaultOptions(browserOptions);

if (!options.skipBrowserExtensionCheck && shouldShowBrowserExtensionError()) {
if (DEBUG_BUILD && !options.skipBrowserExtensionCheck && shouldShowBrowserExtensionError()) {
Copy link
Member

Choose a reason for hiding this comment

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

A side-effect to keep in mind is that our regular CDN bundles would then still init the SDK in browser extensions. If it was just about the warning, I'd say that's fine but we bail in the same block. Should we just guard the console log part instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

ah, yeah, you are right of course 🤔 i mean, this will have less savings (as the code to check all of this still runs) but is probably safer 👍

@mydea mydea force-pushed the fn/guard-browser-extension-check branch from cae14ab to 514ca96 Compare February 5, 2025 16:33
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

thx for changing!

@mydea mydea force-pushed the fn/guard-browser-extension-check branch from 514ca96 to 5d249bc Compare February 6, 2025 08:56
@mydea mydea merged commit 24409e3 into develop Feb 6, 2025
111 of 113 checks passed
@mydea mydea deleted the fn/guard-browser-extension-check branch February 6, 2025 09:47
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.

2 participants