diff --git a/src/component.js b/src/component.js index f3aca45f46..9943b0244c 100644 --- a/src/component.js +++ b/src/component.js @@ -48,7 +48,20 @@ BaseComponent.prototype.setState = function (update, callback) { if (update) { assign(s, update); - } else { + } else if (!callback) { + // nothing to apply and no callback to flush, so this is a true no-op + return; + } else if (this._vnode && !(this._bits & COMPONENT_DIRTY)) { + // The update itself was a no-op (e.g. an updater bailing out by + // returning null/undefined), and there's no render already queued to + // piggyback the callback on. This mirrors React: a no-op update + // doesn't schedule a render, but the callback still fires. Calling it + // directly here avoids enqueueing a render cycle just to flush it. + try { + callback.call(this); + } catch (e) { + options._catchError(e, this._vnode); + } return; } diff --git a/test/browser/components.test.jsx b/test/browser/components.test.jsx index 590537cc63..acf5758954 100644 --- a/test/browser/components.test.jsx +++ b/test/browser/components.test.jsx @@ -1985,6 +1985,155 @@ describe('Components', () => { rerender(); }).to.not.throw(); }); + + it('should still invoke the callback when the updater function bails out by returning null', () => { + let spy = vi.fn(); + let update; + class Foo extends Component { + constructor(props) { + super(props); + this.state = { count: 0 }; + update = () => this.setState(() => null, spy); + } + + render() { + return
{this.state.count}
; + } + } + + render(, scratch); + update(); + rerender(); + + expect(spy).toHaveBeenCalledOnce(); + }); + + it('should still invoke the callback when passing a falsy update directly', () => { + let spy = vi.fn(); + let update; + class Foo extends Component { + constructor(props) { + super(props); + this.state = { count: 0 }; + update = () => this.setState(null, spy); + } + + render() { + return
{this.state.count}
; + } + } + + render(, scratch); + update(); + rerender(); + + expect(spy).toHaveBeenCalledOnce(); + }); + + it('should not schedule a render when flushing a callback for a no-op update', () => { + let spy = vi.fn(); + let update; + class Foo extends Component { + constructor(props) { + super(props); + this.state = { count: 0 }; + update = () => this.setState(() => null, spy); + } + + render() { + return
{this.state.count}
; + } + } + + vi.spyOn(Foo.prototype, 'render'); + + render(, scratch); + expect(Foo.prototype.render).toHaveBeenCalledOnce(); + + update(); + // The callback should have fired immediately/synchronously, without + // waiting on the render queue, since there's nothing to commit. + expect(spy).toHaveBeenCalledOnce(); + // No render was enqueued to flush it, so draining the render queue + // must not cause an additional render call. + rerender(); + + expect(Foo.prototype.render).toHaveBeenCalledOnce(); + }); + + it('should invoke the callback exactly once for multiple stacked no-op updates', () => { + let spy = vi.fn(); + let update; + class Foo extends Component { + constructor(props) { + super(props); + this.state = { count: 0 }; + update = () => { + this.setState(() => null, spy); + this.setState(() => null, spy); + this.setState(() => null, spy); + }; + } + + render() { + return
{this.state.count}
; + } + } + + vi.spyOn(Foo.prototype, 'render'); + + render(, scratch); + expect(Foo.prototype.render).toHaveBeenCalledOnce(); + + update(); + + expect(spy).toHaveBeenCalledTimes(3); + rerender(); + + expect(Foo.prototype.render).toHaveBeenCalledOnce(); + }); + + it('should batch a no-op callback onto an already-pending render instead of firing it early', () => { + let calls = []; + let update; + class Foo extends Component { + constructor(props) { + super(props); + this.state = { count: 0 }; + update = () => { + // A real update enqueues a render for this component... + this.setState({ count: 1 }, () => calls.push('real')); + // ...so this no-op callback should be attached to that same + // pending render instead of being invoked synchronously. + this.setState( + () => null, + () => calls.push('noop') + ); + }; + } + + render() { + return
{this.state.count}
; + } + } + + vi.spyOn(Foo.prototype, 'render'); + + render(, scratch); + expect(Foo.prototype.render).toHaveBeenCalledOnce(); + + update(); + // Neither callback should have run yet: both are attached to the + // still-pending render triggered by the real update. + expect(calls).to.deep.equal([]); + + rerender(); + + // Exactly one additional render for the real update, and both + // callbacks fired, in the order they were registered. + expect(Foo.prototype.render).toHaveBeenCalledTimes(2); + expect(calls).to.deep.equal(['real', 'noop']); + }); }); describe('forceUpdate', () => {