From 5b1eef2f58f3ff1fc288ae49ce8f01717594dc61 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 5 Aug 2024 17:16:19 +0200 Subject: [PATCH 1/2] fix(feedback): Ensure feedback can be lazy loaded in CDN bundles --- .../lazyLoad/feedbackIntegration/init.js | 10 +++++ .../lazyLoad/feedbackIntegration/subject.js | 7 ++++ .../lazyLoad/feedbackIntegration/test.ts | 38 +++++++++++++++++++ .../browser/src/utils/lazyLoadIntegration.ts | 5 ++- packages/integration-shims/src/Feedback.ts | 33 +++++++++------- 5 files changed, 78 insertions(+), 15 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/integrations/lazyLoad/feedbackIntegration/init.js create mode 100644 dev-packages/browser-integration-tests/suites/integrations/lazyLoad/feedbackIntegration/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/integrations/lazyLoad/feedbackIntegration/test.ts diff --git a/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/feedbackIntegration/init.js b/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/feedbackIntegration/init.js new file mode 100644 index 000000000000..ff6211c23b37 --- /dev/null +++ b/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 + }; diff --git a/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/feedbackIntegration/subject.js b/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/feedbackIntegration/subject.js new file mode 100644 index 000000000000..4ef2f8f41622 --- /dev/null +++ b/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; +}; diff --git a/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/feedbackIntegration/test.ts b/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/feedbackIntegration/test.ts new file mode 100644 index 000000000000..d054c2da2e99 --- /dev/null +++ b/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); +}); diff --git a/packages/browser/src/utils/lazyLoadIntegration.ts b/packages/browser/src/utils/lazyLoadIntegration.ts index 4479c2e69590..b023bc18aa95 100644 --- a/packages/browser/src/utils/lazyLoadIntegration.ts +++ b/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; } diff --git a/packages/integration-shims/src/Feedback.ts b/packages/integration-shims/src/Feedback.ts index 189dc074cd1e..ef4010bb0494 100644 --- a/packages/integration-shims/src/Feedback.ts +++ b/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, + }, +); From d1100c421cd98cd04fd80f2cfb4537382ff35805 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 6 Aug 2024 08:25:11 +0200 Subject: [PATCH 2/2] fixes --- .../suites/integrations/lazyLoad/feedbackIntegration/init.js | 4 ++-- dev-packages/rollup-utils/plugins/bundlePlugins.mjs | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/feedbackIntegration/init.js b/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/feedbackIntegration/init.js index ff6211c23b37..d0ff6bbb92c9 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/feedbackIntegration/init.js +++ b/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/feedbackIntegration/init.js @@ -6,5 +6,5 @@ Sentry.init({ }); window.Sentry = { - ...Sentry - }; + ...Sentry, +}; diff --git a/dev-packages/rollup-utils/plugins/bundlePlugins.mjs b/dev-packages/rollup-utils/plugins/bundlePlugins.mjs index 760fdc05daa6..ffc24f58174a 100644 --- a/dev-packages/rollup-utils/plugins/bundlePlugins.mjs +++ b/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', ], }, },