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

feat(utils): Use globalThis #11351

Merged
merged 1 commit into from
Apr 2, 2024
Merged

feat(utils): Use globalThis #11351

merged 1 commit into from
Apr 2, 2024

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Mar 29, 2024

Now we've dropped Node v12 we can simplify to globalThis:

#5611 (comment)

Copy link

codecov bot commented Mar 29, 2024

Bundle Report

Changes will decrease total bundle size by 71.33kB ⬇️

Bundle name Size Change
@sentry-internal/replay-esm 306.46kB 0 bytes
@sentry-internal/replay-cjs 306.35kB 0 bytes
@sentry-internal/replay-canvas-esm 29.43kB 0 bytes
@sentry/node-cjs 334.49kB 2.47kB ⬇️
@sentry-internal/node-integration-tests-cjs 1.04kB 0 bytes
@sentry/bun-esm 10.05kB 0 bytes
@sentry/node-esm 331.09kB 2.47kB ⬇️
@sentry/utils-cjs 176.9kB 1.85kB ⬇️
@sentry/google-cloud-serverless-cjs 23.0kB 0 bytes
@sentry/utils-esm 172.32kB 1.85kB ⬇️
@sentry/core-cjs 240.44kB 0 bytes
@sentry/react-esm 41.18kB 0 bytes
@sentry/browser-cjs 107.36kB 0 bytes
@sentry/astro-cjs 27.13kB 0 bytes
@sentry/wasm-cjs 5.2kB 0 bytes
@sentry-internal/replay-canvas-cjs 29.51kB 0 bytes
@sentry/svelte-cjs 13.84kB 0 bytes
@sentry/google-cloud-serverless-esm 19.16kB 0 bytes
@sentry/astro-esm 23.39kB 0 bytes
@sentry-internal/node-integration-tests-esm 888 bytes 0 bytes
@sentry/aws-serverless-cjs 14.62kB 0 bytes
@sentry/browser-esm 104.53kB 0 bytes
@sentry/bun-cjs 13.5kB 0 bytes
@sentry/core-esm 236.82kB 0 bytes
@sentry/gatsby-cjs 905 bytes 0 bytes
@sentry/vue-esm 18.85kB 0 bytes
@sentry-internal/integration-shims-cjs 3.65kB 0 bytes
@sentry/sveltekit-cjs 69.31kB 0 bytes
@sentry/sveltekit-esm 61.08kB 0 bytes
@sentry/remix-cjs 53.62kB 0 bytes
@sentry/gatsby-esm 385 bytes 0 bytes
@sentry/remix-esm 48.23kB 0 bytes
@sentry/nextjs-cjs 20.52kB 0 bytes
@sentry/profiling-node-cjs 25.5kB 0 bytes
@sentry/profiling-node-esm 25.52kB 0 bytes
@sentry/nextjs-esm 20.02kB 0 bytes
@sentry/vue-cjs 20.19kB 0 bytes
@sentry/vercel-edge-cjs 18.23kB 0 bytes
@sentry-internal/integration-shims-esm 2.99kB 0 bytes
@sentry-internal/feedback-cjs 65.81kB 0 bytes
@sentry/vercel-edge-esm 16.13kB 0 bytes
@sentry/opentelemetry-cjs 68.45kB 0 bytes
@sentry-internal/tracing-cjs 108.01kB 0 bytes
@sentry-internal/feedback-esm 65.5kB 0 bytes
@sentry/opentelemetry-esm 67.4kB 0 bytes
@sentry-internal/tracing-esm 107.26kB 0 bytes
@sentry/types-cjs (removed) 35 bytes ⬇️
@sentry/types-esm (removed) 35 bytes ⬇️
@sentry/wasm-esm (removed) 4.85kB ⬇️
@sentry/svelte-esm (removed) 12.72kB ⬇️
@sentry/react-cjs (removed) 45.04kB ⬇️

Copy link
Contributor

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay, Feedback) 80.41 KB (-0.07% 🔽)
@sentry/browser (incl. Tracing, Replay) 71.75 KB (-0.07% 🔽)
@sentry/browser (incl. Tracing, Replay with Canvas) 75.56 KB (-0.07% 🔽)
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 65.32 KB (-0.06% 🔽)
@sentry/browser (incl. Tracing) 36.58 KB (-0.12% 🔽)
@sentry/browser (incl. browserTracingIntegration) 36.58 KB (-0.12% 🔽)
@sentry/browser (incl. feedbackIntegration) 31.46 KB (-0.23% 🔽)
@sentry/browser (incl. feedbackModalIntegration) 31.57 KB (-0.21% 🔽)
@sentry/browser (incl. feedbackScreenshotIntegration) 31.58 KB (-0.21% 🔽)
@sentry/browser (incl. sendFeedback) 27.54 KB (-0.21% 🔽)
@sentry/browser 22.69 KB (-0.26% 🔽)
CDN Bundle (incl. Tracing, Replay, Feedback) 74.86 KB (-0.08% 🔽)
CDN Bundle (incl. Tracing, Replay) 69.67 KB (-0.1% 🔽)
CDN Bundle (incl. Tracing) 36.22 KB (-0.15% 🔽)
CDN Bundle 23.96 KB (-0.27% 🔽)
CDN Bundle (incl. Tracing, Replay) - uncompressed 218.59 KB (-0.1% 🔽)
CDN Bundle (incl. Tracing) - uncompressed 109.24 KB (-0.2% 🔽)
CDN Bundle - uncompressed 70.87 KB (-0.29% 🔽)
@sentry/react (incl. Tracing, Replay) 71.73 KB (-0.07% 🔽)
@sentry/react 22.71 KB (-0.31% 🔽)

@timfish timfish marked this pull request as ready for review March 29, 2024 22:59
@AbhiPrasad
Copy link
Member

I think this is a good idea, but I'm a little scared about edge cases that we will encounter if we are less defensive of how we access the global in the browser environment. Does that fear seem reasonable?

@timfish
Copy link
Collaborator Author

timfish commented Apr 2, 2024

Does that fear seem reasonable?

I don't think this is any riskier than the changes to target ES20xx. Every browser we support has supported it since at least late 2019 and for older browsers it's easy to polyfill.

The MDN docs say:

it's guaranteed to work in window and non-window contexts

What sort of edge cases are you thinking about?

@AbhiPrasad
Copy link
Member

What sort of edge cases are you thinking about?

breaking react native, web workers, or extension context, but you know what - doing more reading I think we are fine. Let's merge!

@AbhiPrasad AbhiPrasad merged commit ed4a751 into develop Apr 2, 2024
94 checks passed
@AbhiPrasad AbhiPrasad deleted the timfish/globalThis branch April 2, 2024 20:33
cadesalaberry pushed a commit to cadesalaberry/sentry-javascript that referenced this pull request Apr 19, 2024
Now we've dropped Nove v12 we can simplify to `globalThis`:

getsentry#5611 (comment)
Lms24 pushed a commit that referenced this pull request Jul 31, 2024
Since v8, all platforms and versions we support have support for
`globalThis`. Since [this
PR](#11351) we also
use `globalThis` when reading these injectected values.
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