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(browser): Remove browserPerformanceTimeOrigin side-effects #14025

Draft
wants to merge 15 commits into
base: develop
Choose a base branch
from

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Oct 18, 2024

Due to the side-effect, browserPerformanceTimeOrigin ends up in the non-tracing browser bundle.

This change ensures that the code is only included when tracing is in use at a slight cost to the tracing bundles.

Copy link
Contributor

github-actions bot commented Oct 18, 2024

size-limit report 📦

⚠️ Warning: Base artifact is not the latest one, because the latest workflow run is not done yet. This may lead to incorrect results. Try to re-run all tests to get up to date results.

Path Size % Change Change
@sentry/browser 23.16 KB -0.54% -128 B 🔽
@sentry/browser - with treeshaking flags 21.84 KB -0.54% -120 B 🔽
@sentry/browser (incl. Tracing) 35.81 KB +0.05% +18 B 🔺
@sentry/browser (incl. Tracing, Replay) 73.05 KB +0.03% +21 B 🔺
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 63.43 KB +0.03% +18 B 🔺
@sentry/browser (incl. Tracing, Replay with Canvas) 77.47 KB +0.03% +21 B 🔺
@sentry/browser (incl. Tracing, Replay, Feedback) 89.85 KB +0.02% +15 B 🔺
@sentry/browser (incl. Feedback) 39.92 KB -0.3% -121 B 🔽
@sentry/browser (incl. sendFeedback) 27.78 KB -0.42% -119 B 🔽
@sentry/browser (incl. FeedbackAsync) 32.55 KB -0.41% -134 B 🔽
@sentry/react 25.85 KB -0.44% -115 B 🔽
@sentry/react (incl. Tracing) 38.62 KB +0.05% +19 B 🔺
@sentry/vue 27.37 KB -0.44% -122 B 🔽
@sentry/vue (incl. Tracing) 37.65 KB +0.06% +23 B 🔺
@sentry/svelte 23.33 KB -0.53% -127 B 🔽
CDN Bundle 24.34 KB -0.37% -92 B 🔽
CDN Bundle (incl. Tracing) 37.51 KB +0.16% +59 B 🔺
CDN Bundle (incl. Tracing, Replay) 72.58 KB +0.05% +37 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) 77.95 KB +0.03% +17 B 🔺
CDN Bundle - uncompressed 71.5 KB -0.34% -244 B 🔽
CDN Bundle (incl. Tracing) - uncompressed 111.2 KB +0.14% +159 B 🔺
CDN Bundle (incl. Tracing, Replay) - uncompressed 225.25 KB +0.07% +161 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 238.47 KB +0.07% +161 B 🔺
@sentry/nextjs (client) 38.89 KB +0.06% +21 B 🔺
@sentry/sveltekit (client) 36.31 KB +0.06% +20 B 🔺
@sentry/node 162.65 KB -0.07% -110 B 🔽
@sentry/node - without tracing 98.77 KB -0.13% -124 B 🔽
@sentry/aws-serverless 126.51 KB -0.1% -121 B 🔽

View base workflow run

@timfish
Copy link
Collaborator Author

timfish commented Dec 4, 2024

@mydea or @billyvg do you know why my changes could be causing this Replay test failure with an incorrect segmentId?
https://github.com/getsentry/sentry-javascript/actions/runs/12156896052/job/33901817087?pr=14025#step:6:426

@timfish
Copy link
Collaborator Author

timfish commented Dec 13, 2024

@lforst since this is now a breaking change, can I rebase this PR on top of the v9 branch?

@lforst
Copy link
Member

lforst commented Dec 13, 2024

@lforst since this is now a breaking change, can I rebase this PR on top of the v9 branch?

We already cut the v8 branch so the branch to target with breaking changes is now develop.

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.

3 participants