Blacklist spyOn(). Add explicit spyOnProd() and spyOnDevAndProd() (#11691)
* Blacklist spyOn(). Add explicit spyOnProd() and spyOnDevAndProd() * Wording tweak. * Fixed lint no-shadow warning
Brian Vaughn committed
Nov 28, 2017 at 14:06 UTC
53ab1948b5bb1da29f121d7f32afa0bea42bffd0
13 files changed
+79
-37
.eslintrc.js
+2
@@ -60,5 +60,7 @@ module.exports = {
60
61
globals: {
62
spyOnDev: true,
63
+ spyOnDevAndProd: true,
64
+ spyOnProd: true,
65
},
66
};
packages/react-dom/src/__tests__/DOMPropertyOperations-test.js
+2
-2
@@ -122,7 +122,7 @@ describe('DOMPropertyOperations', () => {
122
<input type="radio" value="" onChange={function() {}} />,
123
container,
124
);
125
- spyOn(container.firstChild, 'setAttribute');
125
+ spyOnDevAndProd(container.firstChild, 'setAttribute');
126
ReactDOM.render(
127
<input type="radio" value={0} onChange={function() {}} />,
128
container,
@@ -133,7 +133,7 @@ describe('DOMPropertyOperations', () => {
133
it('should always assign the value attribute for non-inputs', function() {
134
var container = document.createElement('div');
135
ReactDOM.render(<progress />, container);
136
- spyOn(container.firstChild, 'setAttribute');
136
+ spyOnDevAndProd(container.firstChild, 'setAttribute');
137
ReactDOM.render(<progress value={30} />, container);
138
ReactDOM.render(<progress value="30" />, container);
139
expect(container.firstChild.setAttribute.calls.count()).toBe(2);
packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.internal.js
+3
-3
@@ -375,14 +375,14 @@ describe('ReactBrowserEventEmitter', () => {
375
});
376
377
it('should listen to events only once', () => {
378
- spyOn(EventTarget.prototype, 'addEventListener');
378
+ spyOnDevAndProd(EventTarget.prototype, 'addEventListener');
379
ReactBrowserEventEmitter.listenTo(ON_CLICK_KEY, document);
380
ReactBrowserEventEmitter.listenTo(ON_CLICK_KEY, document);
381
expect(EventTarget.prototype.addEventListener.calls.count()).toBe(1);
382
});
383
384
it('should work with event plugins without dependencies', () => {
385
- spyOn(EventTarget.prototype, 'addEventListener');
385
+ spyOnDevAndProd(EventTarget.prototype, 'addEventListener');
386
387
ReactBrowserEventEmitter.listenTo(ON_CLICK_KEY, document);
388
@@ -392,7 +392,7 @@ describe('ReactBrowserEventEmitter', () => {
392
});
393
394
it('should work with event plugins with dependencies', () => {
395
- spyOn(EventTarget.prototype, 'addEventListener');
395
+ spyOnDevAndProd(EventTarget.prototype, 'addEventListener');
396
397
ReactBrowserEventEmitter.listenTo(ON_CHANGE_KEY, document);
398
packages/react-dom/src/__tests__/ReactDOMComponent-test.js
+3
-3
@@ -877,7 +877,7 @@ describe('ReactDOMComponent', () => {
877
});
878
879
it('should work error event on <source> element', () => {
880
- spyOn(console, 'log');
880
+ spyOnDevAndProd(console, 'log');
881
var container = document.createElement('div');
882
ReactDOM.render(
883
<video>
@@ -1239,7 +1239,7 @@ describe('ReactDOMComponent', () => {
1239
1240
it('should support custom elements which extend native elements', () => {
1241
var container = document.createElement('div');
1242
- spyOn(document, 'createElement').and.callThrough();
1242
+ spyOnDevAndProd(document, 'createElement').and.callThrough();
1243
ReactDOM.render(<div is="custom-div" />, container);
1244
expect(document.createElement).toHaveBeenCalledWith('div', {
1245
is: 'custom-div',
@@ -1247,7 +1247,7 @@ describe('ReactDOMComponent', () => {
1247
});
1248
1249
it('should work load and error events on <image> element in SVG', () => {
1250
- spyOn(console, 'log');
1250
+ spyOnDevAndProd(console, 'log');
1251
var container = document.createElement('div');
1252
ReactDOM.render(
1253
<svg>
packages/react-dom/src/__tests__/ReactDOMFiber-test.js
+1
-1
@@ -1149,7 +1149,7 @@ describe('ReactDOMFiber', () => {
1149
var actualDocument;
1150
var textNode;
1151
1152
- spyOn(iframeContainer, 'appendChild').and.callFake(node => {
1152
+ spyOnDevAndProd(iframeContainer, 'appendChild').and.callFake(node => {
1153
actualDocument = node.ownerDocument;
1154
textNode = node;
1155
});
packages/react-dom/src/__tests__/ReactDOMInput-test.js
+4
-4
@@ -1247,7 +1247,7 @@ describe('ReactDOMInput', () => {
1247
it('sets type, step, min, max before value always', () => {
1248
var log = [];
1249
var originalCreateElement = document.createElement;
1250
- spyOn(document, 'createElement').and.callFake(function(type) {
1250
+ spyOnDevAndProd(document, 'createElement').and.callFake(function(type) {
1251
var el = originalCreateElement.apply(this, arguments);
1252
if (type === 'input') {
1253
Object.defineProperty(el, 'value', {
@@ -1256,7 +1256,7 @@ describe('ReactDOMInput', () => {
1256
log.push('set value');
1257
},
1258
});
1259
- spyOn(el, 'setAttribute').and.callFake(function(name, value) {
1259
+ spyOnDevAndProd(el, 'setAttribute').and.callFake(function(name, value) {
1260
log.push('set ' + name);
1261
});
1262
}
@@ -1311,7 +1311,7 @@ describe('ReactDOMInput', () => {
1311
1312
var log = [];
1313
var originalCreateElement = document.createElement;
1314
- spyOn(document, 'createElement').and.callFake(function(type) {
1314
+ spyOnDevAndProd(document, 'createElement').and.callFake(function(type) {
1315
var el = originalCreateElement.apply(this, arguments);
1316
if (type === 'input') {
1317
Object.defineProperty(el, 'value', {
@@ -1319,7 +1319,7 @@ describe('ReactDOMInput', () => {
1319
log.push(`node.value = ${strify(val)}`);
1320
},
1321
});
1322
- spyOn(el, 'setAttribute').and.callFake(function(name, val) {
1322
+ spyOnDevAndProd(el, 'setAttribute').and.callFake(function(name, val) {
1323
log.push(`node.setAttribute(${strify(name)}, ${strify(val)})`);
1324
});
1325
}
packages/react-dom/src/__tests__/ReactTestUtils-test.js
+2
-2
@@ -270,7 +270,7 @@ describe('ReactTestUtils', () => {
270
e.persist();
271
},
272
};
273
- spyOn(obj, 'handler').and.callThrough();
273
+ spyOnDevAndProd(obj, 'handler').and.callThrough();
274
const container = document.createElement('div');
275
const instance = ReactDOM.render(
276
<input type="text" onChange={obj.handler} />,
@@ -306,7 +306,7 @@ describe('ReactTestUtils', () => {
306
e.persist();
307
},
308
};
309
- spyOn(obj, 'handler').and.callThrough();
309
+ spyOnDevAndProd(obj, 'handler').and.callThrough();
310
const container = document.createElement('div');
311
const instance = ReactDOM.render(
312
<SomeComponent handleChange={obj.handler} />,
packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.internal.js
+4
-8
@@ -24,8 +24,7 @@ describe('ReactIncrementalErrorLogging', () => {
24
}
25
26
it('should log errors that occur during the begin phase', () => {
27
- // Intentionally spy in production too.
28
- spyOn(console, 'error');
27
+ spyOnDevAndProd(console, 'error');
28
29
class ErrorThrowingComponent extends React.Component {
30
componentWillMount() {
@@ -70,8 +69,7 @@ describe('ReactIncrementalErrorLogging', () => {
69
});
70
71
it('should log errors that occur during the commit phase', () => {
73
- // Intentionally spy in production too.
74
- spyOn(console, 'error');
72
+ spyOnDevAndProd(console, 'error');
73
74
class ErrorThrowingComponent extends React.Component {
75
componentDidMount() {
@@ -121,8 +119,7 @@ describe('ReactIncrementalErrorLogging', () => {
119
try {
120
React = require('react');
121
ReactNoop = require('react-noop-renderer');
124
- // Intentionally spy in production too.
125
- spyOn(console, 'error');
122
+ spyOnDevAndProd(console, 'error');
123
124
class ErrorThrowingComponent extends React.Component {
125
render() {
@@ -169,8 +166,7 @@ describe('ReactIncrementalErrorLogging', () => {
166
});
167
168
it('should relay info about error boundary and retry attempts if applicable', () => {
172
- // Intentionally spy in production too.
173
- spyOn(console, 'error');
169
+ spyOnDevAndProd(console, 'error');
170
171
class ParentComponent extends React.Component {
172
render() {
packages/react/src/__tests__/ReactElementValidator-test.js
+1
-1
@@ -449,7 +449,7 @@ describe('ReactElementValidator', () => {
449
});
450
451
it('should warn if component declares PropTypes instead of propTypes', () => {
452
- spyOn(console, 'error');
452
+ spyOnDevAndProd(console, 'error');
453
class MisspelledPropTypesComponent extends React.Component {
454
static PropTypes = {
455
prop: PropTypes.string,
packages/react/src/__tests__/ReactJSXElementValidator-test.js
+1
-1
@@ -404,7 +404,7 @@ describe('ReactJSXElementValidator', () => {
404
});
405
406
it('should warn if component declares PropTypes instead of propTypes', () => {
407
- spyOn(console, 'error');
407
+ spyOnDevAndProd(console, 'error');
408
class MisspelledPropTypesComponent extends React.Component {
409
render() {
410
return <span>{this.props.prop}</span>;
scripts/jest/setupTests.js
+31
-11
@@ -13,9 +13,31 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) {
13
// https://github.com/facebook/jest/blob/v20.0.4/packages/jest-matchers/src/spyMatchers.js#L160
14
const isSpy = spy => spy.calls && typeof spy.calls.count === 'function';
15
16
- // Dev-only spyOn should be ignored in production runs.
17
- global.spyOnDev =
18
- process.env.NODE_ENV === 'production' ? () => {} : global.spyOn;
16
+ const spyOn = global.spyOn;
17
+ const noop = function() {};
18
+
19
+ // Spying on console methods in production builds can mask errors.
20
+ // This is why we added an explicit spyOnDev() helper.
21
+ // It's too easy to accidentally use the more familiar spyOn() helper though,
22
+ // So we disable it entirely.
23
+ // Spying on both dev and prod will require using both spyOnDev() and spyOnProd().
24
+ global.spyOn = function() {
25
+ throw new Error(
26
+ 'Do not use spyOn(). ' +
27
+ 'It can accidentally hide unexpected errors in production builds. ' +
28
+ 'Use spyOnDev(), spyOnProd(), or spyOnDevAndProd() instead.'
29
+ );
30
+ };
31
+
32
+ if (process.env.NODE_ENV === 'production') {
33
+ global.spyOnDev = noop;
34
+ global.spyOnProd = spyOn;
35
+ global.spyOnDevAndProd = spyOn;
36
+ } else {
37
+ global.spyOnDev = spyOn;
38
+ global.spyOnProd = noop;
39
+ global.spyOnDevAndProd = spyOn;
40
+ }
41
42
['error', 'warn'].forEach(methodName => {
43
var oldMethod = console[methodName];
@@ -33,19 +55,17 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) {
55
env.afterEach(() => {
56
if (console[methodName] !== newMethod && !isSpy(console[methodName])) {
57
throw new Error(
36
- 'Test did not tear down console.' + methodName + ' mock properly.'
58
+ `Test did not tear down console.${methodName} mock properly.`
59
);
60
}
61
if (console[methodName].__callCount !== 0) {
62
throw new Error(
41
- 'Expected test not to call console.' +
42
- methodName +
43
- '(). ' +
63
+ `Expected test not to call console.${methodName}(). ` +
64
'If the warning is expected, mock it out using ' +
45
- "spyOn(console, '" +
46
- methodName +
47
- "') and test that the " +
48
- 'warning occurs.'
65
+ `spyOnDev(console, '${methodName}') or spyOnProd(console, '${
66
+ methodName
67
+ }'), ` +
68
+ 'and test that the warning occurs.'
69
);
70
}
71
});
scripts/jest/spec-equivalence-reporter/setupTests.js
+23
@@ -16,12 +16,35 @@ global.expect = function() {
16
return expect.apply(this, arguments);
17
};
18
19
+const spyOn = global.spyOn;
20
+
21
+// Spying on console methods in production builds can mask errors.
22
+// This is why we added an explicit spyOnDev() helper.
23
+// It's too easy to accidentally use the more familiar spyOn() helper though,
24
+// So we disable it entirely.
25
+// Spying on both dev and prod will require using both spyOnDev() and spyOnProd().
26
+global.spyOn = function() {
27
+ throw new Error(
28
+ 'Do not use spyOn(). ' +
29
+ 'It can accidentally hide unexpected errors in production builds. ' +
30
+ 'Use spyOnDev(), spyOnProd(), or spyOnDevAndProd() instead.'
31
+ );
32
+};
33
+
34
global.spyOnDev = function(...args) {
35
if (__DEV__) {
36
return spyOn(...args);
37
}
38
};
39
40
+global.spyOnDevAndProd = spyOn;
41
+
42
+global.spyOnProd = function(...args) {
43
+ if (!__DEV__) {
44
+ return spyOn(...args);
45
+ }
46
+};
47
+
48
beforeEach(() => (numExpectations = 0));
49
50
jasmine.currentEnv_.addReporter({
scripts/jest/typescript/jest.d.ts
+2
-1
@@ -11,8 +11,9 @@ declare var it: {
11
declare function expect(val: any): Expect;
12
declare var jest: Jest;
13
declare function pit(name: string, fn: any): void;
14
-declare function spyOn(obj: any, key: string): any;
14
declare function spyOnDev(obj: any, key: string): any;
15
+declare function spyOnDevAndProd(obj: any, key: string): any;
16
+declare function spyOnProd(obj: any, key: string): any;
17
declare function xdescribe(name: string, fn: any): void;
18
declare function xit(name: string, fn: any): void;
19