Skip to content

Commit

Permalink
Deduplicate warning on invalid callback (#11833)
Browse files Browse the repository at this point in the history
  • Loading branch information
@yenshih
yenshih committed Jan 6, 2018
1 parent 7299238 commit 02a87ed
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 56 deletions.
90 changes: 41 additions & 49 deletions packages/react-dom/src/__tests__/ReactUpdates-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -842,8 +842,6 @@ describe('ReactUpdates', () => {
});

it('throws in setState if the update callback is not a function', () => {
spyOnDev(console, 'error');

function Foo() {
this.a = 1;
this.b = 2;
Expand All @@ -859,44 +857,43 @@ describe('ReactUpdates', () => {

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

expect(() => component.setState({}, 'no')).toThrowError(
expect(() => {
expect(() => component.setState({}, 'no')).toWarnDev(
'setState(...): Expected the last optional `callback` argument to be ' +
'a function. Instead received: no.',
);
}).toThrowError(
'Invalid argument passed as callback. Expected a function. Instead ' +
'received: no',
);
if (__DEV__) {
expect(console.error.calls.argsFor(0)[0]).toContain(
component = ReactTestUtils.renderIntoDocument(<A />);
expect(() => {
expect(() => component.setState({}, {foo: 'bar'})).toWarnDev(
'setState(...): Expected the last optional `callback` argument to be ' +
'a function. Instead received: no.',
'a function. Instead received: [object Object].',
);
}
component = ReactTestUtils.renderIntoDocument(<A />);
expect(() => component.setState({}, {foo: 'bar'})).toThrowError(
}).toThrowError(
'Invalid argument passed as callback. Expected a function. Instead ' +
'received: [object Object]',
);
if (__DEV__) {
expect(console.error.calls.argsFor(1)[0]).toContain(
component = ReactTestUtils.renderIntoDocument(<A />);
expect(() => {
expect(() => component.setState({}, new Foo())).toWarnDev(
'setState(...): Expected the last optional `callback` argument to be ' +
'a function. Instead received: [object Object].',
);
}
}).toThrowError(
'Invalid argument passed as callback. Expected a function. Instead ' +
'received: [object Object]',
);
component = ReactTestUtils.renderIntoDocument(<A />);
expect(() => component.setState({}, new Foo())).toThrowError(
expect(() => component.setState({}, {a: 1, b: 2})).toThrowError(
'Invalid argument passed as callback. Expected a function. Instead ' +
'received: [object Object]',
);
if (__DEV__) {
expect(console.error.calls.argsFor(2)[0]).toContain(
'setState(...): Expected the last optional `callback` argument to be ' +
'a function. Instead received: [object Object].',
);
expect(console.error.calls.count()).toBe(3);
}
});

it('throws in forceUpdate if the update callback is not a function', () => {
spyOnDev(console, 'error');

function Foo() {
this.a = 1;
this.b = 2;
Expand All @@ -912,39 +909,40 @@ describe('ReactUpdates', () => {

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

expect(() => component.forceUpdate('no')).toThrowError(
expect(() => {
expect(() => component.forceUpdate('no')).toWarnDev(
'forceUpdate(...): Expected the last optional `callback` argument to be ' +
'a function. Instead received: no.',
);
}).toThrowError(
'Invalid argument passed as callback. Expected a function. Instead ' +
'received: no',
);
if (__DEV__) {
expect(console.error.calls.argsFor(0)[0]).toContain(
component = ReactTestUtils.renderIntoDocument(<A />);
expect(() => {
expect(() => component.forceUpdate({foo: 'bar'})).toWarnDev(
'forceUpdate(...): Expected the last optional `callback` argument to be ' +
'a function. Instead received: no.',
'a function. Instead received: [object Object].',
);
}
component = ReactTestUtils.renderIntoDocument(<A />);
expect(() => component.forceUpdate({foo: 'bar'})).toThrowError(
}).toThrowError(
'Invalid argument passed as callback. Expected a function. Instead ' +
'received: [object Object]',
);
if (__DEV__) {
expect(console.error.calls.argsFor(1)[0]).toContain(
component = ReactTestUtils.renderIntoDocument(<A />);
expect(() => {
expect(() => component.forceUpdate(new Foo())).toWarnDev(
'forceUpdate(...): Expected the last optional `callback` argument to be ' +
'a function. Instead received: [object Object].',
);
}
}).toThrowError(
'Invalid argument passed as callback. Expected a function. Instead ' +
'received: [object Object]',
);
component = ReactTestUtils.renderIntoDocument(<A />);
expect(() => component.forceUpdate(new Foo())).toThrowError(
expect(() => component.forceUpdate({a: 1, b: 2})).toThrowError(
'Invalid argument passed as callback. Expected a function. Instead ' +
'received: [object Object]',
);
if (__DEV__) {
expect(console.error.calls.argsFor(2)[0]).toContain(
'forceUpdate(...): Expected the last optional `callback` argument to be ' +
'a function. Instead received: [object Object].',
);
expect(console.error.calls.count()).toBe(3);
}
});

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

it('uses correct base state for setState inside render phase', () => {
spyOnDev(console, 'error');

let ops = [];

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

const container = document.createElement('div');
ReactDOM.render(<Foo />, container);
expect(() => ReactDOM.render(<Foo />, container)).toWarnDev(
'Cannot update during an existing state transition',
);
expect(ops).toEqual(['base: 0, memoized: 0', 'base: 1, memoized: 1']);
if (__DEV__) {
expect(console.error.calls.count()).toBe(1);
expect(console.error.calls.argsFor(0)[0]).toContain(
'Cannot update during an existing state transition',
);
}
});

it('does not re-render if state update is null', () => {
Expand Down
22 changes: 15 additions & 7 deletions packages/react-reconciler/src/ReactFiberClassComponent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,24 @@ let didWarnAboutStateAssignmentForComponent;
let warnOnInvalidCallback;

if (__DEV__) {
const didWarnOnInvalidCallback = {};
didWarnAboutStateAssignmentForComponent = {};

warnOnInvalidCallback = function(callback: mixed, callerName: string) {
warning(
callback === null || typeof callback === 'function',
'%s(...): Expected the last optional `callback` argument to be a ' +
'function. Instead received: %s.',
callerName,
callback,
);
if (callback === null || typeof callback === 'function') {
return;
}
const key = `${callerName}_${JSON.stringify(callback)}`;
if (!didWarnOnInvalidCallback[key]) {
warning(
false,
'%s(...): Expected the last optional `callback` argument to be a ' +
'function. Instead received: %s.',
callerName,
callback,
);
didWarnOnInvalidCallback[key] = true;
}
};

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

0 comments on commit 02a87ed

Please sign in to comment.