Skip to content

ref(core): Prevent redundant setup work #3862

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

Merged
merged 2 commits into from
Aug 3, 2021

Conversation

lobsterkatie
Copy link
Member

This PR fixes two places where we do work unnecessarily:

  1. When creating a new Hub instance we call bindClient, even if there's no client to bind.

  2. When calling Client.setupIntegrations() (as we do when binding a client to a hub), we run through the whole integration-setup process, even if the client has already had its integrations set up.

(The latter situation comes up, among other places, any time we create a domain: each new domain gets its own hub, which then is bound to the current client, meaning we currently call setupIntegrations() for every incoming request handled by either our Express integration or our nextjs SDK.)

To prevent this, we now (as of this change) check a) if there's a client to bind to the new hub, and b) if the client having its integrations set up has already had that done.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2021

size-limit report

Path Size
@sentry/browser - CDN Bundle (gzipped) 21.55 KB (+0.1% 🔺)
@sentry/browser - Webpack 22.55 KB (+0.1% 🔺)
@sentry/react - Webpack 22.58 KB (+0.1% 🔺)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped) 28.97 KB (+0.06% 🔺)

@lobsterkatie lobsterkatie merged commit efa6c45 into master Aug 3, 2021
@lobsterkatie lobsterkatie deleted the kmclb-prevent-redundant-core-setup branch August 3, 2021 15:50
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.

2 participants