Hardened logic around when and how to patch console methods
Brian Vaughn committed
Jul 17, 2019 at 15:08 UTC
9fb753b5cd399ccc702bd3e53dd8d86a739e3504
4 files changed
+136
-61
src/__tests__/console-test.js
+15
-31
@@ -4,10 +4,9 @@ describe('console', () => {
4
let React;
5
let ReactDOM;
6
let act;
7
- let enableConsole;
8
- let disableConsole;
7
let fakeConsole;
8
let mockError;
9
+ let mockInfo;
10
let mockLog;
11
let mockWarn;
12
let patchConsole;
@@ -15,8 +14,6 @@ describe('console', () => {
14
15
beforeEach(() => {
16
const Console = require('../backend/console');
18
- enableConsole = Console.enable;
19
- disableConsole = Console.disable;
17
patchConsole = Console.patch;
18
unpatchConsole = Console.unpatch;
19
@@ -37,25 +34,36 @@ describe('console', () => {
34
// Patching the real console is too complicated,
35
// because Jest itself has hooks into it as does our test env setup.
36
mockError = jest.fn();
37
+ mockInfo = jest.fn();
38
mockLog = jest.fn();
39
mockWarn = jest.fn();
40
fakeConsole = {
41
error: mockError,
42
+ info: mockInfo,
43
log: mockLog,
44
warn: mockWarn,
45
};
46
48
- patchConsole(fakeConsole);
47
+ Console.dangerous_setTargetConsoleForTesting(fakeConsole);
48
+
49
+ patchConsole();
50
});
51
52
function normalizeCodeLocInfo(str) {
53
return str && str.replace(/\(at .+?:\d+\)/g, '(at **)');
54
}
55
56
+ it('should not patch console methods that do not receive component stacks', () => {
57
+ expect(fakeConsole.error).not.toBe(mockError);
58
+ expect(fakeConsole.info).toBe(mockInfo);
59
+ expect(fakeConsole.log).toBe(mockLog);
60
+ expect(fakeConsole.warn).not.toBe(mockWarn);
61
+ });
62
+
63
it('should only patch the console once', () => {
64
const { error, warn } = fakeConsole;
65
58
- patchConsole(fakeConsole);
66
+ patchConsole();
67
68
expect(fakeConsole.error).toBe(error);
69
expect(fakeConsole.warn).toBe(warn);
@@ -89,30 +97,6 @@ describe('console', () => {
97
expect(mockError.mock.calls[0][0]).toBe('error');
98
});
99
92
- it('should suppress console logging when disabled', () => {
93
- disableConsole();
94
- fakeConsole.log('log');
95
- fakeConsole.warn('warn');
96
- fakeConsole.error('error');
97
- expect(mockLog).toHaveBeenCalledTimes(0);
98
- expect(mockWarn).toHaveBeenCalledTimes(0);
99
- expect(mockError).toHaveBeenCalledTimes(0);
100
-
101
- enableConsole();
102
- fakeConsole.log('log');
103
- fakeConsole.warn('warn');
104
- fakeConsole.error('error');
105
- expect(mockLog).toHaveBeenCalledTimes(1);
106
- expect(mockLog.mock.calls[0]).toHaveLength(1);
107
- expect(mockLog.mock.calls[0][0]).toBe('log');
108
- expect(mockWarn).toHaveBeenCalledTimes(1);
109
- expect(mockWarn.mock.calls[0]).toHaveLength(1);
110
- expect(mockWarn.mock.calls[0][0]).toBe('warn');
111
- expect(mockError).toHaveBeenCalledTimes(1);
112
- expect(mockError.mock.calls[0]).toHaveLength(1);
113
- expect(mockError.mock.calls[0][0]).toBe('error');
114
- });
115
-
100
it('should not append multiple stacks', () => {
101
const Child = () => {
102
fakeConsole.warn('warn\n in Child (at fake.js:123)');
@@ -330,7 +314,7 @@ describe('console', () => {
314
expect(mockError.mock.calls[0]).toHaveLength(1);
315
expect(mockError.mock.calls[0][0]).toBe('error');
316
333
- patchConsole(fakeConsole);
317
+ patchConsole();
318
act(() => ReactDOM.render(<Child />, document.createElement('div')));
319
320
expect(mockWarn).toHaveBeenCalledTimes(2);
src/__tests__/inspectedElementContext-test.js
+68
@@ -255,6 +255,74 @@ describe('InspectedElementContext', () => {
255
done();
256
});
257
258
+ it('should temporarily disable console logging when re-running a component to inspect its hooks', async done => {
259
+ let targetRenderCount = 0;
260
+
261
+ const errorSpy = ((console: any).error = jest.fn());
262
+ const infoSpy = ((console: any).info = jest.fn());
263
+ const logSpy = ((console: any).log = jest.fn());
264
+ const warnSpy = ((console: any).warn = jest.fn());
265
+
266
+ const Target = React.memo(props => {
267
+ targetRenderCount++;
268
+ console.error('error');
269
+ console.info('info');
270
+ console.log('log');
271
+ console.warn('warn');
272
+ React.useState(0);
273
+ return null;
274
+ });
275
+
276
+ const container = document.createElement('div');
277
+ await utils.actAsync(() =>
278
+ ReactDOM.render(<Target a={1} b="abc" />, container)
279
+ );
280
+
281
+ expect(targetRenderCount).toBe(1);
282
+ expect(errorSpy).toHaveBeenCalledTimes(1);
283
+ expect(errorSpy).toHaveBeenCalledWith('error');
284
+ expect(infoSpy).toHaveBeenCalledTimes(1);
285
+ expect(infoSpy).toHaveBeenCalledWith('info');
286
+ expect(logSpy).toHaveBeenCalledTimes(1);
287
+ expect(logSpy).toHaveBeenCalledWith('log');
288
+ expect(warnSpy).toHaveBeenCalledTimes(1);
289
+ expect(warnSpy).toHaveBeenCalledWith('warn');
290
+
291
+ const id = ((store.getElementIDAtIndex(0): any): number);
292
+
293
+ let inspectedElement = null;
294
+
295
+ function Suspender({ target }) {
296
+ const { getInspectedElement } = React.useContext(InspectedElementContext);
297
+ inspectedElement = getInspectedElement(target);
298
+ return null;
299
+ }
300
+
301
+ await utils.actAsync(
302
+ () =>
303
+ TestRenderer.create(
304
+ <Contexts
305
+ defaultSelectedElementID={id}
306
+ defaultSelectedElementIndex={1}
307
+ >
308
+ <React.Suspense fallback={null}>
309
+ <Suspender target={id} />
310
+ </React.Suspense>
311
+ </Contexts>
312
+ ),
313
+ false
314
+ );
315
+
316
+ expect(inspectedElement).not.toBe(null);
317
+ expect(targetRenderCount).toBe(2);
318
+ expect(errorSpy).toHaveBeenCalledTimes(1);
319
+ expect(infoSpy).toHaveBeenCalledTimes(1);
320
+ expect(logSpy).toHaveBeenCalledTimes(1);
321
+ expect(warnSpy).toHaveBeenCalledTimes(1);
322
+
323
+ done();
324
+ });
325
+
326
it('should support simple data types', async done => {
327
const Example = () => null;
328
src/backend/console.js
+35
-25
@@ -5,6 +5,8 @@ import describeComponentFrame from './describeComponentFrame';
5
6
import type { Fiber, ReactRenderer } from './types';
7
8
+const APPEND_STACK_TO_METHODS = ['error', 'trace', 'warn'];
9
+
10
const FRAME_REGEX = /\n {4}in /;
11
12
const injectedRenderers: Map<
@@ -15,17 +17,29 @@ const injectedRenderers: Map<
17
|}
18
> = new Map();
19
18
-let isDisabled: boolean = false;
20
+let targetConsole: Object = console;
21
+let targetConsoleMethods = {};
22
+for (let method in console) {
23
+ targetConsoleMethods[method] = console[method];
24
+}
25
+
26
let unpatchFn: null | (() => void) = null;
27
21
-export function disable(): void {
22
- isDisabled = true;
23
-}
28
+// Enables e.g. Jest tests to inject a mock console object.
29
+export function dangerous_setTargetConsoleForTesting(
30
+ targetConsoleForTesting: Object
31
+): void {
32
+ targetConsole = targetConsoleForTesting;
33
25
-export function enable(): void {
26
- isDisabled = false;
34
+ targetConsoleMethods = {};
35
+ for (let method in targetConsole) {
36
+ targetConsoleMethods[method] = console[method];
37
+ }
38
}
39
40
+// v16 renderers should use this method to inject internals necessary to generate a component stack.
41
+// These internals will be used if the console is patched.
42
+// Injecting them separately allows the console to easily be patched or unpacted later (at runtime).
43
export function registerRenderer(renderer: ReactRenderer): void {
44
const { getCurrentFiber, findFiberByHostInstance, version } = renderer;
45
@@ -44,16 +58,18 @@ export function registerRenderer(renderer: ReactRenderer): void {
58
}
59
}
60
47
-export function patch(targetConsole?: Object = console): void {
61
+// Patches whitelisted console methods to append component stack for the current fiber.
62
+// Call unpatch() to remove the injected behavior.
63
+export function patch(): void {
64
if (unpatchFn !== null) {
65
// Don't patch twice.
66
return;
67
}
68
53
- const originalConsoleMethods = { ...targetConsole };
69
+ const originalConsoleMethods = {};
70
71
unpatchFn = () => {
56
- for (let method in targetConsole) {
72
+ for (let method in originalConsoleMethods) {
73
try {
74
// $FlowFixMe property error|warn is not writable.
75
targetConsole[method] = originalConsoleMethods[method];
@@ -61,15 +77,13 @@ export function patch(targetConsole?: Object = console): void {
77
}
78
};
79
64
- for (let method in targetConsole) {
65
- const appendComponentStack =
66
- method === 'error' || method === 'warn' || method === 'trace';
67
-
68
- const originalMethod = targetConsole[method];
69
- const overrideMethod = (...args) => {
70
- if (isDisabled) return;
80
+ APPEND_STACK_TO_METHODS.forEach(method => {
81
+ try {
82
+ const originalMethod = (originalConsoleMethods[method] =
83
+ targetConsole[method]);
84
72
- if (appendComponentStack) {
85
+ // $FlowFixMe property error|warn is not writable.
86
+ targetConsole[method] = (...args) => {
87
// If we are ever called with a string that already has a component stack, e.g. a React error/warning,
88
// don't append a second stack.
89
const alreadyHasComponentStack =
@@ -105,18 +119,14 @@ export function patch(targetConsole?: Object = console): void {
119
}
120
}
121
}
108
- }
109
-
110
- originalMethod(...args);
111
- };
122
113
- try {
114
- // $FlowFixMe property error|warn is not writable.
115
- targetConsole[method] = overrideMethod;
123
+ originalMethod(...args);
124
+ };
125
} catch (error) {}
117
- }
126
+ });
127
}
128
129
+// Removed component stack patch from whitelisted console methods.
130
export function unpatch(): void {
131
if (unpatchFn !== null) {
132
unpatchFn();
src/backend/renderer.js
+18
-5
@@ -40,8 +40,6 @@ import {
40
} from '../constants';
41
import { inspectHooksOfFiber } from './ReactDebugHooks';
42
import {
43
- disable as disableConsole,
44
- enable as enableConsole,
43
patch as patchConsole,
44
registerRenderer as registerRendererWithConsole,
45
} from './console';
@@ -2254,15 +2252,30 @@ export function attach(
2252
2253
let hooks = null;
2254
if (usesHooks) {
2257
- // Suppress console logging while re-rendering
2255
+ const originalConsoleMethods = {};
2256
+
2257
+ // Temporarily disable all console logging before re-running the hook.
2258
+ for (let method in console) {
2259
+ try {
2260
+ originalConsoleMethods[method] = console[method];
2261
+ // $FlowFixMe property error|warn is not writable.
2262
+ console[method] = () => {};
2263
+ } catch (error) {}
2264
+ }
2265
+
2266
try {
2259
- disableConsole();
2267
hooks = inspectHooksOfFiber(
2268
fiber,
2269
(renderer.currentDispatcherRef: any)
2270
);
2271
} finally {
2265
- enableConsole();
2272
+ // Restore original console functionality.
2273
+ for (let method in originalConsoleMethods) {
2274
+ try {
2275
+ // $FlowFixMe property error|warn is not writable.
2276
+ console[method] = originalConsoleMethods[method];
2277
+ } catch (error) {}
2278
+ }
2279
}
2280
}
2281