From 8f037ac50b05237131cd67e897e4388e3bd01a38 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 30 Jul 2024 09:44:30 +0200 Subject: [PATCH] fix(feedback): Ensure pluggable feedback CDN bundle is correctly built (#13081) Fixes https://github.com/getsentry/sentry-javascript/issues/13080 This was basically just wrong - we need stuff from `browser` package there, so we can't really build the final CDN bundle from the feedback-internal package. I moved this over and also adjusted tests to actually test this with pluggable CDN integrations as well. --- .../suites/feedback/captureFeedback/init.js | 4 +++- .../hasSampling/init.js | 4 +++- .../suites/tracing/trace-lifetime/init.js | 4 +++- .../utils/generatePlugin.ts | 4 ++++ .../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 ++--------------- 8 files changed, 28 insertions(+), 27 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/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 94f3dd20ae81..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', @@ -34,6 +37,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..6a3c6342842b 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 reexportedPluggableIntegrationFiles = [ 'captureconsole', 'debug', 'dedupe', 'extraerrordata', 'rewriteframes', 'sessiontiming', + 'feedback', ]; browserPluggableIntegrationFiles.forEach(integrationName => { @@ -24,7 +25,7 @@ browserPluggableIntegrationFiles.forEach(integrationName => { builds.push(...makeBundleConfigVariants(integrationsBundleConfig)); }); -corePluggableIntegrationFiles.forEach(integrationName => { +reexportedPluggableIntegrationFiles.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..f5c10b970690 --- /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, 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',