From 46ff9f1aaa7ca8f4c49fee1239050e8174321022 Mon Sep 17 00:00:00 2001 From: Alexander Kireev Date: Wed, 1 Jul 2026 05:08:05 +0700 Subject: [PATCH 1/2] fix: setState callback dropped when update is falsy If the update arg to setState is null/undefined (either passed directly, or returned from an updater function bailing out), the function returns early without ever queuing the callback or calling enqueueRender. Since callbacks only flush during a render pass, this silently drops the callback for good instead of just deferring it. Returning null/undefined from an updater is an explicitly supported pattern per the TS types, so setState(() => null, cb) should still run cb even though there's nothing to apply. --- src/component.js | 3 ++- test/browser/components.test.jsx | 44 ++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/component.js b/src/component.js index f3aca45f46..53a3bf1538 100644 --- a/src/component.js +++ b/src/component.js @@ -48,7 +48,8 @@ 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; } diff --git a/test/browser/components.test.jsx b/test/browser/components.test.jsx index 590537cc63..f1ddef6693 100644 --- a/test/browser/components.test.jsx +++ b/test/browser/components.test.jsx @@ -1985,6 +1985,50 @@ 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(); + }); }); describe('forceUpdate', () => { From 6171355acb77c3c357f53aeda02ff3065361d9a9 Mon Sep 17 00:00:00 2001 From: Alexander Kireev Date: Wed, 1 Jul 2026 14:23:55 +0700 Subject: [PATCH 2/2] address review: skip the redundant render when flushing a no-op callback setState(falsy, callback) was enqueueing a full render just to flush the callback, even when there was nothing to commit. Now it only enqueues a render when one isn't already pending for the component (so a callback tacked onto a real update still batches with it); otherwise it invokes the callback right away, matching React's behavior for a no-op update. Errors thrown from that callback go through the same error-boundary path commitRoot uses, so behavior stays consistent either way. Added coverage for: no render triggered for a lone no-op callback, multiple stacked no-op callbacks each firing once, and a no-op callback correctly batching onto an already-pending render instead of jumping the queue. --- src/component.js | 12 ++++ test/browser/components.test.jsx | 105 +++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+) diff --git a/src/component.js b/src/component.js index 53a3bf1538..9943b0244c 100644 --- a/src/component.js +++ b/src/component.js @@ -51,6 +51,18 @@ BaseComponent.prototype.setState = function (update, callback) { } 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; } if (this._vnode) { diff --git a/test/browser/components.test.jsx b/test/browser/components.test.jsx index f1ddef6693..acf5758954 100644 --- a/test/browser/components.test.jsx +++ b/test/browser/components.test.jsx @@ -2029,6 +2029,111 @@ describe('Components', () => { 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', () => {