Skip to content

Commit ca5cb14

Browse files
authored
fix(browser): Ensure lazyLoadIntegration works in NPM mode (#11673)
As noticed by @ryan953 here: #11621 (comment), this was not actually working properly in NPM-mode. I added a proper test for this and fixed this, so hopefully should be all good now.
1 parent aceae05 commit ca5cb14

File tree

5 files changed

+63
-9
lines changed

5 files changed

+63
-9
lines changed
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
// So we can use this in subject.js
4+
// We specifically DO NOT set this on window.Sentry as we want to test a non-CDN bundle environment,
5+
// where window.Sentry is usually not available
6+
window._testSentry = { ...Sentry };
7+
8+
Sentry.init({
9+
dsn: 'https://[email protected]/1337',
10+
integrations: [],
11+
});
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
window._testLazyLoadIntegration = async function run() {
2+
const integration = await window._testSentry.lazyLoadIntegration('httpClientIntegration');
3+
4+
window._testSentry.getClient()?.addIntegration(integration());
5+
6+
window._integrationLoaded = true;
7+
};
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import { expect } from '@playwright/test';
2+
import { SDK_VERSION } from '@sentry/browser';
3+
4+
import { sentryTest } from '../../../../utils/fixtures';
5+
6+
sentryTest('it allows to lazy load an integration when using the NPM package', async ({ getLocalTestUrl, page }) => {
7+
const bundle = process.env.PW_BUNDLE || '';
8+
// We only want to run this in non-CDN bundle mode
9+
if (bundle.startsWith('bundle')) {
10+
sentryTest.skip();
11+
}
12+
13+
const url = await getLocalTestUrl({ testDir: __dirname });
14+
15+
await page.route(`https://browser.sentry-cdn.com/${SDK_VERSION}/httpclient.min.js`, route => {
16+
return route.fulfill({
17+
status: 200,
18+
contentType: 'application/javascript;',
19+
body: "window.Sentry = window.Sentry || {};window.Sentry.httpClientIntegration = () => ({ name: 'HttpClient' })",
20+
});
21+
});
22+
23+
await page.goto(url);
24+
25+
const hasIntegration = await page.evaluate('!!window._testSentry.getClient().getIntegrationByName("HttpClient")');
26+
expect(hasIntegration).toBe(false);
27+
28+
const scriptTagsBefore = await page.evaluate<number>('document.querySelectorAll("script").length');
29+
30+
await page.evaluate('window._testLazyLoadIntegration()');
31+
await page.waitForFunction('window._integrationLoaded');
32+
33+
const scriptTagsAfter = await page.evaluate<number>('document.querySelectorAll("script").length');
34+
35+
const hasIntegration2 = await page.evaluate('!!window._testSentry.getClient().getIntegrationByName("HttpClient")');
36+
expect(hasIntegration2).toBe(true);
37+
38+
expect(scriptTagsAfter).toBe(scriptTagsBefore + 1);
39+
});

packages/browser/src/utils/lazyLoadIntegration.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,15 @@ const WindowWithMaybeIntegration = WINDOW as {
3333
export async function lazyLoadIntegration(name: keyof typeof LazyLoadableIntegrations): Promise<IntegrationFn> {
3434
const bundle = LazyLoadableIntegrations[name];
3535

36-
if (!bundle || !WindowWithMaybeIntegration.Sentry) {
36+
// `window.Sentry` is only set when using a CDN bundle, but this method can also be used via the NPM package
37+
const sentryOnWindow = (WindowWithMaybeIntegration.Sentry = WindowWithMaybeIntegration.Sentry || {});
38+
39+
if (!bundle) {
3740
throw new Error(`Cannot lazy load integration: ${name}`);
3841
}
3942

4043
// Bail if the integration already exists
41-
const existing = WindowWithMaybeIntegration.Sentry[name];
44+
const existing = sentryOnWindow[name];
4245
if (typeof existing === 'function') {
4346
return existing;
4447
}
@@ -61,7 +64,7 @@ export async function lazyLoadIntegration(name: keyof typeof LazyLoadableIntegra
6164
throw new Error(`Error when loading integration: ${name}`);
6265
}
6366

64-
const integrationFn = WindowWithMaybeIntegration.Sentry[name];
67+
const integrationFn = sentryOnWindow[name];
6568

6669
if (typeof integrationFn !== 'function') {
6770
throw new Error(`Could not load integration: ${name}`);

packages/browser/test/unit/utils/lazyLoadIntegration.test.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,6 @@ describe('lazyLoadIntegration', () => {
4848
await expect(() => lazyLoadIntegration('invalid!!!')).rejects.toThrow('Cannot lazy load integration: invalid!!!');
4949
});
5050

51-
test('it rejects without global Sentry variable', async () => {
52-
await expect(() => lazyLoadIntegration('httpClientIntegration')).rejects.toThrow(
53-
'Cannot lazy load integration: httpClientIntegration',
54-
);
55-
});
56-
5751
test('it does not inject a script tag if integration already exists', async () => {
5852
// @ts-expect-error For testing sake
5953
global.Sentry = Sentry;

0 commit comments

Comments
 (0)