Skip to content

Commit

Permalink
fix(feedback): Ensure pluggable feedback CDN bundle is correctly built (
Browse files Browse the repository at this point in the history
#13081)

Fixes #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.
  • Loading branch information
mydea authored Jul 30, 2024
1 parent e9897ab commit 8f037ac
Show file tree
Hide file tree
Showing 8 changed files with 28 additions and 27 deletions.
Original file line number Diff line number Diff line change
@@ -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' },
}),
],
Expand Down
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -12,6 +14,6 @@ Sentry.init({
flushMaxDelay: 200,
minReplayDuration: 0,
}),
Sentry.feedbackIntegration(),
feedbackIntegration(),
],
});
Original file line number Diff line number Diff line change
@@ -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,
});
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string> = {
httpClientIntegration: 'httpclient',
captureConsoleIntegration: 'captureconsole',
Expand All @@ -34,6 +37,7 @@ const IMPORTED_INTEGRATION_CDN_BUNDLE_PATHS: Record<string, string> = {
extraErrorDataIntegration: 'extraerrordata',
reportingObserverIntegration: 'reportingobserver',
sessionTimingIntegration: 'sessiontiming',
feedbackIntegration: 'feedback',
};

const BUNDLE_PATHS: Record<string, Record<string, string>> = {
Expand Down
10 changes: 3 additions & 7 deletions dev-packages/browser-integration-tests/utils/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
5 changes: 3 additions & 2 deletions packages/browser/rollup.bundle.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand All @@ -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`],
Expand Down
7 changes: 7 additions & 0 deletions packages/browser/src/integrations-bundle/index.feedback.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { feedbackAsyncIntegration } from '../feedbackAsync';

export { getFeedback } from '@sentry-internal/feedback';

export { feedbackAsyncIntegration, feedbackAsyncIntegration as feedbackIntegration };

export { captureFeedback } from '@sentry/core';
17 changes: 2 additions & 15 deletions packages/feedback/rollup.bundle.config.mjs
Original file line number Diff line number Diff line change
@@ -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',
Expand Down

0 comments on commit 8f037ac

Please sign in to comment.