From c8451e66293cfdde0d3f5d7f4bfa4abb2e517d03 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 29 Jul 2024 14:11:51 +0200 Subject: [PATCH 1/4] fix(feedback): Ensure pluggable feedback CDN bundle is correctly built --- .../suites/feedback/captureFeedback/init.js | 4 +++- .../hasSampling/init.js | 4 +++- .../utils/generatePlugin.ts | 1 + .../browser-integration-tests/utils/helpers.ts | 10 +++------- packages/browser/rollup.bundle.config.mjs | 5 +++-- .../src/integrations-bundle/index.feedback.ts | 7 +++++++ packages/feedback/rollup.bundle.config.mjs | 17 ++--------------- 7 files changed, 22 insertions(+), 26 deletions(-) create mode 100644 packages/browser/src/integrations-bundle/index.feedback.ts diff --git a/dev-packages/browser-integration-tests/suites/feedback/captureFeedback/init.js b/dev-packages/browser-integration-tests/suites/feedback/captureFeedback/init.js index 7d36dc233847..e6da8b5973bb 100644 --- a/dev-packages/browser-integration-tests/suites/feedback/captureFeedback/init.js +++ b/dev-packages/browser-integration-tests/suites/feedback/captureFeedback/init.js @@ -1,11 +1,13 @@ import * as Sentry from '@sentry/browser'; +// Import this separately so that generatePlugin can handle it for CDN scenarios +import { feedbackIntegration } from '@sentry/browser'; window.Sentry = Sentry; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', integrations: [ - Sentry.feedbackIntegration({ + feedbackIntegration({ tags: { from: 'integration init' }, }), ], diff --git a/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackAndReplay/hasSampling/init.js b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackAndReplay/hasSampling/init.js index 020e045c780d..c8d5dbcdb232 100644 --- a/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackAndReplay/hasSampling/init.js +++ b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackAndReplay/hasSampling/init.js @@ -1,4 +1,6 @@ import * as Sentry from '@sentry/browser'; +// Import this separately so that generatePlugin can handle it for CDN scenarios +import { feedbackIntegration } from '@sentry/browser'; window.Sentry = Sentry; @@ -12,6 +14,6 @@ Sentry.init({ flushMaxDelay: 200, minReplayDuration: 0, }), - Sentry.feedbackIntegration(), + feedbackIntegration(), ], }); diff --git a/dev-packages/browser-integration-tests/utils/generatePlugin.ts b/dev-packages/browser-integration-tests/utils/generatePlugin.ts index 94f3dd20ae81..328738e08b51 100644 --- a/dev-packages/browser-integration-tests/utils/generatePlugin.ts +++ b/dev-packages/browser-integration-tests/utils/generatePlugin.ts @@ -34,6 +34,7 @@ const IMPORTED_INTEGRATION_CDN_BUNDLE_PATHS: Record = { extraErrorDataIntegration: 'extraerrordata', reportingObserverIntegration: 'reportingobserver', sessionTimingIntegration: 'sessiontiming', + feedbackIntegration: 'feedback', }; const BUNDLE_PATHS: Record> = { diff --git a/dev-packages/browser-integration-tests/utils/helpers.ts b/dev-packages/browser-integration-tests/utils/helpers.ts index e0f72bdfacbc..898cef6a67dd 100644 --- a/dev-packages/browser-integration-tests/utils/helpers.ts +++ b/dev-packages/browser-integration-tests/utils/helpers.ts @@ -246,15 +246,11 @@ export function shouldSkipTracingTest(): boolean { } /** - * We can only test feedback tests in certain bundles/packages: - * - NPM (ESM, CJS) - * - CDN bundles that contain the Replay integration - * - * @returns `true` if we should skip the feedback test + * Today we always run feedback tests, but this can be used to guard this if we ever need to. */ export function shouldSkipFeedbackTest(): boolean { - const bundle = process.env.PW_BUNDLE as string | undefined; - return bundle != null && !bundle.includes('feedback') && !bundle.includes('esm') && !bundle.includes('cjs'); + // We always run these, in bundles the pluggable integration is automatically added + return false; } /** diff --git a/packages/browser/rollup.bundle.config.mjs b/packages/browser/rollup.bundle.config.mjs index 278b9cdc1b87..07b2473fe9b1 100644 --- a/packages/browser/rollup.bundle.config.mjs +++ b/packages/browser/rollup.bundle.config.mjs @@ -4,13 +4,14 @@ const builds = []; const browserPluggableIntegrationFiles = ['contextlines', 'httpclient', 'reportingobserver', 'browserprofiling']; -const corePluggableIntegrationFiles = [ +const rexportedPluggableIntegrationFiles = [ 'captureconsole', 'debug', 'dedupe', 'extraerrordata', 'rewriteframes', 'sessiontiming', + 'feedback', ]; browserPluggableIntegrationFiles.forEach(integrationName => { @@ -24,7 +25,7 @@ browserPluggableIntegrationFiles.forEach(integrationName => { builds.push(...makeBundleConfigVariants(integrationsBundleConfig)); }); -corePluggableIntegrationFiles.forEach(integrationName => { +rexportedPluggableIntegrationFiles.forEach(integrationName => { const integrationsBundleConfig = makeBaseBundleConfig({ bundleType: 'addon', entrypoints: [`src/integrations-bundle/index.${integrationName}.ts`], diff --git a/packages/browser/src/integrations-bundle/index.feedback.ts b/packages/browser/src/integrations-bundle/index.feedback.ts new file mode 100644 index 000000000000..d866d6b7d46f --- /dev/null +++ b/packages/browser/src/integrations-bundle/index.feedback.ts @@ -0,0 +1,7 @@ +import { feedbackAsyncIntegration } from '../feedbackAsync'; + +export { getFeedback } from '@sentry-internal/feedback'; + +export { feedbackAsyncIntegration as feedbackAsyncIntegration, feedbackAsyncIntegration as feedbackIntegration }; + +export { captureFeedback } from '@sentry/core'; diff --git a/packages/feedback/rollup.bundle.config.mjs b/packages/feedback/rollup.bundle.config.mjs index f22f4fc9c647..18343602562c 100644 --- a/packages/feedback/rollup.bundle.config.mjs +++ b/packages/feedback/rollup.bundle.config.mjs @@ -1,21 +1,8 @@ import { makeBaseBundleConfig, makeBundleConfigVariants } from '@sentry-internal/rollup-utils'; export default [ - ...makeBundleConfigVariants( - makeBaseBundleConfig({ - bundleType: 'addon', - entrypoints: ['src/index.ts'], - jsVersion: 'es6', - licenseTitle: '@sentry-internal/feedback', - outputFileBase: () => 'bundles/feedback', - sucrase: { - // The feedback widget is using preact so we need different pragmas and jsx runtimes - jsxPragma: 'h', - jsxFragmentPragma: 'Fragment', - jsxRuntime: 'classic', - }, - }), - ), + // The core `feedback` bundle is built in the browser package + // Sub-bundles are built here ...makeBundleConfigVariants( makeBaseBundleConfig({ bundleType: 'addon', From 5b5b0dab79c1a184fdae23599fc211ee47bc762f Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 29 Jul 2024 14:13:23 +0200 Subject: [PATCH 2/4] fix typo --- packages/browser/rollup.bundle.config.mjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/browser/rollup.bundle.config.mjs b/packages/browser/rollup.bundle.config.mjs index 07b2473fe9b1..6a3c6342842b 100644 --- a/packages/browser/rollup.bundle.config.mjs +++ b/packages/browser/rollup.bundle.config.mjs @@ -4,7 +4,7 @@ const builds = []; const browserPluggableIntegrationFiles = ['contextlines', 'httpclient', 'reportingobserver', 'browserprofiling']; -const rexportedPluggableIntegrationFiles = [ +const reexportedPluggableIntegrationFiles = [ 'captureconsole', 'debug', 'dedupe', @@ -25,7 +25,7 @@ browserPluggableIntegrationFiles.forEach(integrationName => { builds.push(...makeBundleConfigVariants(integrationsBundleConfig)); }); -rexportedPluggableIntegrationFiles.forEach(integrationName => { +reexportedPluggableIntegrationFiles.forEach(integrationName => { const integrationsBundleConfig = makeBaseBundleConfig({ bundleType: 'addon', entrypoints: [`src/integrations-bundle/index.${integrationName}.ts`], From a903ba0f4f76d7c1fda866788f635992232a446d Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 29 Jul 2024 14:14:13 +0200 Subject: [PATCH 3/4] fix unnecessary --- packages/browser/src/integrations-bundle/index.feedback.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/browser/src/integrations-bundle/index.feedback.ts b/packages/browser/src/integrations-bundle/index.feedback.ts index d866d6b7d46f..f5c10b970690 100644 --- a/packages/browser/src/integrations-bundle/index.feedback.ts +++ b/packages/browser/src/integrations-bundle/index.feedback.ts @@ -2,6 +2,6 @@ import { feedbackAsyncIntegration } from '../feedbackAsync'; export { getFeedback } from '@sentry-internal/feedback'; -export { feedbackAsyncIntegration as feedbackAsyncIntegration, feedbackAsyncIntegration as feedbackIntegration }; +export { feedbackAsyncIntegration, feedbackAsyncIntegration as feedbackIntegration }; export { captureFeedback } from '@sentry/core'; From 5ce5a9d0784ef708a914834d6da86b8d0c41f84d Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 29 Jul 2024 14:32:21 +0200 Subject: [PATCH 4/4] fix test --- .../suites/tracing/trace-lifetime/init.js | 4 +++- .../browser-integration-tests/utils/generatePlugin.ts | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/init.js b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/init.js index 9c34f4d99f69..3104d71ca659 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/init.js +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/init.js @@ -1,10 +1,12 @@ import * as Sentry from '@sentry/browser'; +// Import this separately so that generatePlugin can handle it for CDN scenarios +import { feedbackIntegration } from '@sentry/browser'; window.Sentry = Sentry; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', - integrations: [Sentry.browserTracingIntegration(), Sentry.feedbackIntegration()], + integrations: [Sentry.browserTracingIntegration(), feedbackIntegration()], tracePropagationTargets: ['http://example.com'], tracesSampleRate: 1, }); diff --git a/dev-packages/browser-integration-tests/utils/generatePlugin.ts b/dev-packages/browser-integration-tests/utils/generatePlugin.ts index 328738e08b51..69e8f946fc89 100644 --- a/dev-packages/browser-integration-tests/utils/generatePlugin.ts +++ b/dev-packages/browser-integration-tests/utils/generatePlugin.ts @@ -24,6 +24,9 @@ const useLoader = bundleKey.startsWith('loader'); // These are imports that, when using CDN bundles, are not included in the main CDN bundle. // In this case, if we encounter this import, we want to add this CDN bundle file instead +// IMPORTANT NOTE: In order for this to work, you need to import this from browser like this: +// import { httpClientIntegration } from '@sentry/browser'; +// You cannot use e.g. Sentry.httpClientIntegration, as this will not be detected const IMPORTED_INTEGRATION_CDN_BUNDLE_PATHS: Record = { httpClientIntegration: 'httpclient', captureConsoleIntegration: 'captureconsole',