ref(core): Rename Hub to AsyncContextStack & remove unneeded methods
#11630
Conversation
Should save some bundle size...!
| const sentry = getSentryCarrier(registry) as { hub?: AsyncContextStack }; | ||
|
|
||
| // If there's no hub, or its an old API, assign a new one | ||
| if (sentry.hub) { | ||
| return sentry.hub; | ||
| } | ||
|
|
||
| sentry.hub = new AsyncContextStack(getDefaultCurrentScope(), getDefaultIsolationScope()); | ||
| return sentry.hub; |
There was a problem hiding this comment.
can we stop referencing it by .hub here?
There was a problem hiding this comment.
sadly not because some stuff like the loader depends on this 😬 @Lms24 learned this the hard way I believe?
There was a problem hiding this comment.
that's also the reason that this still has a getClient() method!
There was a problem hiding this comment.
yes, if we remove/rename .hub our loader tests will fail because our loader script accesses __SENTRY__.hub.getClient() to check if the SDK was initialized before the loader otherwise initializes the SDK (at least that's what I think it does; the script certainly makes the access). So I guess we can only remove this once we update the loader script 🤔
There was a problem hiding this comment.
we can still do this when v8 becomes stable I guess, but it's possibly not worth it too as it may break stuff weirdly I guess that depends on internals 😅
There was a problem hiding this comment.
could we leave a comment about this somewhere?
size-limit report 📦
|
Added a comment for #11630 (comment)!
Now that #11630 was merged in, we can remove `setupGlobalHub`.
Should save some bundle size...! I also moved stuff around a bit so we don't have so much somewhat unrelated stuff packed into the former
hub.tsfile - needed some work to avoid circular dependencies, but I think it's fine now.I left the stuff we actually need in, and renamed it for clarity so this is not confusing anymore.
closes #11482