From 299e18c540c80dd1fa1758cd89185919ae97b2fb Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 16 Mar 2023 11:32:20 +0100 Subject: [PATCH 1/4] feat(tracing): Expose `BrowserTracing` in non-tracing bundles In order to avoid breaking the bundler when a user opts out of tracing there. --- .../suites/replay/replayShim/test.ts | 4 +-- .../suites/tracing/browserTracingShim/init.js | 9 +++++ .../tracing/browserTracingShim/template.html | 9 +++++ .../suites/tracing/browserTracingShim/test.ts | 33 +++++++++++++++++++ packages/browser/rollup.bundle.config.js | 2 ++ .../integration-shims/src/BrowserTracing.ts | 31 +++++++++++++++++ packages/integration-shims/src/Replay.ts | 2 +- packages/integration-shims/src/index.ts | 3 +- rollup/bundleHelpers.js | 15 ++++----- rollup/plugins/bundlePlugins.js | 19 ++++++----- 10 files changed, 105 insertions(+), 22 deletions(-) create mode 100644 packages/browser-integration-tests/suites/tracing/browserTracingShim/init.js create mode 100644 packages/browser-integration-tests/suites/tracing/browserTracingShim/template.html create mode 100644 packages/browser-integration-tests/suites/tracing/browserTracingShim/test.ts create mode 100644 packages/integration-shims/src/BrowserTracing.ts diff --git a/packages/browser-integration-tests/suites/replay/replayShim/test.ts b/packages/browser-integration-tests/suites/replay/replayShim/test.ts index ca11e1c431f2..5cb297551dfe 100644 --- a/packages/browser-integration-tests/suites/replay/replayShim/test.ts +++ b/packages/browser-integration-tests/suites/replay/replayShim/test.ts @@ -30,8 +30,6 @@ sentryTest( await forceFlushReplay(); expect(requestCount).toBe(0); - expect(consoleMessages).toEqual([ - 'You are using new Replay() even though this bundle does not include the replay integration.', - ]); + expect(consoleMessages).toEqual(['You are using new Replay() even though this bundle does not include replay.']); }, ); diff --git a/packages/browser-integration-tests/suites/tracing/browserTracingShim/init.js b/packages/browser-integration-tests/suites/tracing/browserTracingShim/init.js new file mode 100644 index 000000000000..737322575d3b --- /dev/null +++ b/packages/browser-integration-tests/suites/tracing/browserTracingShim/init.js @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + sampleRate: 1, + integrations: [new Sentry.Integrations.BrowserTracing()], +}); diff --git a/packages/browser-integration-tests/suites/tracing/browserTracingShim/template.html b/packages/browser-integration-tests/suites/tracing/browserTracingShim/template.html new file mode 100644 index 000000000000..2b3e2f0b27b4 --- /dev/null +++ b/packages/browser-integration-tests/suites/tracing/browserTracingShim/template.html @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/packages/browser-integration-tests/suites/tracing/browserTracingShim/test.ts b/packages/browser-integration-tests/suites/tracing/browserTracingShim/test.ts new file mode 100644 index 000000000000..f920e8ec3cf6 --- /dev/null +++ b/packages/browser-integration-tests/suites/tracing/browserTracingShim/test.ts @@ -0,0 +1,33 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../utils/fixtures'; + +sentryTest('exports a shim BrowserTracing integration for non-tracing bundles', async ({ getLocalTestPath, page }) => { + const bundle = process.env.PW_BUNDLE; + + if (!bundle || !bundle.startsWith('bundle_') || bundle.includes('tracing')) { + sentryTest.skip(); + } + + const consoleMessages: string[] = []; + page.on('console', msg => consoleMessages.push(msg.text())); + + let requestCount = 0; + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + requestCount++; + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + const url = await getLocalTestPath({ testDir: __dirname }); + + await page.goto(url); + + expect(requestCount).toBe(0); + expect(consoleMessages).toEqual([ + 'You are using new BrowserTracing() even though this bundle does not include tracing.', + ]); +}); diff --git a/packages/browser/rollup.bundle.config.js b/packages/browser/rollup.bundle.config.js index 380ca59eed95..846a033ed57e 100644 --- a/packages/browser/rollup.bundle.config.js +++ b/packages/browser/rollup.bundle.config.js @@ -9,6 +9,7 @@ const builds = []; jsVersion, licenseTitle: '@sentry/browser', includeReplay: 'shim', + includeBrowserTracing: 'shim', outputFileBase: () => `bundles/bundle${jsVersion === 'es5' ? '.es5' : ''}`, }); @@ -23,6 +24,7 @@ const replayBaseBundleConfig = makeBaseBundleConfig({ licenseTitle: '@sentry/browser & @sentry/replay', outputFileBase: () => 'bundles/bundle.replay', includeReplay: true, + includeBrowserTracing: 'shim', }); builds.push(...makeBundleConfigVariants(replayBaseBundleConfig)); diff --git a/packages/integration-shims/src/BrowserTracing.ts b/packages/integration-shims/src/BrowserTracing.ts new file mode 100644 index 000000000000..f371c3805eb1 --- /dev/null +++ b/packages/integration-shims/src/BrowserTracing.ts @@ -0,0 +1,31 @@ +import type { Integration } from '@sentry/types'; + +/** + * This is a shim for the BrowserTracing integration. + * It is needed in order for the CDN bundles to continue working when users add/remove tracing + * from it, without changing their config. This is necessary for the loader mechanism. + */ +class BrowserTracingShim implements Integration { + /** + * @inheritDoc + */ + public static id: string = 'BrowserTracing'; + + /** + * @inheritDoc + */ + public name: string = BrowserTracingShim.id; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + public constructor(_options: any) { + // eslint-disable-next-line no-console + console.error('You are using new BrowserTracing() even though this bundle does not include tracing.'); + } + + /** jsdoc */ + public setupOnce(): void { + // noop + } +} + +export { BrowserTracingShim as BrowserTracing }; diff --git a/packages/integration-shims/src/Replay.ts b/packages/integration-shims/src/Replay.ts index c89bbd415d5f..3b81addb404d 100644 --- a/packages/integration-shims/src/Replay.ts +++ b/packages/integration-shims/src/Replay.ts @@ -19,7 +19,7 @@ class ReplayShim implements Integration { // eslint-disable-next-line @typescript-eslint/no-explicit-any public constructor(_options: any) { // eslint-disable-next-line no-console - console.error('You are using new Replay() even though this bundle does not include the replay integration.'); + console.error('You are using new Replay() even though this bundle does not include replay.'); } /** jsdoc */ diff --git a/packages/integration-shims/src/index.ts b/packages/integration-shims/src/index.ts index a55c52bcb483..abff99dee11c 100644 --- a/packages/integration-shims/src/index.ts +++ b/packages/integration-shims/src/index.ts @@ -1 +1,2 @@ -export * from './Replay'; +export { Replay } from './Replay'; +export { BrowserTracing } from './BrowserTracing'; diff --git a/rollup/bundleHelpers.js b/rollup/bundleHelpers.js index 6266717aec0b..eb5515788b08 100644 --- a/rollup/bundleHelpers.js +++ b/rollup/bundleHelpers.js @@ -18,7 +18,7 @@ import { makeTSPlugin, makeExcludeBlockPlugin, makeSetSDKSourcePlugin, - makeReplayShimPlugin, + makeIntegrationShimPlugin, } from './plugins/index.js'; import { mergePlugins } from './utils'; @@ -45,11 +45,11 @@ export function makeBaseBundleConfig(options) { const licensePlugin = makeLicensePlugin(licenseTitle); const tsPlugin = makeTSPlugin(jsVersion.toLowerCase()); const excludeReplayPlugin = makeExcludeBlockPlugin('REPLAY'); - const excludeReplayShimPlugin = makeExcludeBlockPlugin('REPLAY_SHIM'); const excludeOfflineTransport = makeExcludeBlockPlugin('OFFLINE'); const excludeBrowserProfiling = makeExcludeBlockPlugin('BROWSER_PROFILING'); const excludeBrowserTracing = makeExcludeBlockPlugin('BROWSER_TRACING'); - const replayShimPlugin = makeReplayShimPlugin(); + const replayShimPlugin = makeIntegrationShimPlugin('@sentry/replay'); + const browserTracingShimPlugin = makeIntegrationShimPlugin('@sentry-internal/tracing'); // The `commonjs` plugin is the `esModuleInterop` of the bundling world. When used with `transformMixedEsModules`, it // will include all dependencies, imported or required, in the final bundle. (Without it, CJS modules aren't included @@ -68,11 +68,7 @@ export function makeBaseBundleConfig(options) { if (includeReplay === 'shim') { standAloneBundleConfig.plugins.push(replayShimPlugin); - } else { - standAloneBundleConfig.plugins.push(excludeReplayShimPlugin); - } - - if (!includeReplay) { + } else if (!includeReplay) { standAloneBundleConfig.plugins.push(excludeReplayPlugin); } @@ -84,6 +80,9 @@ export function makeBaseBundleConfig(options) { standAloneBundleConfig.plugins.push(excludeBrowserProfiling); } + if (includeBrowserTracing === 'shim') { + standAloneBundleConfig.plugins.push(browserTracingShimPlugin); + } if (!includeBrowserTracing) { standAloneBundleConfig.plugins.push(excludeBrowserTracing); } diff --git a/rollup/plugins/bundlePlugins.js b/rollup/plugins/bundlePlugins.js index f00e090c740c..2eeb6f358031 100644 --- a/rollup/plugins/bundlePlugins.js +++ b/rollup/plugins/bundlePlugins.js @@ -183,9 +183,8 @@ export function makeTSPlugin(jsVersion) { /** * Creates a Rollup plugin that removes all code between the `__ROLLUP_EXCLUDE_FROM_BUNDLES_BEGIN__` - * and `__ROLLUP_EXCLUDE_FROM_BUNDLES_END__` comment guards. This is used to exclude the Replay integration - * from the browser and browser+tracing bundles. - * If we need to add more such guards in the future, we might want to refactor this into a more generic plugin. + * and `__ROLLUP_EXCLUDE_FROM_BUNDLES_END__` comment guards. + * This is used to exclude certain exports from variants of the browser bundle. */ export function makeExcludeBlockPlugin(type) { const replacementRegex = new RegExp( @@ -197,8 +196,10 @@ export function makeExcludeBlockPlugin(type) { const plugin = { transform(code, id) { - const isBrowserIndexFile = path.resolve(id) === browserIndexFilePath; - if (!isBrowserIndexFile || !replacementRegex.test(code)) { + const resolvedPath = path.resolve(id); + const shouldReplaceInFile = resolvedPath === browserIndexFilePath; + + if (!shouldReplaceInFile || !replacementRegex.test(code)) { return null; } @@ -211,20 +212,20 @@ export function makeExcludeBlockPlugin(type) { }, }; - plugin.name = 'excludeReplay'; + plugin.name = 'excludeBlock'; return plugin; } -export function makeReplayShimPlugin() { +export function makeIntegrationShimPlugin(originalImportName) { // This is designed to replace the re-export in browser/index.ts to export the shim const plugin = modify({ - find: '@sentry/replay', + find: originalImportName, replace: '@sentry-internal/integration-shims', }); // give it a nicer name for later, when we'll need to sort the plugins - plugin.name = 'replayShim'; + plugin.name = 'integrationShimPlugin'; return plugin; } From 538ae59666be20be843628cbc3612683a3f1ecc9 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 20 Mar 2023 11:53:24 +0100 Subject: [PATCH 2/4] fix test path --- package.json | 2 - .../browserTracingIntegrationShim/init.js | 12 +++++ .../template.html | 9 ++++ .../browserTracingIntegrationShim/test.ts | 36 +++++++++++++ .../suites/tracing/browserTracingShim/init.js | 5 +- packages/browser/rollup.bundle.config.js | 8 +-- packages/browser/src/index.bundle.base.ts | 23 ++++++++ packages/browser/src/index.bundle.replay.ts | 13 +++++ packages/browser/src/index.bundle.ts | 12 +++++ packages/browser/src/index.ts | 16 ------ .../integration-shims/src/BrowserTracing.ts | 5 ++ packages/integration-shims/src/index.ts | 2 +- packages/tracing/rollup.bundle.config.js | 2 - packages/tracing/src/index.bundle.base.ts | 7 +-- packages/tracing/src/index.bundle.replay.ts | 1 - packages/tracing/src/index.bundle.ts | 10 +++- rollup/bundleHelpers.js | 42 +-------------- rollup/plugins/bundlePlugins.js | 53 ------------------- yarn.lock | 22 -------- 19 files changed, 129 insertions(+), 151 deletions(-) create mode 100644 packages/browser-integration-tests/suites/tracing/browserTracingIntegrationShim/init.js create mode 100644 packages/browser-integration-tests/suites/tracing/browserTracingIntegrationShim/template.html create mode 100644 packages/browser-integration-tests/suites/tracing/browserTracingIntegrationShim/test.ts create mode 100644 packages/browser/src/index.bundle.base.ts create mode 100644 packages/browser/src/index.bundle.replay.ts create mode 100644 packages/browser/src/index.bundle.ts diff --git a/package.json b/package.json index f2177c40bb71..2296a10dfede 100644 --- a/package.json +++ b/package.json @@ -97,7 +97,6 @@ "karma-firefox-launcher": "^1.1.0", "lerna": "6.5.0-alpha.2", "madge": "4.0.2", - "magic-string": "^0.27.0", "mocha": "^6.1.4", "nodemon": "^2.0.16", "npm-run-all": "^4.1.5", @@ -108,7 +107,6 @@ "rollup": "^2.67.1", "rollup-plugin-cleanup": "3.2.1", "rollup-plugin-license": "^2.6.1", - "rollup-plugin-modify": "^3.0.0", "rollup-plugin-terser": "^7.0.2", "rollup-plugin-typescript2": "^0.31.2", "sinon": "^7.3.2", diff --git a/packages/browser-integration-tests/suites/tracing/browserTracingIntegrationShim/init.js b/packages/browser-integration-tests/suites/tracing/browserTracingIntegrationShim/init.js new file mode 100644 index 000000000000..cd05f29615bb --- /dev/null +++ b/packages/browser-integration-tests/suites/tracing/browserTracingIntegrationShim/init.js @@ -0,0 +1,12 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + sampleRate: 1, + integrations: [new Sentry.Integrations.BrowserTracing()], +}); + +// This should not fail +Sentry.addTracingExtensions(); diff --git a/packages/browser-integration-tests/suites/tracing/browserTracingIntegrationShim/template.html b/packages/browser-integration-tests/suites/tracing/browserTracingIntegrationShim/template.html new file mode 100644 index 000000000000..2b3e2f0b27b4 --- /dev/null +++ b/packages/browser-integration-tests/suites/tracing/browserTracingIntegrationShim/template.html @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/packages/browser-integration-tests/suites/tracing/browserTracingIntegrationShim/test.ts b/packages/browser-integration-tests/suites/tracing/browserTracingIntegrationShim/test.ts new file mode 100644 index 000000000000..730170b14344 --- /dev/null +++ b/packages/browser-integration-tests/suites/tracing/browserTracingIntegrationShim/test.ts @@ -0,0 +1,36 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../utils/fixtures'; + +sentryTest( + 'exports a shim Integrations.BrowserTracing integration for non-tracing bundles', + async ({ getLocalTestPath, page }) => { + const bundle = process.env.PW_BUNDLE; + + if (!bundle || !bundle.startsWith('bundle_') || bundle.includes('tracing')) { + sentryTest.skip(); + } + + const consoleMessages: string[] = []; + page.on('console', msg => consoleMessages.push(msg.text())); + + let requestCount = 0; + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + requestCount++; + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + const url = await getLocalTestPath({ testDir: __dirname }); + + await page.goto(url); + + expect(requestCount).toBe(0); + expect(consoleMessages).toEqual([ + 'You are using new BrowserTracing() even though this bundle does not include tracing.', + ]); + }, +); diff --git a/packages/browser-integration-tests/suites/tracing/browserTracingShim/init.js b/packages/browser-integration-tests/suites/tracing/browserTracingShim/init.js index 737322575d3b..95f654dd4428 100644 --- a/packages/browser-integration-tests/suites/tracing/browserTracingShim/init.js +++ b/packages/browser-integration-tests/suites/tracing/browserTracingShim/init.js @@ -5,5 +5,8 @@ window.Sentry = Sentry; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', sampleRate: 1, - integrations: [new Sentry.Integrations.BrowserTracing()], + integrations: [new Sentry.BrowserTracing()], }); + +// This should not fail +Sentry.addTracingExtensions(); diff --git a/packages/browser/rollup.bundle.config.js b/packages/browser/rollup.bundle.config.js index 846a033ed57e..6ef24dd841a5 100644 --- a/packages/browser/rollup.bundle.config.js +++ b/packages/browser/rollup.bundle.config.js @@ -5,11 +5,9 @@ const builds = []; ['es5', 'es6'].forEach(jsVersion => { const baseBundleConfig = makeBaseBundleConfig({ bundleType: 'standalone', - entrypoints: ['src/index.ts'], + entrypoints: ['src/index.bundle.ts'], jsVersion, licenseTitle: '@sentry/browser', - includeReplay: 'shim', - includeBrowserTracing: 'shim', outputFileBase: () => `bundles/bundle${jsVersion === 'es5' ? '.es5' : ''}`, }); @@ -19,12 +17,10 @@ const builds = []; // Full bundle incl. replay only available for es6 const replayBaseBundleConfig = makeBaseBundleConfig({ bundleType: 'standalone', - entrypoints: ['src/index.ts'], + entrypoints: ['src/index.bundle.replay.ts'], jsVersion: 'es6', licenseTitle: '@sentry/browser & @sentry/replay', outputFileBase: () => 'bundles/bundle.replay', - includeReplay: true, - includeBrowserTracing: 'shim', }); builds.push(...makeBundleConfigVariants(replayBaseBundleConfig)); diff --git a/packages/browser/src/index.bundle.base.ts b/packages/browser/src/index.bundle.base.ts new file mode 100644 index 000000000000..464a1170d138 --- /dev/null +++ b/packages/browser/src/index.bundle.base.ts @@ -0,0 +1,23 @@ +export * from './exports'; + +import { Integrations as CoreIntegrations } from '@sentry/core'; +import type { Integration } from '@sentry/types'; + +import { WINDOW } from './helpers'; +import * as BrowserIntegrations from './integrations'; + +let windowIntegrations = {}; + +// This block is needed to add compatibility with the integrations packages when used with a CDN +if (WINDOW.Sentry && WINDOW.Sentry.Integrations) { + windowIntegrations = WINDOW.Sentry.Integrations; +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +const INTEGRATIONS: Record Integration> = { + ...windowIntegrations, + ...CoreIntegrations, + ...BrowserIntegrations, +}; + +export { INTEGRATIONS as Integrations }; diff --git a/packages/browser/src/index.bundle.replay.ts b/packages/browser/src/index.bundle.replay.ts new file mode 100644 index 000000000000..9a220d0ecca9 --- /dev/null +++ b/packages/browser/src/index.bundle.replay.ts @@ -0,0 +1,13 @@ +// This is exported so the loader does not fail when switching off Replay/Tracing +import { addTracingExtensions, BrowserTracing } from '@sentry-internal/integration-shims'; +import { Replay } from '@sentry/replay'; + +import * as Sentry from './index.bundle.base'; + +// TODO (v8): Remove this as it was only needed for backwards compatibility +Sentry.Integrations.Replay = Replay; + +Sentry.Integrations.BrowserTracing = BrowserTracing; + +export * from './index.bundle.base'; +export { BrowserTracing, addTracingExtensions, Replay }; diff --git a/packages/browser/src/index.bundle.ts b/packages/browser/src/index.bundle.ts new file mode 100644 index 000000000000..d16c008e7575 --- /dev/null +++ b/packages/browser/src/index.bundle.ts @@ -0,0 +1,12 @@ +// This is exported so the loader does not fail when switching off Replay/Tracing +import { addTracingExtensions, BrowserTracing, Replay } from '@sentry-internal/integration-shims'; + +import * as Sentry from './index.bundle.base'; + +// TODO (v8): Remove this as it was only needed for backwards compatibility +Sentry.Integrations.Replay = Replay; + +Sentry.Integrations.BrowserTracing = BrowserTracing; + +export * from './index.bundle.base'; +export { BrowserTracing, addTracingExtensions, Replay }; diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index 010ef61e79df..59d814ac9412 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -20,25 +20,9 @@ const INTEGRATIONS = { export { INTEGRATIONS as Integrations }; -// DO NOT DELETE THESE COMMENTS! -// We want to exclude Replay/Offline from CDN bundles, so we remove the block below with our -// makeExcludeBlockPlugin Rollup plugin when generating bundles. Everything between -// ROLLUP_EXCLUDE_*_FROM_BUNDLES_BEGIN and _END__ is removed for bundles. - -// __ROLLUP_EXCLUDE_REPLAY_FROM_BUNDLES_BEGIN__ export { Replay } from '@sentry/replay'; -// __ROLLUP_EXCLUDE_REPLAY_FROM_BUNDLES_END__ - -// __ROLLUP_EXCLUDE_BROWSER_TRACING_FROM_BUNDLES_BEGIN__ export { BrowserTracing } from '@sentry-internal/tracing'; export { addTracingExtensions } from '@sentry/core'; -// __ROLLUP_EXCLUDE_BROWSER_TRACING_FROM_BUNDLES_END__ - -// __ROLLUP_EXCLUDE_OFFLINE_FROM_BUNDLES_BEGIN__ export { makeBrowserOfflineTransport } from './transports/offline'; -// __ROLLUP_EXCLUDE_OFFLINE_FROM_BUNDLES_END__ - -// __ROLLUP_EXCLUDE_BROWSER_PROFILING_FROM_BUNDLES_BEGIN__ export { onProfilingStartRouteTransaction } from './profiling/hubextensions'; export { BrowserProfilingIntegration } from './profiling/integration'; -// __ROLLUP_EXCLUDE_BROWSER_PROFILING_FROM_BUNDLES_END__ diff --git a/packages/integration-shims/src/BrowserTracing.ts b/packages/integration-shims/src/BrowserTracing.ts index f371c3805eb1..5aa1f809b492 100644 --- a/packages/integration-shims/src/BrowserTracing.ts +++ b/packages/integration-shims/src/BrowserTracing.ts @@ -29,3 +29,8 @@ class BrowserTracingShim implements Integration { } export { BrowserTracingShim as BrowserTracing }; + +/** Shim function */ +export function addTracingExtensions(): void { + // noop +} diff --git a/packages/integration-shims/src/index.ts b/packages/integration-shims/src/index.ts index abff99dee11c..1a11515053a8 100644 --- a/packages/integration-shims/src/index.ts +++ b/packages/integration-shims/src/index.ts @@ -1,2 +1,2 @@ export { Replay } from './Replay'; -export { BrowserTracing } from './BrowserTracing'; +export { BrowserTracing, addTracingExtensions } from './BrowserTracing'; diff --git a/packages/tracing/rollup.bundle.config.js b/packages/tracing/rollup.bundle.config.js index 52aa6396a370..a1ff70411f00 100644 --- a/packages/tracing/rollup.bundle.config.js +++ b/packages/tracing/rollup.bundle.config.js @@ -8,7 +8,6 @@ const builds = []; entrypoints: ['src/index.bundle.ts'], jsVersion, licenseTitle: '@sentry/tracing & @sentry/browser', - includeReplay: false, outputFileBase: () => `bundles/bundle.tracing${jsVersion === 'es5' ? '.es5' : ''}`, }); @@ -22,7 +21,6 @@ const replayBaseBundleConfig = makeBaseBundleConfig({ jsVersion: 'es6', licenseTitle: '@sentry/tracing & @sentry/browser & @sentry/replay', outputFileBase: () => 'bundles/bundle.tracing.replay', - includeReplay: true, }); builds.push(...makeBundleConfigVariants(replayBaseBundleConfig)); diff --git a/packages/tracing/src/index.bundle.base.ts b/packages/tracing/src/index.bundle.base.ts index 42e2c67fa4a8..bdc0a69337e8 100644 --- a/packages/tracing/src/index.bundle.base.ts +++ b/packages/tracing/src/index.bundle.base.ts @@ -67,11 +67,8 @@ if (GLOBAL_OBJ.Sentry && GLOBAL_OBJ.Sentry.Integrations) { windowIntegrations = GLOBAL_OBJ.Sentry.Integrations; } -// For whatever reason, it does not recognize BrowserTracing or some of the BrowserIntegrations as Integration -const INTEGRATIONS: Record< - string, - Integration | typeof BrowserTracing | typeof BrowserIntegrations[keyof typeof BrowserIntegrations] -> = { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +const INTEGRATIONS: Record Integration> = { ...windowIntegrations, ...BrowserIntegrations, BrowserTracing, diff --git a/packages/tracing/src/index.bundle.replay.ts b/packages/tracing/src/index.bundle.replay.ts index fe503b961461..7f4d5a2f3bb7 100644 --- a/packages/tracing/src/index.bundle.replay.ts +++ b/packages/tracing/src/index.bundle.replay.ts @@ -8,5 +8,4 @@ import * as Sentry from './index.bundle'; Sentry.Integrations.Replay = Replay; export { Replay }; - export * from './index.bundle.base'; diff --git a/packages/tracing/src/index.bundle.ts b/packages/tracing/src/index.bundle.ts index ac897b145298..7f56d0233b4a 100644 --- a/packages/tracing/src/index.bundle.ts +++ b/packages/tracing/src/index.bundle.ts @@ -1,4 +1,12 @@ // This is exported so the loader does not fail when switching off Replay -export { Replay } from '@sentry-internal/integration-shims'; +import { Replay } from '@sentry-internal/integration-shims'; +import * as Sentry from './index.bundle'; + +// TODO (v8): Remove this as it was only needed for backwards compatibility +// We want replay to be available under Sentry.Replay, to be consistent +// with the NPM package version. +Sentry.Integrations.Replay = Replay; + +export { Replay }; export * from './index.bundle.base'; diff --git a/rollup/bundleHelpers.js b/rollup/bundleHelpers.js index eb5515788b08..9e808cb7a4f0 100644 --- a/rollup/bundleHelpers.js +++ b/rollup/bundleHelpers.js @@ -16,27 +16,14 @@ import { makeSucrasePlugin, makeTerserPlugin, makeTSPlugin, - makeExcludeBlockPlugin, makeSetSDKSourcePlugin, - makeIntegrationShimPlugin, } from './plugins/index.js'; import { mergePlugins } from './utils'; const BUNDLE_VARIANTS = ['.js', '.min.js', '.debug.min.js']; export function makeBaseBundleConfig(options) { - const { - bundleType, - entrypoints, - jsVersion, - licenseTitle, - outputFileBase, - packageSpecificConfig, - includeReplay, - includeOffline, - includeBrowserProfiling, - includeBrowserTracing, - } = options; + const { bundleType, entrypoints, jsVersion, licenseTitle, outputFileBase, packageSpecificConfig } = options; const nodeResolvePlugin = makeNodeResolvePlugin(); const sucrasePlugin = makeSucrasePlugin(); @@ -44,12 +31,6 @@ export function makeBaseBundleConfig(options) { const markAsBrowserBuildPlugin = makeBrowserBuildPlugin(true); const licensePlugin = makeLicensePlugin(licenseTitle); const tsPlugin = makeTSPlugin(jsVersion.toLowerCase()); - const excludeReplayPlugin = makeExcludeBlockPlugin('REPLAY'); - const excludeOfflineTransport = makeExcludeBlockPlugin('OFFLINE'); - const excludeBrowserProfiling = makeExcludeBlockPlugin('BROWSER_PROFILING'); - const excludeBrowserTracing = makeExcludeBlockPlugin('BROWSER_TRACING'); - const replayShimPlugin = makeIntegrationShimPlugin('@sentry/replay'); - const browserTracingShimPlugin = makeIntegrationShimPlugin('@sentry-internal/tracing'); // The `commonjs` plugin is the `esModuleInterop` of the bundling world. When used with `transformMixedEsModules`, it // will include all dependencies, imported or required, in the final bundle. (Without it, CJS modules aren't included @@ -66,27 +47,6 @@ export function makeBaseBundleConfig(options) { plugins: [markAsBrowserBuildPlugin], }; - if (includeReplay === 'shim') { - standAloneBundleConfig.plugins.push(replayShimPlugin); - } else if (!includeReplay) { - standAloneBundleConfig.plugins.push(excludeReplayPlugin); - } - - if (!includeOffline) { - standAloneBundleConfig.plugins.push(excludeOfflineTransport); - } - - if (!includeBrowserProfiling) { - standAloneBundleConfig.plugins.push(excludeBrowserProfiling); - } - - if (includeBrowserTracing === 'shim') { - standAloneBundleConfig.plugins.push(browserTracingShimPlugin); - } - if (!includeBrowserTracing) { - standAloneBundleConfig.plugins.push(excludeBrowserTracing); - } - // used by `@sentry/integrations` and `@sentry/wasm` (bundles which need to be combined with a stand-alone SDK bundle) const addOnBundleConfig = { // These output settings are designed to mimic an IIFE. We don't use Rollup's `iife` format because we don't want to diff --git a/rollup/plugins/bundlePlugins.js b/rollup/plugins/bundlePlugins.js index 2eeb6f358031..f06c8b613155 100644 --- a/rollup/plugins/bundlePlugins.js +++ b/rollup/plugins/bundlePlugins.js @@ -8,8 +8,6 @@ * Typescript plugin docs: https://github.com/ezolenko/rollup-plugin-typescript2 */ -import path from 'path'; - import commonjs from '@rollup/plugin-commonjs'; import deepMerge from 'deepmerge'; import license from 'rollup-plugin-license'; @@ -17,8 +15,6 @@ import resolve from '@rollup/plugin-node-resolve'; import replace from '@rollup/plugin-replace'; import { terser } from 'rollup-plugin-terser'; import typescript from 'rollup-plugin-typescript2'; -import MagicString from 'magic-string'; -import modify from 'rollup-plugin-modify'; /** * Create a plugin to add an identification banner to the top of stand-alone bundles. @@ -181,55 +177,6 @@ export function makeTSPlugin(jsVersion) { return plugin; } -/** - * Creates a Rollup plugin that removes all code between the `__ROLLUP_EXCLUDE_FROM_BUNDLES_BEGIN__` - * and `__ROLLUP_EXCLUDE_FROM_BUNDLES_END__` comment guards. - * This is used to exclude certain exports from variants of the browser bundle. - */ -export function makeExcludeBlockPlugin(type) { - const replacementRegex = new RegExp( - `\\/\\/ __ROLLUP_EXCLUDE_${type}_FROM_BUNDLES_BEGIN__(.|\n)*__ROLLUP_EXCLUDE_${type}_FROM_BUNDLES_END__`, - 'm', - ); - - const browserIndexFilePath = path.resolve(__dirname, '../../packages/browser/src/index.ts'); - - const plugin = { - transform(code, id) { - const resolvedPath = path.resolve(id); - const shouldReplaceInFile = resolvedPath === browserIndexFilePath; - - if (!shouldReplaceInFile || !replacementRegex.test(code)) { - return null; - } - - const ms = new MagicString(code); - const transformedCode = ms.replace(replacementRegex, ''); - return { - code: transformedCode.toString(), - map: transformedCode.generateMap({ hires: true }), - }; - }, - }; - - plugin.name = 'excludeBlock'; - - return plugin; -} - -export function makeIntegrationShimPlugin(originalImportName) { - // This is designed to replace the re-export in browser/index.ts to export the shim - const plugin = modify({ - find: originalImportName, - replace: '@sentry-internal/integration-shims', - }); - - // give it a nicer name for later, when we'll need to sort the plugins - plugin.name = 'integrationShimPlugin'; - - return plugin; -} - // We don't pass these plugins any options which need to be calculated or changed by us, so no need to wrap them in // another factory function, as they are themselves already factory functions. export { resolve as makeNodeResolvePlugin }; diff --git a/yarn.lock b/yarn.lock index 9e05ec7d4107..0243a33b9b4b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -17812,13 +17812,6 @@ madge@4.0.2: typescript "^3.9.5" walkdir "^0.4.1" -magic-string@0.25.2: - version "0.25.2" - resolved "https://registry.yarnpkg.com/magic-string/-/magic-string-0.25.2.tgz#139c3a729515ec55e96e69e82a11fe890a293ad9" - integrity sha512-iLs9mPjh9IuTtRsqqhNGYcZXGei0Nh/A4xirrsqW7c+QhKVFL2vm7U09ru6cHRD22azaP/wMDgI+HCqbETMTtg== - dependencies: - sourcemap-codec "^1.4.4" - magic-string@0.25.7: version "0.25.7" resolved "https://registry.yarnpkg.com/magic-string/-/magic-string-0.25.7.tgz#3f497d6fd34c669c6798dcb821f2ef31f5445051" @@ -20019,13 +20012,6 @@ osenv@^0.1.3, osenv@^0.1.5: os-homedir "^1.0.0" os-tmpdir "^1.0.0" -ospec@3.1.0: - version "3.1.0" - resolved "https://registry.yarnpkg.com/ospec/-/ospec-3.1.0.tgz#d36b8e10110f58f63a463df2390a7a73fe9579a8" - integrity sha512-+nGtjV3vlADp+UGfL51miAh/hB4awPBkQrArhcgG4trAaoA2gKt5bf9w0m9ch9zOr555cHWaCHZEDiBOkNZSxw== - dependencies: - glob "^7.1.3" - p-cancelable@^0.4.0: version "0.4.1" resolved "https://registry.yarnpkg.com/p-cancelable/-/p-cancelable-0.4.1.tgz#35f363d67d52081c8d9585e37bcceb7e0bbcb2a0" @@ -23440,14 +23426,6 @@ rollup-plugin-license@^2.6.1: spdx-expression-validate "2.0.0" spdx-satisfies "5.0.1" -rollup-plugin-modify@^3.0.0: - version "3.0.0" - resolved "https://registry.yarnpkg.com/rollup-plugin-modify/-/rollup-plugin-modify-3.0.0.tgz#5326e11dfec247e8bbdd9507f3da1da1e5c7818b" - integrity sha512-p/ffs0Y2jz2dEnWjq1oVC7SY37tuS+aP7whoNaQz1EAAOPg+k3vKJo8cMMWx6xpdd0NzhX4y2YF9o/NPu5YR0Q== - dependencies: - magic-string "0.25.2" - ospec "3.1.0" - rollup-plugin-sourcemaps@^0.6.0, rollup-plugin-sourcemaps@^0.6.3: version "0.6.3" resolved "https://registry.yarnpkg.com/rollup-plugin-sourcemaps/-/rollup-plugin-sourcemaps-0.6.3.tgz#bf93913ffe056e414419607f1d02780d7ece84ed" From b73d3a273895f1fe08365827af34d441c8f2416b Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 20 Mar 2023 13:19:00 +0100 Subject: [PATCH 3/4] add unit tests --- .../test/unit/index.bundle.replay.test.ts | 23 +++++++++++++++++++ .../browser/test/unit/index.bundle.test.ts | 22 ++++++++++++++++++ packages/tracing/src/index.bundle.replay.ts | 2 +- packages/tracing/src/index.bundle.ts | 2 +- .../tracing/test/index.bundle.replay.test.ts | 21 +++++++++-------- packages/tracing/test/index.bundle.test.ts | 21 +++++++++++------ 6 files changed, 73 insertions(+), 18 deletions(-) create mode 100644 packages/browser/test/unit/index.bundle.replay.test.ts create mode 100644 packages/browser/test/unit/index.bundle.test.ts diff --git a/packages/browser/test/unit/index.bundle.replay.test.ts b/packages/browser/test/unit/index.bundle.replay.test.ts new file mode 100644 index 000000000000..760232ec57cd --- /dev/null +++ b/packages/browser/test/unit/index.bundle.replay.test.ts @@ -0,0 +1,23 @@ +import { BrowserTracing as BrowserTracingShim } from '@sentry-internal/integration-shims'; +import { Replay } from '@sentry/browser'; + +import * as TracingReplayBundle from '../../src/index.bundle.replay'; + +describe('index.bundle.replay', () => { + it('has correct exports', () => { + Object.keys(TracingReplayBundle.Integrations).forEach(key => { + // Skip BrowserTracing because it doesn't have a static id field. + if (key === 'BrowserTracing') { + return; + } + + expect((TracingReplayBundle.Integrations[key] as any).id).toStrictEqual(expect.any(String)); + }); + + expect(TracingReplayBundle.Integrations.Replay).toBe(Replay); + expect(TracingReplayBundle.Replay).toBe(Replay); + + expect(TracingReplayBundle.Integrations.BrowserTracing).toBe(BrowserTracingShim); + expect(TracingReplayBundle.BrowserTracing).toBe(BrowserTracingShim); + }); +}); diff --git a/packages/browser/test/unit/index.bundle.test.ts b/packages/browser/test/unit/index.bundle.test.ts new file mode 100644 index 000000000000..7bb866dabaad --- /dev/null +++ b/packages/browser/test/unit/index.bundle.test.ts @@ -0,0 +1,22 @@ +import { BrowserTracing as BrowserTracingShim, Replay as ReplayShim } from '@sentry-internal/integration-shims'; + +import * as TracingBundle from '../../src/index.bundle'; + +describe('index.bundle', () => { + it('has correct exports', () => { + Object.keys(TracingBundle.Integrations).forEach(key => { + // Skip BrowserTracing because it doesn't have a static id field. + if (key === 'BrowserTracing') { + return; + } + + expect((TracingBundle.Integrations[key] as any).id).toStrictEqual(expect.any(String)); + }); + + expect(TracingBundle.Integrations.Replay).toBe(ReplayShim); + expect(TracingBundle.Replay).toBe(ReplayShim); + + expect(TracingBundle.Integrations.BrowserTracing).toBe(BrowserTracingShim); + expect(TracingBundle.BrowserTracing).toBe(BrowserTracingShim); + }); +}); diff --git a/packages/tracing/src/index.bundle.replay.ts b/packages/tracing/src/index.bundle.replay.ts index 7f4d5a2f3bb7..d0901d0fdbf8 100644 --- a/packages/tracing/src/index.bundle.replay.ts +++ b/packages/tracing/src/index.bundle.replay.ts @@ -1,6 +1,6 @@ import { Replay } from '@sentry/browser'; -import * as Sentry from './index.bundle'; +import * as Sentry from './index.bundle.base'; // TODO (v8): Remove this as it was only needed for backwards compatibility // We want replay to be available under Sentry.Replay, to be consistent diff --git a/packages/tracing/src/index.bundle.ts b/packages/tracing/src/index.bundle.ts index 7f56d0233b4a..8169e348b0a7 100644 --- a/packages/tracing/src/index.bundle.ts +++ b/packages/tracing/src/index.bundle.ts @@ -1,7 +1,7 @@ // This is exported so the loader does not fail when switching off Replay import { Replay } from '@sentry-internal/integration-shims'; -import * as Sentry from './index.bundle'; +import * as Sentry from './index.bundle.base'; // TODO (v8): Remove this as it was only needed for backwards compatibility // We want replay to be available under Sentry.Replay, to be consistent diff --git a/packages/tracing/test/index.bundle.replay.test.ts b/packages/tracing/test/index.bundle.replay.test.ts index 6de5a19d1f61..b860e6336679 100644 --- a/packages/tracing/test/index.bundle.replay.test.ts +++ b/packages/tracing/test/index.bundle.replay.test.ts @@ -1,20 +1,23 @@ -import * as Sentry from '../src/index.bundle.replay'; +import { BrowserTracing } from '@sentry-internal/tracing'; +import { Replay } from '@sentry/browser'; -// Because of the way how we re-export stuff for the replay bundle, we only have a single default export -const { Integrations } = Sentry; +import * as TracingReplayBundle from '../src/index.bundle.replay'; -describe('Integrations export', () => { - it('is exported correctly', () => { - Object.keys(Integrations).forEach(key => { +describe('index.bundle.replay', () => { + it('has correct exports', () => { + Object.keys(TracingReplayBundle.Integrations).forEach(key => { // Skip BrowserTracing because it doesn't have a static id field. if (key === 'BrowserTracing') { return; } - expect((Integrations[key] as any).id).toStrictEqual(expect.any(String)); + expect((TracingReplayBundle.Integrations[key] as any).id).toStrictEqual(expect.any(String)); }); - expect(Integrations.Replay).toBeDefined(); - expect(Sentry.Replay).toBeDefined(); + expect(TracingReplayBundle.Integrations.Replay).toBe(Replay); + expect(TracingReplayBundle.Replay).toBe(Replay); + + expect(TracingReplayBundle.Integrations.BrowserTracing).toBe(BrowserTracing); + expect(TracingReplayBundle.BrowserTracing).toBe(BrowserTracing); }); }); diff --git a/packages/tracing/test/index.bundle.test.ts b/packages/tracing/test/index.bundle.test.ts index 494c94a28bfb..db1cb7b9f433 100644 --- a/packages/tracing/test/index.bundle.test.ts +++ b/packages/tracing/test/index.bundle.test.ts @@ -1,16 +1,23 @@ -import { Integrations } from '../src/index.bundle'; +import { Replay as ReplayShim } from '@sentry-internal/integration-shims'; +import { BrowserTracing } from '@sentry-internal/tracing'; -describe('Integrations export', () => { - it('is exported correctly', () => { - Object.keys(Integrations).forEach(key => { +import * as TracingBundle from '../src/index.bundle'; + +describe('index.bundle', () => { + it('has correct exports', () => { + Object.keys(TracingBundle.Integrations).forEach(key => { // Skip BrowserTracing because it doesn't have a static id field. if (key === 'BrowserTracing') { return; } - expect((Integrations[key] as any).id).toStrictEqual(expect.any(String)); + expect((TracingBundle.Integrations[key] as any).id).toStrictEqual(expect.any(String)); }); - }); - expect(Integrations.Replay).toBeUndefined(); + expect(TracingBundle.Integrations.Replay).toBe(ReplayShim); + expect(TracingBundle.Replay).toBe(ReplayShim); + + expect(TracingBundle.Integrations.BrowserTracing).toBe(BrowserTracing); + expect(TracingBundle.BrowserTracing).toBe(BrowserTracing); + }); }); From 010f60cfddfa91bc9b4a4b4d5e1b4ad417d3875f Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 20 Mar 2023 13:46:13 +0100 Subject: [PATCH 4/4] skip tests correctly --- .../suites/tracing/browserTracingIntegrationShim/test.ts | 3 ++- .../suites/tracing/browserTracingShim/test.ts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/browser-integration-tests/suites/tracing/browserTracingIntegrationShim/test.ts b/packages/browser-integration-tests/suites/tracing/browserTracingIntegrationShim/test.ts index 730170b14344..5d2b96232d16 100644 --- a/packages/browser-integration-tests/suites/tracing/browserTracingIntegrationShim/test.ts +++ b/packages/browser-integration-tests/suites/tracing/browserTracingIntegrationShim/test.ts @@ -6,8 +6,9 @@ sentryTest( 'exports a shim Integrations.BrowserTracing integration for non-tracing bundles', async ({ getLocalTestPath, page }) => { const bundle = process.env.PW_BUNDLE; + const tracingOnly = Boolean(process.env.PW_TRACING_ONLY); - if (!bundle || !bundle.startsWith('bundle_') || bundle.includes('tracing')) { + if (!bundle || !bundle.startsWith('bundle_') || tracingOnly) { sentryTest.skip(); } diff --git a/packages/browser-integration-tests/suites/tracing/browserTracingShim/test.ts b/packages/browser-integration-tests/suites/tracing/browserTracingShim/test.ts index f920e8ec3cf6..74cb8b2fce9f 100644 --- a/packages/browser-integration-tests/suites/tracing/browserTracingShim/test.ts +++ b/packages/browser-integration-tests/suites/tracing/browserTracingShim/test.ts @@ -4,8 +4,9 @@ import { sentryTest } from '../../../utils/fixtures'; sentryTest('exports a shim BrowserTracing integration for non-tracing bundles', async ({ getLocalTestPath, page }) => { const bundle = process.env.PW_BUNDLE; + const tracingOnly = Boolean(process.env.PW_TRACING_ONLY); - if (!bundle || !bundle.startsWith('bundle_') || bundle.includes('tracing')) { + if (!bundle || !bundle.startsWith('bundle_') || tracingOnly) { sentryTest.skip(); }