Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Aug 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import * as Sentry from '@sentry/browser';

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
integrations: [],
});

window.Sentry = {
...Sentry,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
window._testLazyLoadIntegration = async function run() {
const integration = await window.Sentry.lazyLoadIntegration('feedbackIntegration');

window.Sentry.getClient()?.addIntegration(integration());

window._integrationLoaded = true;
};
Original file line number Diff line number Diff line change
@@ -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);
});
2 changes: 2 additions & 0 deletions dev-packages/rollup-utils/plugins/bundlePlugins.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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',
],
},
},
Expand Down
5 changes: 4 additions & 1 deletion packages/browser/src/utils/lazyLoadIntegration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
33 changes: 19 additions & 14 deletions packages/integration-shims/src/Feedback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;

Expand All @@ -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,
},
);
Loading