Skip to content

Conversation

@mydea
Copy link
Member

@mydea mydea commented Dec 15, 2023

To align this with OpenTelemetry and make some things possible that are currently not easily doable without pushScope / popScope.

Noticed this because currently it's not easily possible to e.g. use withScope in places like this.

This should be backwards compatible because any code that previously relied on this returning void should still work.

@mydea mydea requested review from AbhiPrasad and Lms24 December 15, 2023 12:08
@mydea mydea self-assigned this Dec 15, 2023
@mydea mydea requested a review from lforst December 15, 2023 12:09
@github-actions
Copy link
Contributor

github-actions bot commented Dec 15, 2023

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay, Feedback) - Webpack (gzipped) 75.17 KB (-0.04% 🔽)
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped) 66.58 KB (0%)
@sentry/browser (incl. Tracing, Replay) - Webpack with treeshaking flags (gzipped) 60.18 KB (0%)
@sentry/browser (incl. Tracing) - Webpack (gzipped) 31.29 KB (-0.01% 🔽)
@sentry/browser (incl. Feedback) - Webpack (gzipped) 29.89 KB (-0.08% 🔽)
@sentry/browser - Webpack (gzipped) 21.58 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing, Replay, Feedback) - ES6 CDN Bundle (gzipped) 72.53 KB (-0.03% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped) 64.3 KB (+0.01% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped) 30.54 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped) 22.6 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed) 202.31 KB (+0.01% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed) 92.51 KB (+0.02% 🔺)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed) 67.54 KB (+0.03% 🔺)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped) 33.45 KB (0%)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped) 66.96 KB (-0.01% 🔽)
@sentry/react - Webpack (gzipped) 21.62 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped) 83.7 KB (0%)
@sentry/nextjs Client - Webpack (gzipped) 48.4 KB (-0.01% 🔽)
@sentry-internal/feedback - Webpack (gzipped) 16.18 KB (-0.11% 🔽)

@mydea
Copy link
Member Author

mydea commented Dec 15, 2023

Noticed that linting may fail with this update - with the @typescript-eslint/no-floating-promises rule if you had withScope(async () => {}) without voiding/returning/using this, lint may complain. But I think we should still do it, otherwise a bunch of the pre-v8 refactors (getting rid of pushScope internally etc) will be impossible to do 😬

To align this with OpenTelemetry and make some things possible that are currently not easily doable without `pushScope` / `popScope`.
if ([FEEDBACK_API_SOURCE, FEEDBACK_WIDGET_SOURCE].includes(String(source))) {
scope.setLevel('info');
}
return withScope(async scope => {
Copy link
Member Author

Choose a reason for hiding this comment

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

This actually shows how much better this is :D

@mydea mydea merged commit f2a4caa into develop Dec 18, 2023
@mydea mydea deleted the fn/withScope branch December 18, 2023 08:55
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