-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(utils): Introduce getGlobalSingleton helper #4860
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
Conversation
This helper abstracts away the logic of getting an variable from the global `__SENTRY__` object. It does this by introducing a new `getGlobalSingleton` utility function. For now, this is used by the event processors and hub logic. In an upcoming patch, we'll update the logger logic to also use this utility.
size-limit report 📦
|
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.
Good change!
packages/hub/src/hub.ts
Outdated
@@ -631,7 +636,7 @@ export function getHubFromCarrier(carrier: Carrier): Hub { | |||
*/ | |||
export function setHubOnCarrier(carrier: Carrier, hub: Hub): boolean { | |||
if (!carrier) return false; | |||
carrier.__SENTRY__ = carrier.__SENTRY__ || {}; | |||
carrier.__SENTRY__.hub = hub; | |||
const sentry = (carrier.__SENTRY__ = carrier.__SENTRY__ || {}); |
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.
const sentry = (carrier.__SENTRY__ = carrier.__SENTRY__ || {}); | |
const sentry = carrier.__SENTRY__ = carrier.__SENTRY__ || {}; |
Will prettier let you do this without ()
? If so, I think it's easier to read without them.
Also, what do you think of calling this variable __SENTRY__
, since that's what it is?
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.
switched to variable __SENTRY__
, can't make the ()
change because of prettier.
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!
@AbhiPrasad I'm getting this error when I follow the directions at https://docs.sentry.io/platforms/javascript/guides/nextjs/#install and I think it's related to what you did here:
Do you have any ideas how I can make this error go away? |
Hey @zeckdude to help us take a look, could you open a GitHub issue with:
On the latest version, this function should be existing and properly exported. See it on the unpkg CDN here: https://unpkg.com/browse/@sentry/[email protected]/esm/global.js |
This helper abstracts away the logic of getting an variable from the global
__SENTRY__
object. It does this by introducing a newgetGlobalSingleton
utility function.For now, this is used by the event processors and hub logic. In an upcoming PR, we'll update the logger logic to also use this utility.
Based on the excellent work in #4285 -
should be a small bundle size win as well.Well ok size-bot, say no more.Resolves https://getsentry.atlassian.net/browse/WEB-796