Skip to content

Commit 1ae4e5e

Browse files
antonisclaude
andcommitted
fix(feedback): Break circular dependency in ShakeToReportBug
Make startShakeListener accept an onShake callback instead of importing showFeedbackWidget directly, breaking the cycle: FeedbackWidgetManager -> ShakeToReportBug -> FeedbackWidgetManager. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 8f49d94 commit 1ae4e5e

4 files changed

Lines changed: 19 additions & 29 deletions

File tree

packages/core/src/js/feedback/FeedbackWidgetManager.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ const resetScreenshotButtonManager = (): void => {
135135

136136
const showFeedbackOnShake = (): void => {
137137
lazyLoadAutoInjectFeedbackIntegration();
138-
startShakeListener();
138+
startShakeListener(showFeedbackWidget);
139139
};
140140

141141
const hideFeedbackOnShake = (): void => {

packages/core/src/js/feedback/FeedbackWidgetProvider.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
SLIDE_ANIMATION_DURATION,
1717
} from './FeedbackWidgetManager';
1818
import { getFeedbackButtonOptions, getFeedbackOptions, getScreenshotButtonOptions, isShakeToReportEnabled } from './integration';
19+
import { showFeedbackWidget } from './FeedbackWidgetManager';
1920
import { ScreenshotButton } from './ScreenshotButton';
2021
import { startShakeListener, stopShakeListener } from './ShakeToReportBug';
2122
import { isModalSupported, isNativeDriverSupportedForColorAnimations } from './utils';
@@ -101,7 +102,7 @@ export class FeedbackWidgetProvider extends React.Component<FeedbackWidgetProvid
101102
});
102103

103104
if (isShakeToReportEnabled()) {
104-
startShakeListener();
105+
startShakeListener(showFeedbackWidget);
105106
}
106107
}
107108

packages/core/src/js/feedback/ShakeToReportBug.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import type { EmitterSubscription, NativeModule } from 'react-native';
33
import { NativeEventEmitter } from 'react-native';
44
import { getRNSentryModule } from '../wrapper';
55
import { isWeb } from '../utils/environment';
6-
import { showFeedbackWidget } from './FeedbackWidgetManager';
76

87
export const OnShakeEventName = 'rn_sentry_on_shake';
98

@@ -19,13 +18,16 @@ const defaultEmitterFactory: EmitterFactory = nativeModule =>
1918
new NativeEventEmitter(nativeModule);
2019

2120
/**
22-
* Starts listening for device shake events and shows the feedback widget when a shake is detected.
21+
* Starts listening for device shake events and invokes the provided callback when a shake is detected.
2322
*
2423
* This starts native shake detection:
2524
* - iOS: Uses UIKit's motion event detection (no permissions required)
2625
* - Android: Uses the accelerometer sensor (no permissions required)
2726
*/
28-
export function startShakeListener(createEmitter: EmitterFactory = defaultEmitterFactory): void {
27+
export function startShakeListener(
28+
onShake: () => void,
29+
createEmitter: EmitterFactory = defaultEmitterFactory,
30+
): void {
2931
if (_shakeSubscription) {
3032
debug.log('Shake listener is already active.');
3133
return;
@@ -44,8 +46,8 @@ export function startShakeListener(createEmitter: EmitterFactory = defaultEmitte
4446

4547
const emitter = createEmitter(nativeModule);
4648
_shakeSubscription = emitter.addListener(OnShakeEventName, () => {
47-
debug.log('Shake detected, showing feedback widget.');
48-
showFeedbackWidget();
49+
debug.log('Shake detected.');
50+
onShake();
4951
});
5052
}
5153

packages/core/test/feedback/ShakeToReportBug.test.tsx

Lines changed: 9 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -85,24 +85,24 @@ describe('ShakeToReportBug', () => {
8585
describe('startShakeListener / stopShakeListener', () => {
8686
it('starts listening for shake events', () => {
8787
const actual = jest.requireActual('../../src/js/feedback/ShakeToReportBug');
88-
actual.startShakeListener(mockEmitterFactory);
88+
actual.startShakeListener(jest.fn(), mockEmitterFactory);
8989

9090
expect(actual.isShakeListenerActive()).toBe(true);
9191
expect(mockEmitterFactory).toHaveBeenCalledTimes(1);
9292
});
9393

9494
it('does not start a second listener if already active', () => {
9595
const actual = jest.requireActual('../../src/js/feedback/ShakeToReportBug');
96-
actual.startShakeListener(mockEmitterFactory);
97-
actual.startShakeListener(mockEmitterFactory);
96+
actual.startShakeListener(jest.fn(), mockEmitterFactory);
97+
actual.startShakeListener(jest.fn(), mockEmitterFactory);
9898

9999
expect(actual.isShakeListenerActive()).toBe(true);
100100
expect(mockEmitterFactory).toHaveBeenCalledTimes(1);
101101
});
102102

103103
it('stops listening for shake events', () => {
104104
const actual = jest.requireActual('../../src/js/feedback/ShakeToReportBug');
105-
actual.startShakeListener(mockEmitterFactory);
105+
actual.startShakeListener(jest.fn(), mockEmitterFactory);
106106
actual.stopShakeListener();
107107

108108
expect(actual.isShakeListenerActive()).toBe(false);
@@ -114,27 +114,14 @@ describe('ShakeToReportBug', () => {
114114
expect(() => actual.stopShakeListener()).not.toThrow();
115115
});
116116

117-
it('calls showFeedbackWidget when shake event is received', async () => {
118-
mockedIsModalSupported.mockReturnValue(true);
119-
120-
const { getByTestId, queryByTestId } = render(
121-
<FeedbackWidgetProvider>
122-
<Text>App Components</Text>
123-
</FeedbackWidgetProvider>,
124-
);
125-
117+
it('invokes onShake callback when shake event is received', () => {
126118
const actual = jest.requireActual('../../src/js/feedback/ShakeToReportBug');
127-
actual.startShakeListener(mockEmitterFactory);
119+
const onShake = jest.fn();
120+
actual.startShakeListener(onShake, mockEmitterFactory);
128121

129-
expect(queryByTestId('feedback-form-modal')).toBeNull();
122+
mockShakeCallback?.();
130123

131-
await act(async () => {
132-
mockShakeCallback?.();
133-
});
134-
135-
await waitFor(() => {
136-
expect(getByTestId('feedback-form-modal')).toBeTruthy();
137-
});
124+
expect(onShake).toHaveBeenCalledTimes(1);
138125
});
139126
});
140127

0 commit comments

Comments
 (0)