Skip to content

Commit 8cd4c97

Browse files
authored
feat(core): Add includeFeedback Metro option to exclude feedback widget from bundle (#6025)
* feat(core): Add includeFeedback Metro option to exclude feedback widget from bundle Mirrors the existing `includeWebReplay` option. When set to `false`, `@sentry-internal/feedback` (and related subpackages) are resolved to an empty module, removing the web user-feedback widget from the bundle. Refactored the shared resolver logic into `buildSentryPackageExcludeResolver` so `withSentryResolver` and the new `withSentryFeedbackResolver` both call into it instead of duplicating the resolver body. Closes #5629 * Fix changelog * refactor(core): Rename include to includePackage in buildSentryPackageExcludeResolver Addresses review feedback: bare `include` is ambiguous in the helper — `includePackage` makes the intent obvious alongside `moduleRegex`. * refactor(core): Rename includeFeedback to includeWebFeedback Addresses review feedback from @lucas-zimerman: mirror the `includeWebReplay` naming to make it obvious the option only affects the web feedback package, not the native feedback integration. Also moves the changelog entry from 8.9.1 (where it landed during the merge with main) to Unreleased since this change hasn't shipped yet.
1 parent fe656cd commit 8cd4c97

3 files changed

Lines changed: 241 additions & 10 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@
88
99
## Unreleased
1010

11+
### Features
12+
13+
- Add `includeWebFeedback` Metro config option to exclude `@sentry-internal/feedback` from the bundle ([#6025](https://github.com/getsentry/sentry-react-native/pull/6025))
14+
1115
### Fixes
1216

1317
- Stop the Hermes sampling profiler on React instance teardown to prevent `pthread_kill` SIGABRT when the JS thread is torn down with profiling active ([#6035](https://github.com/getsentry/sentry-react-native/pull/6035))

packages/core/src/js/tools/metroconfig.ts

Lines changed: 52 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/* oxlint-disable eslint(max-lines) */
12
import type { MetroConfig, MixedOutput, Module, ReadOnlyGraph } from 'metro';
23
import type { CustomResolutionContext, CustomResolver, Resolution } from 'metro-resolver';
34

@@ -38,6 +39,11 @@ export interface SentryMetroConfigOptions {
3839
* @default true
3940
*/
4041
includeWebReplay?: boolean;
42+
/**
43+
* Adds the Sentry user feedback widget package.
44+
* @default true
45+
*/
46+
includeWebFeedback?: boolean;
4147
/**
4248
* Add Sentry Metro Server Middleware which
4349
* enables the app to fetch stack frames source context.
@@ -79,6 +85,7 @@ export function withSentryConfig(
7985
{
8086
annotateReactComponents = false,
8187
includeWebReplay = true,
88+
includeWebFeedback = true,
8289
enableSourceContextInDevelopment = true,
8390
optionsFile = true,
8491
}: SentryMetroConfigOptions = {},
@@ -95,6 +102,9 @@ export function withSentryConfig(
95102
if (includeWebReplay === false) {
96103
newConfig = withSentryResolver(newConfig, includeWebReplay);
97104
}
105+
if (includeWebFeedback === false) {
106+
newConfig = withSentryFeedbackResolver(newConfig, includeWebFeedback);
107+
}
98108
newConfig = withSentryExcludeServerOnlyResolver(newConfig);
99109
if (enableSourceContextInDevelopment) {
100110
newConfig = withSentryMiddleware(newConfig);
@@ -135,6 +145,9 @@ export function getSentryExpoConfig(
135145
if (options.includeWebReplay === false) {
136146
newConfig = withSentryResolver(newConfig, options.includeWebReplay);
137147
}
148+
if (options.includeWebFeedback === false) {
149+
newConfig = withSentryFeedbackResolver(newConfig, options.includeWebFeedback);
150+
}
138151
newConfig = withSentryExcludeServerOnlyResolver(newConfig);
139152

140153
if (options.enableSourceContextInDevelopment ?? true) {
@@ -230,21 +243,26 @@ type CustomResolverBeforeMetro068 = (
230243
) => Resolution;
231244

232245
/**
233-
* Includes `@sentry/replay` packages based on the `includeWebReplay` flag and current bundle `platform`.
246+
* Builds a Metro resolver that returns `{ type: 'empty' }` for Sentry sub-packages
247+
* matching `moduleRegex` when the user opts out on web or the platform is native.
234248
*/
235-
export function withSentryResolver(config: MetroConfig, includeWebReplay: boolean | undefined): MetroConfig {
249+
function buildSentryPackageExcludeResolver(
250+
config: MetroConfig,
251+
includePackage: boolean | undefined,
252+
moduleRegex: RegExp,
253+
optionName: string,
254+
): MetroConfig {
236255
const originalResolver = config.resolver?.resolveRequest as CustomResolver | CustomResolverBeforeMetro068 | undefined;
237256

238-
const sentryResolverRequest: CustomResolver = (
257+
const resolverRequest: CustomResolver = (
239258
context: CustomResolutionContext,
240259
moduleName: string,
241260
platform: string | null,
242261
oldMetroModuleName?: string,
243262
) => {
244263
if (
245-
(includeWebReplay === false ||
246-
(includeWebReplay === undefined && (platform === 'android' || platform === 'ios'))) &&
247-
!!(oldMetroModuleName ?? moduleName).match(/@sentry(?:-internal)?\/replay/)
264+
(includePackage === false || (includePackage === undefined && (platform === 'android' || platform === 'ios'))) &&
265+
!!(oldMetroModuleName ?? moduleName).match(moduleRegex)
248266
) {
249267
return { type: 'empty' } as Resolution;
250268
}
@@ -254,15 +272,15 @@ export function withSentryResolver(config: MetroConfig, includeWebReplay: boolea
254272
: originalResolver(context, moduleName, platform);
255273
}
256274

257-
// Prior 0.68, resolve context.resolveRequest is sentryResolver itself, where on later version it is the default resolver.
258-
if (context.resolveRequest === sentryResolverRequest) {
275+
// Prior 0.68, context.resolveRequest is resolverRequest itself, where on later version it is the default resolver.
276+
if (context.resolveRequest === resolverRequest) {
259277
// oxlint-disable-next-line eslint(no-console)
260278
console.error(
261279
`Error: [@sentry/react-native/metro] Can not resolve the defaultResolver on Metro older than 0.68.
262280
Please follow one of the following options:
263281
- Include your resolverRequest on your metroconfig.
264282
- Update your Metro version to 0.68 or higher.
265-
- Set includeWebReplay as true on your metro config.
283+
- Set ${optionName} as true on your metro config.
266284
- If you are still facing issues, report the issue at http://www.github.com/getsentry/sentry-react-native/issues`,
267285
);
268286
// Return required for test.
@@ -276,11 +294,35 @@ Please follow one of the following options:
276294
...config,
277295
resolver: {
278296
...config.resolver,
279-
resolveRequest: sentryResolverRequest,
297+
resolveRequest: resolverRequest,
280298
},
281299
};
282300
}
283301

302+
/**
303+
* Includes `@sentry/replay` packages based on the `includeWebReplay` flag and current bundle `platform`.
304+
*/
305+
export function withSentryResolver(config: MetroConfig, includeWebReplay: boolean | undefined): MetroConfig {
306+
return buildSentryPackageExcludeResolver(
307+
config,
308+
includeWebReplay,
309+
/@sentry(?:-internal)?\/replay/,
310+
'includeWebReplay',
311+
);
312+
}
313+
314+
/**
315+
* Includes `@sentry-internal/feedback` packages based on the `includeWebFeedback` flag and current bundle `platform`.
316+
*/
317+
export function withSentryFeedbackResolver(config: MetroConfig, includeWebFeedback: boolean | undefined): MetroConfig {
318+
return buildSentryPackageExcludeResolver(
319+
config,
320+
includeWebFeedback,
321+
/@sentry(?:-internal)?\/feedback/,
322+
'includeWebFeedback',
323+
);
324+
}
325+
284326
/**
285327
* Matches relative import paths to server-only AI/MCP modules within `@sentry/core`.
286328
*

packages/core/test/tools/metroconfig.test.ts

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
getSentryExpoConfig,
1010
withSentryBabelTransformer,
1111
withSentryExcludeServerOnlyResolver,
12+
withSentryFeedbackResolver,
1213
withSentryFramesCollapsed,
1314
withSentryResolver,
1415
} from '../../src/js/tools/metroconfig';
@@ -364,6 +365,190 @@ describe('metroconfig', () => {
364365
}
365366
});
366367
});
368+
describe('withSentryFeedbackResolver', () => {
369+
let originalResolverMock: any;
370+
371+
// @ts-expect-error Can't see type CustomResolutionContext
372+
let contextMock: CustomResolutionContext;
373+
let config: MetroConfig = {};
374+
375+
beforeEach(() => {
376+
originalResolverMock = jest.fn();
377+
contextMock = {
378+
resolveRequest: jest.fn(),
379+
};
380+
381+
config = {
382+
resolver: {
383+
resolveRequest: originalResolverMock,
384+
},
385+
};
386+
});
387+
388+
describe.each([
389+
['new Metro', false, '0.70.0'],
390+
['old Metro', true, '0.67.0'],
391+
])('on %s', (_description, oldMetro, metroVersion) => {
392+
beforeEach(() => {
393+
jest.resetModules();
394+
// Mock metro/package.json
395+
jest.mock('metro/package.json', () => ({
396+
version: metroVersion,
397+
}));
398+
});
399+
400+
describe.each([['@sentry-internal/feedback'], ['@sentry/feedback']])('with %s', feedbackPackage => {
401+
test('keep Feedback when platform is web and includeWebFeedback is true', () => {
402+
const modifiedConfig = withSentryFeedbackResolver(config, true);
403+
resolveRequest(modifiedConfig, contextMock, feedbackPackage, 'web');
404+
405+
ExpectToBeCalledWithMetroParameters(originalResolverMock, contextMock, feedbackPackage, 'web');
406+
});
407+
408+
test('removes Feedback when platform is web and includeWebFeedback is false', () => {
409+
const modifiedConfig = withSentryFeedbackResolver(config, false);
410+
const result = resolveRequest(modifiedConfig, contextMock, feedbackPackage, 'web');
411+
412+
expect(result).toEqual({ type: 'empty' });
413+
expect(originalResolverMock).not.toHaveBeenCalled();
414+
});
415+
416+
test('keep Feedback when platform is android and includeWebFeedback is true', () => {
417+
const modifiedConfig = withSentryFeedbackResolver(config, true);
418+
resolveRequest(modifiedConfig, contextMock, feedbackPackage, 'android');
419+
420+
ExpectToBeCalledWithMetroParameters(originalResolverMock, contextMock, feedbackPackage, 'android');
421+
});
422+
423+
test('removes Feedback when platform is android and includeWebFeedback is false', () => {
424+
const modifiedConfig = withSentryFeedbackResolver(config, false);
425+
const result = resolveRequest(modifiedConfig, contextMock, feedbackPackage, 'android');
426+
427+
expect(result).toEqual({ type: 'empty' });
428+
expect(originalResolverMock).not.toHaveBeenCalled();
429+
});
430+
431+
test('removes Feedback when platform is android and includeWebFeedback is undefined', () => {
432+
const modifiedConfig = withSentryFeedbackResolver(config, undefined);
433+
const result = resolveRequest(modifiedConfig, contextMock, feedbackPackage, 'android');
434+
435+
expect(result).toEqual({ type: 'empty' });
436+
expect(originalResolverMock).not.toHaveBeenCalled();
437+
});
438+
439+
test('keep Feedback when platform is undefined and includeWebFeedback is null', () => {
440+
const modifiedConfig = withSentryFeedbackResolver(config, undefined);
441+
resolveRequest(modifiedConfig, contextMock, feedbackPackage, null);
442+
443+
ExpectToBeCalledWithMetroParameters(originalResolverMock, contextMock, feedbackPackage, null);
444+
});
445+
446+
test('keep Feedback when platform is ios and includeWebFeedback is true', () => {
447+
const modifiedConfig = withSentryFeedbackResolver(config, true);
448+
resolveRequest(modifiedConfig, contextMock, feedbackPackage, 'ios');
449+
450+
ExpectToBeCalledWithMetroParameters(originalResolverMock, contextMock, feedbackPackage, 'ios');
451+
});
452+
453+
test('removes Feedback when platform is ios and includeWebFeedback is false', () => {
454+
const modifiedConfig = withSentryFeedbackResolver(config, false);
455+
const result = resolveRequest(modifiedConfig, contextMock, feedbackPackage, 'ios');
456+
457+
expect(result).toEqual({ type: 'empty' });
458+
expect(originalResolverMock).not.toHaveBeenCalled();
459+
});
460+
461+
test('removes Feedback when platform is ios and includeWebFeedback is undefined', () => {
462+
const modifiedConfig = withSentryFeedbackResolver(config, undefined);
463+
const result = resolveRequest(modifiedConfig, contextMock, feedbackPackage, 'ios');
464+
465+
expect(result).toEqual({ type: 'empty' });
466+
expect(originalResolverMock).not.toHaveBeenCalled();
467+
});
468+
});
469+
470+
test('calls originalResolver when moduleName is not @sentry-internal/feedback', () => {
471+
const modifiedConfig = withSentryFeedbackResolver(config, true);
472+
const moduleName = 'some/other/module';
473+
resolveRequest(modifiedConfig, contextMock, moduleName, 'web');
474+
475+
ExpectToBeCalledWithMetroParameters(originalResolverMock, contextMock, moduleName, 'web');
476+
});
477+
478+
test('calls originalResolver when moduleName is not @sentry-internal/feedback and includeWebFeedback set to false', () => {
479+
const modifiedConfig = withSentryFeedbackResolver(config, false);
480+
const moduleName = 'some/other/module';
481+
resolveRequest(modifiedConfig, contextMock, moduleName, 'web');
482+
483+
ExpectToBeCalledWithMetroParameters(originalResolverMock, contextMock, moduleName, 'web');
484+
});
485+
486+
test('calls default resolver on new metro resolver when originalResolver is not provided', () => {
487+
if (oldMetro) {
488+
return;
489+
}
490+
491+
const modifiedConfig = withSentryFeedbackResolver({ resolver: {} }, true);
492+
const moduleName = 'some/other/module';
493+
const platform = 'web';
494+
resolveRequest(modifiedConfig, contextMock, moduleName, platform);
495+
496+
ExpectToBeCalledWithMetroParameters(contextMock.resolveRequest, contextMock, moduleName, platform);
497+
});
498+
499+
test('throws error when running on old metro and includeWebFeedback is set to false', () => {
500+
if (!oldMetro) {
501+
return;
502+
}
503+
504+
// @ts-expect-error mock.
505+
const mockExit = jest.spyOn(process, 'exit').mockImplementation(() => {});
506+
const modifiedConfig = withSentryFeedbackResolver({ resolver: {} }, true);
507+
const moduleName = 'some/other/module';
508+
resolveRequest(modifiedConfig, contextMock, moduleName, 'web');
509+
510+
expect(mockExit).toHaveBeenCalledWith(-1);
511+
});
512+
513+
type CustomResolverBeforeMetro067 = (
514+
// @ts-expect-error Can't see type CustomResolutionContext
515+
context: CustomResolutionContext,
516+
realModuleName: string,
517+
platform: string | null,
518+
moduleName?: string,
519+
// @ts-expect-error Can't see type CustomResolutionContext
520+
) => Resolution;
521+
522+
function resolveRequest(
523+
metroConfig: MetroConfig,
524+
context: any,
525+
moduleName: string,
526+
platform: string | null,
527+
// @ts-expect-error Can't see type Resolution.
528+
): Resolution {
529+
if (oldMetro) {
530+
const resolver = metroConfig.resolver?.resolveRequest as CustomResolverBeforeMetro067;
531+
// On older Metro the resolveRequest is the creater resolver.
532+
context.resolveRequest = resolver;
533+
return resolver(context, `real${moduleName}`, platform, moduleName);
534+
}
535+
return metroConfig.resolver?.resolveRequest?.(context, moduleName, platform);
536+
}
537+
538+
function ExpectToBeCalledWithMetroParameters(
539+
received: CustomResolverBeforeMetro067,
540+
contextMock: CustomResolverBeforeMetro067,
541+
moduleName: string,
542+
platform: string | null,
543+
) {
544+
if (oldMetro) {
545+
expect(received).toHaveBeenCalledWith(contextMock, `real${moduleName}`, platform, moduleName);
546+
} else {
547+
expect(received).toHaveBeenCalledWith(contextMock, moduleName, platform);
548+
}
549+
}
550+
});
551+
});
367552
describe('withSentryExcludeServerOnlyResolver', () => {
368553
const SENTRY_CORE_ORIGIN = '/project/node_modules/@sentry/core/build/esm/index.js';
369554

0 commit comments

Comments
 (0)