Skip to content

Commit 02a87ed

Browse files
committed
Deduplicate warning on invalid callback (facebook#11833)
1 parent 7299238 commit 02a87ed

File tree

2 files changed

+56
-56
lines changed

2 files changed

+56
-56
lines changed

packages/react-dom/src/__tests__/ReactUpdates-test.js

Lines changed: 41 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -842,8 +842,6 @@ describe('ReactUpdates', () => {
842842
});
843843

844844
it('throws in setState if the update callback is not a function', () => {
845-
spyOnDev(console, 'error');
846-
847845
function Foo() {
848846
this.a = 1;
849847
this.b = 2;
@@ -859,44 +857,43 @@ describe('ReactUpdates', () => {
859857

860858
let component = ReactTestUtils.renderIntoDocument(<A />);
861859

862-
expect(() => component.setState({}, 'no')).toThrowError(
860+
expect(() => {
861+
expect(() => component.setState({}, 'no')).toWarnDev(
862+
'setState(...): Expected the last optional `callback` argument to be ' +
863+
'a function. Instead received: no.',
864+
);
865+
}).toThrowError(
863866
'Invalid argument passed as callback. Expected a function. Instead ' +
864867
'received: no',
865868
);
866-
if (__DEV__) {
867-
expect(console.error.calls.argsFor(0)[0]).toContain(
869+
component = ReactTestUtils.renderIntoDocument(<A />);
870+
expect(() => {
871+
expect(() => component.setState({}, {foo: 'bar'})).toWarnDev(
868872
'setState(...): Expected the last optional `callback` argument to be ' +
869-
'a function. Instead received: no.',
873+
'a function. Instead received: [object Object].',
870874
);
871-
}
872-
component = ReactTestUtils.renderIntoDocument(<A />);
873-
expect(() => component.setState({}, {foo: 'bar'})).toThrowError(
875+
}).toThrowError(
874876
'Invalid argument passed as callback. Expected a function. Instead ' +
875877
'received: [object Object]',
876878
);
877-
if (__DEV__) {
878-
expect(console.error.calls.argsFor(1)[0]).toContain(
879+
component = ReactTestUtils.renderIntoDocument(<A />);
880+
expect(() => {
881+
expect(() => component.setState({}, new Foo())).toWarnDev(
879882
'setState(...): Expected the last optional `callback` argument to be ' +
880883
'a function. Instead received: [object Object].',
881884
);
882-
}
885+
}).toThrowError(
886+
'Invalid argument passed as callback. Expected a function. Instead ' +
887+
'received: [object Object]',
888+
);
883889
component = ReactTestUtils.renderIntoDocument(<A />);
884-
expect(() => component.setState({}, new Foo())).toThrowError(
890+
expect(() => component.setState({}, {a: 1, b: 2})).toThrowError(
885891
'Invalid argument passed as callback. Expected a function. Instead ' +
886892
'received: [object Object]',
887893
);
888-
if (__DEV__) {
889-
expect(console.error.calls.argsFor(2)[0]).toContain(
890-
'setState(...): Expected the last optional `callback` argument to be ' +
891-
'a function. Instead received: [object Object].',
892-
);
893-
expect(console.error.calls.count()).toBe(3);
894-
}
895894
});
896895

897896
it('throws in forceUpdate if the update callback is not a function', () => {
898-
spyOnDev(console, 'error');
899-
900897
function Foo() {
901898
this.a = 1;
902899
this.b = 2;
@@ -912,39 +909,40 @@ describe('ReactUpdates', () => {
912909

913910
let component = ReactTestUtils.renderIntoDocument(<A />);
914911

915-
expect(() => component.forceUpdate('no')).toThrowError(
912+
expect(() => {
913+
expect(() => component.forceUpdate('no')).toWarnDev(
914+
'forceUpdate(...): Expected the last optional `callback` argument to be ' +
915+
'a function. Instead received: no.',
916+
);
917+
}).toThrowError(
916918
'Invalid argument passed as callback. Expected a function. Instead ' +
917919
'received: no',
918920
);
919-
if (__DEV__) {
920-
expect(console.error.calls.argsFor(0)[0]).toContain(
921+
component = ReactTestUtils.renderIntoDocument(<A />);
922+
expect(() => {
923+
expect(() => component.forceUpdate({foo: 'bar'})).toWarnDev(
921924
'forceUpdate(...): Expected the last optional `callback` argument to be ' +
922-
'a function. Instead received: no.',
925+
'a function. Instead received: [object Object].',
923926
);
924-
}
925-
component = ReactTestUtils.renderIntoDocument(<A />);
926-
expect(() => component.forceUpdate({foo: 'bar'})).toThrowError(
927+
}).toThrowError(
927928
'Invalid argument passed as callback. Expected a function. Instead ' +
928929
'received: [object Object]',
929930
);
930-
if (__DEV__) {
931-
expect(console.error.calls.argsFor(1)[0]).toContain(
931+
component = ReactTestUtils.renderIntoDocument(<A />);
932+
expect(() => {
933+
expect(() => component.forceUpdate(new Foo())).toWarnDev(
932934
'forceUpdate(...): Expected the last optional `callback` argument to be ' +
933935
'a function. Instead received: [object Object].',
934936
);
935-
}
937+
}).toThrowError(
938+
'Invalid argument passed as callback. Expected a function. Instead ' +
939+
'received: [object Object]',
940+
);
936941
component = ReactTestUtils.renderIntoDocument(<A />);
937-
expect(() => component.forceUpdate(new Foo())).toThrowError(
942+
expect(() => component.forceUpdate({a: 1, b: 2})).toThrowError(
938943
'Invalid argument passed as callback. Expected a function. Instead ' +
939944
'received: [object Object]',
940945
);
941-
if (__DEV__) {
942-
expect(console.error.calls.argsFor(2)[0]).toContain(
943-
'forceUpdate(...): Expected the last optional `callback` argument to be ' +
944-
'a function. Instead received: [object Object].',
945-
);
946-
expect(console.error.calls.count()).toBe(3);
947-
}
948946
});
949947

950948
it('does not update one component twice in a batch (#2410)', () => {
@@ -1228,8 +1226,6 @@ describe('ReactUpdates', () => {
12281226
);
12291227

12301228
it('uses correct base state for setState inside render phase', () => {
1231-
spyOnDev(console, 'error');
1232-
12331229
let ops = [];
12341230

12351231
class Foo extends React.Component {
@@ -1246,14 +1242,10 @@ describe('ReactUpdates', () => {
12461242
}
12471243

12481244
const container = document.createElement('div');
1249-
ReactDOM.render(<Foo />, container);
1245+
expect(() => ReactDOM.render(<Foo />, container)).toWarnDev(
1246+
'Cannot update during an existing state transition',
1247+
);
12501248
expect(ops).toEqual(['base: 0, memoized: 0', 'base: 1, memoized: 1']);
1251-
if (__DEV__) {
1252-
expect(console.error.calls.count()).toBe(1);
1253-
expect(console.error.calls.argsFor(0)[0]).toContain(
1254-
'Cannot update during an existing state transition',
1255-
);
1256-
}
12571249
});
12581250

12591251
it('does not re-render if state update is null', () => {

packages/react-reconciler/src/ReactFiberClassComponent.js

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,16 +44,24 @@ let didWarnAboutStateAssignmentForComponent;
4444
let warnOnInvalidCallback;
4545

4646
if (__DEV__) {
47+
const didWarnOnInvalidCallback = {};
4748
didWarnAboutStateAssignmentForComponent = {};
4849

4950
warnOnInvalidCallback = function(callback: mixed, callerName: string) {
50-
warning(
51-
callback === null || typeof callback === 'function',
52-
'%s(...): Expected the last optional `callback` argument to be a ' +
53-
'function. Instead received: %s.',
54-
callerName,
55-
callback,
56-
);
51+
if (callback === null || typeof callback === 'function') {
52+
return;
53+
}
54+
const key = `${callerName}_${JSON.stringify(callback)}`;
55+
if (!didWarnOnInvalidCallback[key]) {
56+
warning(
57+
false,
58+
'%s(...): Expected the last optional `callback` argument to be a ' +
59+
'function. Instead received: %s.',
60+
callerName,
61+
callback,
62+
);
63+
didWarnOnInvalidCallback[key] = true;
64+
}
5765
};
5866

5967
// This is so gross but it's at least non-critical and can be removed if

0 commit comments

Comments
 (0)