Skip to content

feat: Allow for multiple onLoad callbacks in loader #1797

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 1 commit into from
Dec 13, 2018
Merged

Conversation

kamilogorek
Copy link
Contributor

@kamilogorek kamilogorek commented Dec 13, 2018

This will allow @sentry/minimal users to load SDK on demand and make sure their calls are sent through as well.

Example usage:

<!doctype html>
<html>
  <head>
    <script src="https://loader/from/our/cdn.min.js"></script>
    <script>
      Sentry.onLoad(() => {
        Sentry.init({
          release: '1.0.0',
          environment: 'ianlocal',
        }); 
      });
    </script>
    <script src="app.js"></script>
  </head>
  <body></body>
</html>

And in app.js:

const Sentry = require('@sentry/minimal');

function captureWithSentry(error, info, props) {
  // Check for an existence of global Sentry object, which can be the loader
  const sentryLoader = window.Sentry;
  // If it has `SDK_VERSION`, then it's full SDK. So either loader fetched the SDK
  // or it's a CDN version. Doesn't matter at this point really. (could be improved tbh)
  const isSentryLoaded = sentryLoader && sentryLoader.SDK_VERSION;
  
  // If it's loaded, just use @sentry/minimal and send all the calls through
  if (isSentryLoaded) {
    sentryCall(error, info, props);
  }

  // If it's not loaded, hook up to the `onLoad` callback to queue the call and pull down the SDK
  if (!isSentryLoaded) {
    sentryLoader.onLoad(() => sentryCall(error, info, props));
    sentryLoader.forceLoad();
  }
}

function sentryCall(tags, extra) {
  Sentry.withScope(scope => {
    Object.entries(tags).forEach(([key, value]) => scope.setTag(key, value));
    Object.entries(extra).forEach(([key, value]) => scope.setExtra(key, value));
    Sentry.captureException(error);
  }); 
}

// some code that uses it, eg. React's ErrorBoundry component

It's the most reasonable solution for now, as otherwise, we'd have to either modify hub, minimal or the loader and introduce a direct dependency between them, which we definitely don't want.

Also cc @ndmanvar.

@kamilogorek kamilogorek requested a review from HazAT December 13, 2018 12:47
@kamilogorek kamilogorek changed the title feat: allow for multiple onLoad callbacks in loader feat: Allow for multiple onLoad callbacks in loader Dec 13, 2018
@getsentry-bot
Copy link
Contributor

Fails
🚫

TSLint failed: @sentry/utils

  • ERROR: /home/travis/build/getsentry/sentry-javascript/packages/utils/test/object.test.ts[1, 36]: Named imports must be alphabetized.
Warnings
⚠️ Please add a changelog entry for your changes.
Messages
📖 @sentry/browser gzip'ed minified size: 21.8262 kB

Generated by 🚫 dangerJS

@HazAT HazAT merged commit c38a7a1 into master Dec 13, 2018
@HazAT HazAT deleted the multiple-onload branch December 13, 2018 18:09
@IanMitchell
Copy link

Thank you guys for this! Really appreciated 😄

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.

4 participants