Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion src/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
149 changes: 149 additions & 0 deletions test/browser/components.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 <div>{this.state.count}</div>;
}
}

render(<Foo />, 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 <div>{this.state.count}</div>;
}
}

render(<Foo />, 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 <div>{this.state.count}</div>;
}
}

vi.spyOn(Foo.prototype, 'render');

render(<Foo />, 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 <div>{this.state.count}</div>;
}
}

vi.spyOn(Foo.prototype, 'render');

render(<Foo />, 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 <div>{this.state.count}</div>;
}
}

vi.spyOn(Foo.prototype, 'render');

render(<Foo />, 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', () => {
Expand Down