Skip to content

Commit 25a65cd

Browse files
blakefblakef
committed
Revert [0.76] Fix errors with component stacks reported as warnings
- "Fix errors with component stacks reported as warnings (#46637)": 2da46a8 - "Refactor LogBox tests to spies (#46638)": 8926364 - "Add integration tests for console errors + ExceptionManager (#46636)": 094f036 - "Re-enable integration tests (#46639)": bf40710
1 parent 7a601f4 commit 25a65cd

File tree

4 files changed

+76
-325
lines changed

4 files changed

+76
-325
lines changed

‎packages/react-native/Libraries/LogBox/Data/LogBoxData.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,9 @@ let warningFilter: WarningFilter = function (format) {
8282
return {
8383
finalFormat: format,
8484
forceDialogImmediately: false,
85-
suppressDialog_LEGACY: false,
85+
suppressDialog_LEGACY: true,
8686
suppressCompletely: false,
87-
monitorEvent: 'warning_unhandled',
87+
monitorEvent: 'unknown',
8888
monitorListVersion: 0,
8989
monitorSampleRate: 1,
9090
};

‎packages/react-native/Libraries/LogBox/__tests__/LogBox-integration-test.js

Lines changed: 44 additions & 169 deletions
Original file line numberDiff line numberDiff line change
@@ -11,44 +11,56 @@
1111
import {
1212
DoesNotUseKey,
1313
FragmentWithProp,
14-
ManualConsoleError,
15-
ManualConsoleErrorWithStack,
1614
} from './__fixtures__/ReactWarningFixtures';
1715
import * as React from 'react';
1816

1917
const LogBoxData = require('../Data/LogBoxData');
2018
const TestRenderer = require('react-test-renderer');
2119

22-
const ExceptionsManager = require('../../Core/ExceptionsManager.js');
23-
2420
const installLogBox = () => {
25-
const LogBox = require('../LogBox').default;
21+
const LogBox = require('../LogBox');
22+
2623
LogBox.install();
2724
};
2825

2926
const uninstallLogBox = () => {
30-
const LogBox = require('../LogBox').default;
27+
const LogBox = require('../LogBox');
3128
LogBox.uninstall();
3229
};
3330

34-
// TODO: we can remove all the symetric matchers once OSS lands component stack frames.
35-
// For now, the component stack parsing differs in ways we can't easily detect in this test.
36-
describe('LogBox', () => {
31+
const BEFORE_SLASH_RE = /(?:\/[a-zA-Z]+\/)(.+?)(?:\/.+)\//;
32+
33+
const cleanPath = message => {
34+
return message.replace(BEFORE_SLASH_RE, '/path/to/');
35+
};
36+
37+
const cleanLog = logs => {
38+
return logs.map(log => {
39+
return {
40+
...log,
41+
componentStack: log.componentStack.map(stack => ({
42+
...stack,
43+
fileName: cleanPath(stack.fileName),
44+
})),
45+
};
46+
});
47+
};
48+
49+
// TODO(T71117418): Re-enable skipped LogBox integration tests once React component
50+
// stack frames are the same internally and in open source.
51+
// eslint-disable-next-line jest/no-disabled-tests
52+
describe.skip('LogBox', () => {
3753
const {error, warn} = console;
3854
const mockError = jest.fn();
3955
const mockWarn = jest.fn();
4056

4157
beforeEach(() => {
4258
jest.resetModules();
4359
jest.restoreAllMocks();
44-
jest.spyOn(console, 'error').mockImplementation(() => {});
4560

4661
mockError.mockClear();
4762
mockWarn.mockClear();
48-
// Reset ExceptionManager patching.
49-
if (console._errorOriginal) {
50-
console._errorOriginal = null;
51-
}
63+
5264
(console: any).error = mockError;
5365
(console: any).warn = mockWarn;
5466
});
@@ -67,10 +79,7 @@ describe('LogBox', () => {
6779
// so we can assert on what React logs.
6880
jest.spyOn(console, 'error');
6981

70-
let output;
71-
TestRenderer.act(() => {
72-
output = TestRenderer.create(<DoesNotUseKey />);
73-
});
82+
const output = TestRenderer.create(<DoesNotUseKey />);
7483

7584
// The key error should always be the highest severity.
7685
// In LogBox, we expect these errors to:
@@ -79,37 +88,16 @@ describe('LogBox', () => {
7988
// - Pass to console.error, with a "Warning" prefix so it does not pop a RedBox.
8089
expect(output).toBeDefined();
8190
expect(mockWarn).not.toBeCalled();
82-
expect(console.error).toBeCalledTimes(1);
83-
expect(console.error.mock.calls[0]).toEqual([
84-
'Each child in a list should have a unique "key" prop.%s%s See https://react.dev/link/warning-keys for more information.%s',
85-
'\n\nCheck the render method of `DoesNotUseKey`.',
86-
'',
87-
expect.stringMatching('at DoesNotUseKey'),
88-
]);
89-
expect(spy).toHaveBeenCalledWith({
90-
level: 'error',
91-
category: expect.stringContaining(
92-
'Warning: Each child in a list should have a unique',
93-
),
94-
componentStack: expect.anything(),
95-
componentStackType: 'stack',
96-
message: {
97-
content:
98-
'Warning: Each child in a list should have a unique "key" prop.\n\nCheck the render method of `DoesNotUseKey`. See https://react.dev/link/warning-keys for more information.',
99-
substitutions: [
100-
{length: 45, offset: 62},
101-
{length: 0, offset: 107},
102-
],
103-
},
104-
});
91+
expect(console.error.mock.calls[0].map(cleanPath)).toMatchSnapshot(
92+
'Log sent from React',
93+
);
94+
expect(cleanLog(spy.mock.calls[0])).toMatchSnapshot('Log added to LogBox');
95+
expect(mockError.mock.calls[0].map(cleanPath)).toMatchSnapshot(
96+
'Log passed to console error',
97+
);
10598

10699
// The Warning: prefix is added due to a hack in LogBox to prevent double logging.
107-
// We also interpolate the string before passing to the underlying console method.
108-
expect(mockError.mock.calls[0]).toEqual([
109-
expect.stringMatching(
110-
'Warning: Each child in a list should have a unique "key" prop.\n\nCheck the render method of `DoesNotUseKey`. See https://react.dev/link/warning-keys for more information.\n at ',
111-
),
112-
]);
100+
expect(mockError.mock.calls[0][0].startsWith('Warning: ')).toBe(true);
113101
});
114102

115103
it('integrates with React and handles a fragment warning in LogBox', () => {
@@ -120,10 +108,7 @@ describe('LogBox', () => {
120108
// so we can assert on what React logs.
121109
jest.spyOn(console, 'error');
122110

123-
let output;
124-
TestRenderer.act(() => {
125-
output = TestRenderer.create(<FragmentWithProp />);
126-
});
111+
const output = TestRenderer.create(<FragmentWithProp />);
127112

128113
// The fragment warning is not as severe. For this warning we don't want to
129114
// pop open a dialog, so we show a collapsed error UI.
@@ -133,125 +118,15 @@ describe('LogBox', () => {
133118
// - Pass to console.error, with a "Warning" prefix so it does not pop a RedBox.
134119
expect(output).toBeDefined();
135120
expect(mockWarn).not.toBeCalled();
136-
expect(console.error).toBeCalledTimes(1);
137-
expect(console.error.mock.calls[0]).toEqual([
138-
'Invalid prop `%s` supplied to `React.Fragment`. React.Fragment can only have `key` and `children` props.%s',
139-
'invalid',
140-
expect.stringMatching('at FragmentWithProp'),
141-
]);
142-
expect(spy).toHaveBeenCalledWith({
143-
level: 'error',
144-
category: expect.stringContaining('Warning: Invalid prop'),
145-
componentStack: expect.anything(),
146-
componentStackType: expect.stringMatching(/(stack|legacy)/),
147-
message: {
148-
content:
149-
'Warning: Invalid prop `invalid` supplied to `React.Fragment`. React.Fragment can only have `key` and `children` props.',
150-
substitutions: [{length: 7, offset: 23}],
151-
},
152-
});
153-
154-
// The Warning: prefix is added due to a hack in LogBox to prevent double logging.
155-
// We also interpolate the string before passing to the underlying console method.
156-
expect(mockError.mock.calls[0]).toEqual([
157-
expect.stringMatching(
158-
'Warning: Invalid prop `invalid` supplied to `React.Fragment`. React.Fragment can only have `key` and `children` props.\n at FragmentWithProp',
159-
),
160-
]);
161-
});
162-
163-
it('handles a manual console.error without a component stack in LogBox', () => {
164-
const LogBox = require('../LogBox').default;
165-
const spy = jest.spyOn(LogBox, 'addException');
166-
installLogBox();
167-
168-
// console.error handling depends on installing the ExceptionsManager error reporter.
169-
ExceptionsManager.installConsoleErrorReporter();
170-
171-
// Spy console.error after LogBox is installed
172-
// so we can assert on what React logs.
173-
jest.spyOn(console, 'error');
174-
175-
let output;
176-
TestRenderer.act(() => {
177-
output = TestRenderer.create(<ManualConsoleError />);
178-
});
179-
180-
// Manual console errors should show a collapsed error dialog.
181-
// When there is no component stack, we expect these errors to:
182-
// - Go to the LogBox patch and fall through to console.error.
183-
// - Get picked up by the ExceptionsManager console.error override.
184-
// - Get passed back to LogBox via addException (non-fatal).
185-
expect(output).toBeDefined();
186-
expect(mockWarn).not.toBeCalled();
187-
expect(spy).toBeCalledTimes(1);
188-
expect(console.error).toBeCalledTimes(1);
189-
expect(console.error.mock.calls[0]).toEqual(['Manual console error']);
190-
expect(spy).toHaveBeenCalledWith({
191-
id: 1,
192-
isComponentError: false,
193-
isFatal: false,
194-
name: 'console.error',
195-
originalMessage: 'Manual console error',
196-
message: 'console.error: Manual console error',
197-
extraData: expect.anything(),
198-
componentStack: null,
199-
stack: expect.anything(),
200-
});
201-
202-
// No Warning: prefix is added due since this is falling through.
203-
expect(mockError.mock.calls[0]).toEqual(['Manual console error']);
204-
});
205-
206-
it('handles a manual console.error with a component stack in LogBox', () => {
207-
const spy = jest.spyOn(LogBoxData, 'addLog');
208-
installLogBox();
209-
210-
// console.error handling depends on installing the ExceptionsManager error reporter.
211-
ExceptionsManager.installConsoleErrorReporter();
212-
213-
// Spy console.error after LogBox is installed
214-
// so we can assert on what React logs.
215-
jest.spyOn(console, 'error');
216-
217-
let output;
218-
TestRenderer.act(() => {
219-
output = TestRenderer.create(<ManualConsoleErrorWithStack />);
220-
});
221-
222-
// Manual console errors should show a collapsed error dialog.
223-
// When there is a component stack, we expect these errors to:
224-
// - Go to the LogBox patch and be detected as a React error.
225-
// - Check the warning filter to see if there is a fiter setting.
226-
// - Call console.error with the parsed error.
227-
// - Get picked up by ExceptionsManager console.error override.
228-
// - Log to console.error.
229-
expect(output).toBeDefined();
230-
expect(mockWarn).not.toBeCalled();
231-
expect(console.error).toBeCalledTimes(1);
232-
expect(spy).toBeCalledTimes(1);
233-
expect(console.error.mock.calls[0]).toEqual([
234-
expect.stringContaining(
235-
'Manual console error\n at ManualConsoleErrorWithStack',
236-
),
237-
]);
238-
expect(spy).toHaveBeenCalledWith({
239-
level: 'error',
240-
category: expect.stringContaining('Warning: Manual console error'),
241-
componentStack: expect.anything(),
242-
componentStackType: 'stack',
243-
message: {
244-
content: 'Warning: Manual console error',
245-
substitutions: [],
246-
},
247-
});
121+
expect(console.error.mock.calls[0].map(cleanPath)).toMatchSnapshot(
122+
'Log sent from React',
123+
);
124+
expect(cleanLog(spy.mock.calls[0])).toMatchSnapshot('Log added to LogBox');
125+
expect(mockError.mock.calls[0].map(cleanPath)).toMatchSnapshot(
126+
'Log passed to console error',
127+
);
248128

249129
// The Warning: prefix is added due to a hack in LogBox to prevent double logging.
250-
// We also interpolate the string before passing to the underlying console method.
251-
expect(mockError.mock.calls[0]).toEqual([
252-
expect.stringMatching(
253-
'Warning: Manual console error\n at ManualConsoleErrorWithStack',
254-
),
255-
]);
130+
expect(mockError.mock.calls[0][0].startsWith('Warning: ')).toBe(true);
256131
});
257132
});

0 commit comments

Comments
 (0)