-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
build(replay): Provide full browser+tracing+replay bundle #6793
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -105,13 +105,7 @@ You have to add it in addition to the Sentry Browser SDK bundle: | |
| ```js | ||
| // Browser SDK bundle | ||
| <script | ||
| src="https://browser.sentry-cdn.com/7.24.1/bundle.tracing.min.js" | ||
| crossorigin="anonymous" | ||
| ></script> | ||
|
|
||
| // Replay integration bundle | ||
| <script | ||
| src="https://browser.sentry-cdn.com/7.24.1/replay.min.js" | ||
| src="https://browser.sentry-cdn.com/7.31.0/bundle.tracing.replay.min.js" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't comment there directly but: |
||
| crossorigin="anonymous" | ||
| ></script> | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,9 @@ | ||
| import replace from '@rollup/plugin-replace'; | ||
|
|
||
| import { makeBaseBundleConfig, makeBundleConfigVariants } from '../../rollup/index.js'; | ||
|
|
||
| import pkg from './package.json'; | ||
|
|
||
| const builds = []; | ||
|
|
||
| ['es5', 'es6'].forEach(jsVersion => { | ||
|
|
@@ -14,4 +18,26 @@ const builds = []; | |
| builds.push(...makeBundleConfigVariants(baseBundleConfig)); | ||
| }); | ||
|
|
||
| // Full bundle incl. replay only avaialable for es6 | ||
| const replayBaseBundleConfig = makeBaseBundleConfig({ | ||
| bundleType: 'standalone', | ||
| entrypoints: ['src/index.bundle.replay.ts'], | ||
| jsVersion: 'es6', | ||
| licenseTitle: '@sentry/tracing & @sentry/browser & @sentry/replay', | ||
| outputFileBase: () => 'bundles/bundle.tracing.replay', | ||
| includeReplay: true, | ||
| packageSpecificConfig: { | ||
| plugins: [ | ||
| replace({ | ||
| preventAssignment: true, | ||
| values: { | ||
| __SENTRY_REPLAY_VERSION__: JSON.stringify(pkg.version), | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reminder to ourselves: In the next version, we should get rid of this. By now all docs etc. point to importing replay directly from @sentry/browser, so this should always be in sync.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we document this here #5194 or in a new issue?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh wait you mean next version of the SDK, not next major version. Ignore me!
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this needs to be in a major release. We're tracking this in #6366 |
||
| }, | ||
| }), | ||
| ], | ||
| }, | ||
| }); | ||
|
|
||
| builds.push(...makeBundleConfigVariants(replayBaseBundleConfig)); | ||
|
|
||
| export default builds; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| import { Replay } from '@sentry/browser'; | ||
|
|
||
| import * as Sentry from './index.bundle'; | ||
|
|
||
| Sentry.Integrations.Replay = Replay; | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was the easiest/cleanest way I could find to handle this in a dedicated input file. Otherwise it gets quite messy when trying to do this inside of
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: This means we only have a single default export here, and can't do our usual
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is fine and with this change, the full CDN bundle usage is identical to the addon bundle (docs). |
||
|
|
||
| export default 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.
l: Hmm it probably makes sense to show the full CDN bundle here but I'd like to keep the addon bundle around and documented (at least in docs) as I think some people might prefer it 🤔 Especially, if we're considering using the loader to lazy load it in the future.
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.
Hmm yeah, I guess the reason to use it is if you want replay but not tracing - fair! I can add both somehow - probably leave the "full" bundle on top and add a section below on standalone usage.
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.
Added another section below 👍