fix(feedback): Ensure feedback can be lazy loaded in CDN bundles (#13241

)

This was brought up in slack - if you use a CDN bundle (or the loader)
without feedback included, and you try to lazy-load the
feedbackIntegration, it fails as of today. The reason is that we check
if `window.Sentry.feedbackIntegration` exists, which it _does_, because
we register a shim integration for compatibility in the loader.

So this PR adds a property on the shim integration which we can check
for during lazy loading. While at it, I also added a missing method to
the feedback integration shim.

dev-packages/browser-integration-tests/suites/integrations/lazyLoad/feedbackIntegration/init.js

@@ -0,0 +1,10 @@
+import * as Sentry from '@sentry/browser';
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  integrations: [],
+});
+
+window.Sentry = {
+  ...Sentry,
+};

dev-packages/browser-integration-tests/suites/integrations/lazyLoad/feedbackIntegration/subject.js

@@ -0,0 +1,7 @@
+window._testLazyLoadIntegration = async function run() {
+  const integration = await window.Sentry.lazyLoadIntegration('feedbackIntegration');
+
+  window.Sentry.getClient()?.addIntegration(integration());
+
+  window._integrationLoaded = true;
+};

dev-packages/browser-integration-tests/suites/integrations/lazyLoad/feedbackIntegration/test.ts

@@ -0,0 +1,38 @@
+import { expect } from '@playwright/test';
+import { SDK_VERSION } from '@sentry/browser';
+
+import { sentryTest } from '../../../../utils/fixtures';
+
+sentryTest('it allows to lazy load the feedback integration', async ({ getLocalTestUrl, page }) => {
+  const bundle = process.env.PW_BUNDLE || '';
+  const url = await getLocalTestUrl({ testDir: __dirname });
+
+  await page.route(`https://browser.sentry-cdn.com/${SDK_VERSION}/feedback.min.js`, route => {
+    return route.fulfill({
+      status: 200,
+      contentType: 'application/javascript;',
+      body: "window.Sentry.feedbackIntegration = () => ({ name: 'Feedback', attachTo: () => {} })",
+    });
+  });
+
+  await page.goto(url);
+
+  await page.waitForFunction('window.Sentry?.getClient()');
+
+  const integrationOutput1 = await page.evaluate('window.Sentry.feedbackIntegration?._isShim');
+
+  // Multiple cases are possible here:
+  // 1. Bundle without feedback, should have _isShim property
+  if (bundle.startsWith('bundle') && !bundle.includes('feedback')) {
+    expect(integrationOutput1).toBe(true);
+  } else {
+    // 2. Either bundle with feedback, or ESM, should not have _isShim property
+    expect(integrationOutput1).toBe(undefined);
+  }
+
+  await page.evaluate('window._testLazyLoadIntegration()');
+  await page.waitForFunction('window._integrationLoaded');
+
+  const integrationOutput2 = await page.evaluate('window.Sentry.feedbackIntegration?._isShim');
+  expect(integrationOutput2).toBe(undefined);
+});

dev-packages/rollup-utils/plugins/bundlePlugins.mjs

@@ -133,6 +133,8 @@ export function makeTerserPlugin() {
           '_sentryIsolationScope',
           // require-in-the-middle calls `Module._resolveFilename`. We cannot mangle this (AWS lambda layer bundle).
           '_resolveFilename',
+          // Set on e.g. the shim feedbackIntegration to be able to detect it
+          '_isShim',
         ],
       },
     },

packages/browser/src/utils/lazyLoadIntegration.ts

@@ -43,7 +43,10 @@ export async function lazyLoadIntegration(name: keyof typeof LazyLoadableIntegra
 
   // Bail if the integration already exists
   const existing = sentryOnWindow[name];
-  if (typeof existing === 'function') {
+  // The `feedbackIntegration` is loaded by default in the CDN bundles,
+  // so we need to differentiate between the real integration and the shim.
+  // if only the shim exists, we still want to lazy load the real integration.
+  if (typeof existing === 'function' && !('_isShim' in existing)) {
     return existing;
   }
 

packages/integration-shims/src/Feedback.ts

@@ -2,7 +2,7 @@ import type { Integration } from '@sentry/types';
 import { consoleSandbox } from '@sentry/utils';
 import { FAKE_FUNCTION } from './common';
 
-const FEEDBACK_INTEGRATION_METHODS = ['attachTo', 'createWidget', 'remove'] as const;
+const FEEDBACK_INTEGRATION_METHODS = ['attachTo', 'createForm', 'createWidget', 'remove'] as const;
 
 type FeedbackSpecificMethods = Record<(typeof FEEDBACK_INTEGRATION_METHODS)[number], () => void>;
 
@@ -13,17 +13,22 @@ interface FeedbackIntegration extends Integration, FeedbackSpecificMethods {}
  * It is needed in order for the CDN bundles to continue working when users add/remove feedback
  * from it, without changing their config. This is necessary for the loader mechanism.
  */
-export function feedbackIntegrationShim(_options: unknown): FeedbackIntegration {
-  consoleSandbox(() => {
-    // eslint-disable-next-line no-console
-    console.warn('You are using feedbackIntegration() even though this bundle does not include feedback.');
-  });
+export const feedbackIntegrationShim = Object.assign(
+  (_options: unknown): FeedbackIntegration => {
+    consoleSandbox(() => {
+      // eslint-disable-next-line no-console
+      console.warn('You are using feedbackIntegration() even though this bundle does not include feedback.');
+    });
 
-  return {
-    name: 'Feedback',
-    ...(FEEDBACK_INTEGRATION_METHODS.reduce((acc, method) => {
-      acc[method] = FAKE_FUNCTION;
-      return acc;
-    }, {} as FeedbackSpecificMethods) as FeedbackSpecificMethods),
-  };
-}
+    return {
+      name: 'Feedback',
+      ...(FEEDBACK_INTEGRATION_METHODS.reduce((acc, method) => {
+        acc[method] = FAKE_FUNCTION;
+        return acc;
+      }, {} as FeedbackSpecificMethods) as FeedbackSpecificMethods),
+    };
+  },
+  {
+    _isShim: true,
+  },
+);