Skip to content

Commit daa1f7d

Browse files
krystofwoldrichkrystofwoldrich
authored
chore: Disable AppStart and NativeFrames integration by default in environments without native support (#4897)
* chore: Disable AppStart and NativeFrames integration by default in environments without native support * add changelog
1 parent 480a5d3 commit daa1f7d

5 files changed

Lines changed: 122 additions & 70 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
### Changes
1212

1313
- Remove deprecated `appOwnership` constant use in Expo Go detection ([#4893](https://github.com/getsentry/sentry-react-native/pull/4893))
14+
- Disable AppStart and NativeFrames in unsupported environments (web, Expo Go) ([#4897](https://github.com/getsentry/sentry-react-native/pull/4897))
1415

1516
## 7.0.0-beta.0
1617

‎packages/core/src/js/integrations/default.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,11 +98,11 @@ export function getDefaultIntegrations(options: ReactNativeClientOptions): Integ
9898
// that's different from prev imp here and might lead misconfiguration
9999
// `tracesSampleRate: undefined` should not enable tracing
100100
const hasTracingEnabled = typeof options.tracesSampleRate === 'number' || typeof options.tracesSampler === 'function';
101-
if (hasTracingEnabled && options.enableAppStartTracking) {
101+
if (hasTracingEnabled && options.enableAppStartTracking && options.enableNative) {
102102
integrations.push(appStartIntegration());
103103
}
104104
const nativeFramesIntegrationInstance = createNativeFramesIntegrations(
105-
hasTracingEnabled && options.enableNativeFramesTracking,
105+
hasTracingEnabled && options.enableNativeFramesTracking && options.enableNative,
106106
);
107107
if (nativeFramesIntegrationInstance) {
108108
integrations.push(nativeFramesIntegrationInstance);

‎packages/core/src/js/sdk.tsx‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,7 @@ import {
1111
stackParserFromStackParserOptions,
1212
withScope as coreWithScope,
1313
} from '@sentry/core';
14-
import {
15-
defaultStackParser,
16-
makeFetchTransport,
17-
} from '@sentry/react';
14+
import { defaultStackParser, makeFetchTransport, Profiler } from '@sentry/react';
1815
import * as React from 'react';
1916
import { ReactNativeClient } from './client';
2017
import { FeedbackWidgetProvider } from './feedback/FeedbackWidgetProvider';
@@ -27,7 +24,7 @@ import { TouchEventBoundary } from './touchevents';
2724
import { ReactNativeProfiler } from './tracing';
2825
import { useEncodePolyfill } from './transports/encodePolyfill';
2926
import { DEFAULT_BUFFER_SIZE, makeNativeTransportFactory } from './transports/native';
30-
import { getDefaultEnvironment, isExpoGo, isRunningInMetroDevServer } from './utils/environment';
27+
import { getDefaultEnvironment, isExpoGo, isRunningInMetroDevServer, isWeb } from './utils/environment';
3128
import { safeFactory, safeTracesSampler } from './utils/safe';
3229
import { NATIVE } from './wrapper';
3330

@@ -170,14 +167,16 @@ export function wrap<P extends Record<string, unknown>>(
170167
updateProps: {}
171168
};
172169

173-
const RootApp: React.FC<P> = (appProps) => {
170+
const ProfilerComponent = isWeb() ? Profiler : ReactNativeProfiler;
171+
172+
const RootApp: React.FC<P> = appProps => {
174173
return (
175174
<TouchEventBoundary {...(options?.touchEventBoundaryProps ?? {})}>
176-
<ReactNativeProfiler {...profilerProps}>
175+
<ProfilerComponent {...profilerProps}>
177176
<FeedbackWidgetProvider>
178177
<RootComponent {...appProps} />
179178
</FeedbackWidgetProvider>
180-
</ReactNativeProfiler>
179+
</ProfilerComponent>
181180
</TouchEventBoundary>
182181
);
183182
};

‎packages/core/test/sdk.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -678,6 +678,15 @@ describe('Tests the SDK functionality', () => {
678678
expectNotIntegration('AppStart');
679679
});
680680

681+
it('when tracing enabled app start without native (on web, Expo Go) integration is not added', () => {
682+
init({
683+
tracesSampleRate: 0.5,
684+
enableNative: false,
685+
});
686+
687+
expectNotIntegration('AppStart');
688+
});
689+
681690
it('no native frames integration by default', () => {
682691
init({});
683692

@@ -701,6 +710,15 @@ describe('Tests the SDK functionality', () => {
701710
expectNotIntegration('NativeFrames');
702711
});
703712

713+
it('when tracing enabled (on web, Expo Go) native frames integration is not added', () => {
714+
init({
715+
tracesSampleRate: 0.5,
716+
enableNative: false,
717+
});
718+
719+
expectNotIntegration('NativeFrames');
720+
});
721+
704722
it('when tracing not set stall tracking the integration is not added', () => {
705723
init({});
706724

‎packages/core/test/wrap.mocked.test.tsx‎

Lines changed: 94 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,33 @@
22
import { render } from '@testing-library/react-native';
33
import * as React from 'react';
44
import type { ReactNativeWrapperOptions } from 'src/js/options';
5+
import * as environment from '../src/js/utils/environment';
56

67
jest.doMock('../src/js/touchevents', () => {
78
return {
8-
TouchEventBoundary: ({ children }: { children: React.ReactNode }) => (
9-
// eslint-disable-next-line react/no-unknown-property
10-
<div testID="touch-boundaryID">{children}</div>
11-
),
12-
}
9+
TouchEventBoundary: ({ children }: { children: React.ReactNode }) => (
10+
// eslint-disable-next-line react/no-unknown-property
11+
<div testID="touch-boundaryID">{children}</div>
12+
),
13+
};
1314
});
1415

1516
jest.doMock('../src/js/tracing', () => {
1617
return {
17-
ReactNativeProfiler: jest.fn(({ children }: { children: React.ReactNode }) => (
18-
// eslint-disable-next-line react/no-unknown-property
19-
<div testID="profilerID">{children}</div>
20-
)),
21-
}
18+
ReactNativeProfiler: jest.fn(({ children }: { children: React.ReactNode }) => (
19+
// eslint-disable-next-line react/no-unknown-property
20+
<div testID="react-native-profilerID">{children}</div>
21+
)),
22+
};
23+
});
24+
25+
jest.doMock('@sentry/react', () => {
26+
return {
27+
Profiler: jest.fn(({ children }: { children: React.ReactNode }) => (
28+
// eslint-disable-next-line react/no-unknown-property
29+
<div testID="react-profilerID">{children}</div>
30+
)),
31+
};
2232
});
2333

2434
jest.doMock('../src/js/feedback/FeedbackWidgetProvider', () => {
@@ -34,26 +44,50 @@ import { wrap } from '../src/js/sdk';
3444
import { ReactNativeProfiler } from '../src/js/tracing';
3545

3646
describe('Sentry.wrap', () => {
47+
const DummyComponent: React.FC<{ value?: string }> = ({ value }) => <div>{value}</div>;
3748

38-
const DummyComponent: React.FC<{ value?: string }> = ({ value }) => <div>{value}</div>;
49+
it('should not enforce any keys on the wrapped component', () => {
50+
const Mock: React.FC<{ test: 23 }> = () => <></>;
51+
const ActualWrapped = wrap(Mock);
3952

40-
it('should not enforce any keys on the wrapped component', () => {
41-
const Mock: React.FC<{ test: 23 }> = () => <></>;
42-
const ActualWrapped = wrap(Mock);
53+
expect(typeof ActualWrapped.defaultProps).toBe(typeof Mock.defaultProps);
54+
});
4355

44-
expect(typeof ActualWrapped.defaultProps).toBe(typeof Mock.defaultProps);
45-
});
56+
it('wraps components with Sentry wrappers', () => {
57+
const Wrapped = wrap(DummyComponent);
58+
const renderResult = render(<Wrapped value="wrapped" />);
4659

47-
it('wraps components with Sentry wrappers', () => {
48-
const Wrapped = wrap(DummyComponent);
49-
const renderResult = render(<Wrapped value="wrapped" />);
60+
expect(renderResult.toJSON()).toMatchInlineSnapshot(`
61+
<div
62+
testID="touch-boundaryID"
63+
>
64+
<div
65+
testID="react-native-profilerID"
66+
>
67+
<div
68+
testID="feedback-widgetID"
69+
>
70+
<div>
71+
wrapped
72+
</div>
73+
</div>
74+
</div>
75+
</div>
76+
`);
77+
});
78+
79+
it('wraps components with JS React Profiler on web', () => {
80+
jest.spyOn(environment, 'isWeb').mockReturnValueOnce(true);
5081

51-
expect(renderResult.toJSON()).toMatchInlineSnapshot(`
82+
const Wrapped = wrap(DummyComponent);
83+
const renderResult = render(<Wrapped value="wrapped" />);
84+
85+
expect(renderResult.toJSON()).toMatchInlineSnapshot(`
5286
<div
5387
testID="touch-boundaryID"
5488
>
5589
<div
56-
testID="profilerID"
90+
testID="react-profilerID"
5791
>
5892
<div
5993
testID="feedback-widgetID"
@@ -65,46 +99,46 @@ describe('Sentry.wrap', () => {
6599
</div>
66100
</div>
67101
`);
102+
});
103+
104+
describe('ReactNativeProfiler', () => {
105+
it('uses given options when set', () => {
106+
const options: ReactNativeWrapperOptions = {
107+
profilerProps: { disabled: false, includeRender: true, includeUpdates: true },
108+
};
109+
const Wrapped = wrap(DummyComponent, options);
110+
render(<Wrapped value="wrapped" />);
111+
112+
expect(ReactNativeProfiler).toHaveBeenCalledWith(
113+
expect.objectContaining({
114+
name: 'Root',
115+
disabled: false,
116+
includeRender: true,
117+
includeUpdates: true,
118+
}),
119+
expect.anything(),
120+
);
121+
122+
expect(ReactNativeProfiler).not.toHaveBeenCalledWith(
123+
expect.objectContaining({
124+
updateProps: expect.anything(),
125+
}),
126+
);
68127
});
69128

70-
describe('ReactNativeProfiler', () => {
71-
it('uses given options when set', () => {
72-
const options: ReactNativeWrapperOptions = {
73-
profilerProps: { disabled: false, includeRender: true, includeUpdates: true },
74-
};
75-
const Wrapped = wrap(DummyComponent, options);
76-
render(<Wrapped value="wrapped" />);
77-
78-
expect(ReactNativeProfiler).toHaveBeenCalledWith(
79-
expect.objectContaining({
80-
name: 'Root',
81-
disabled: false,
82-
includeRender: true,
83-
includeUpdates: true
84-
}),
85-
expect.anything(),
86-
);
87-
88-
expect(ReactNativeProfiler).not.toHaveBeenCalledWith(
89-
expect.objectContaining({
90-
updateProps: expect.anything(),
91-
})
92-
);
93-
});
94-
95-
it('ignore updateProps when set', () => {
96-
const { wrap } = jest.requireActual('../src/js/sdk');
97-
98-
const Wrapped = wrap(DummyComponent, { updateProps: ['prop'] });
99-
render(<Wrapped value="wrapped" />);
100-
101-
expect(ReactNativeProfiler).toHaveBeenCalledWith(
102-
expect.objectContaining({
103-
name: 'Root',
104-
updateProps: {},
105-
}),
106-
expect.anything(),
107-
);
108-
});
129+
it('ignore updateProps when set', () => {
130+
const { wrap } = jest.requireActual('../src/js/sdk');
131+
132+
const Wrapped = wrap(DummyComponent, { updateProps: ['prop'] });
133+
render(<Wrapped value="wrapped" />);
134+
135+
expect(ReactNativeProfiler).toHaveBeenCalledWith(
136+
expect.objectContaining({
137+
name: 'Root',
138+
updateProps: {},
139+
}),
140+
expect.anything(),
141+
);
109142
});
110143
});
144+
});

0 commit comments

Comments
 (0)