Skip to content

Commit

Permalink
Avoid synchronously adding setState callbacks (#3743)
Browse files Browse the repository at this point in the history
* create test case for #3742

* solve issue

* types

* golf

* add mangle
  • Loading branch information
JoviDeCroock committed Sep 29, 2022
1 parent dfd45aa commit d30a0ed
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 1 deletion.
1 change: 1 addition & 0 deletions mangle.json
Expand Up @@ -44,6 +44,7 @@
"$_force": "__e",
"$_nextState": "__s",
"$_renderCallbacks": "__h",
"$_stateCallbacks": "_sb",
"$_vnode": "__v",
"$_children": "__k",
"$_pendingSuspensionCount": "__u",
Expand Down
4 changes: 3 additions & 1 deletion src/component.js
Expand Up @@ -47,7 +47,9 @@ Component.prototype.setState = function(update, callback) {
if (update == null) return;

if (this._vnode) {
if (callback) this._renderCallbacks.push(callback);
if (callback) {
this._stateCallbacks.push(callback);
}
enqueueRender(this);
}
};
Expand Down
6 changes: 6 additions & 0 deletions src/diff/index.js
Expand Up @@ -89,6 +89,7 @@ export function diff(
c._globalContext = globalContext;
isNew = c._dirty = true;
c._renderCallbacks = [];
c._stateCallbacks = [];
}

// Invoke getDerivedStateFromProps
Expand All @@ -109,6 +110,11 @@ export function diff(
oldProps = c.props;
oldState = c.state;

for (tmp = 0; tmp < c._stateCallbacks.length; tmp++) {
c._renderCallbacks.push(c._stateCallbacks[tmp]);
c._stateCallbacks = [];
}

// Invoke pre-render lifecycle methods
if (isNew) {
if (
Expand Down
1 change: 1 addition & 0 deletions src/internal.d.ts
Expand Up @@ -133,6 +133,7 @@ export interface Component<P = {}, S = {}> extends preact.Component<P, S> {
_dirty: boolean;
_force?: boolean;
_renderCallbacks: Array<() => void>; // Only class components
_stateCallbacks: Array<() => void>; // Only class components
_globalContext?: any;
_vnode?: VNode<P> | null;
_nextState?: S | null; // Only class components
Expand Down
50 changes: 50 additions & 0 deletions test/browser/lifecycles/componentDidUpdate.test.js
Expand Up @@ -381,5 +381,55 @@ describe('Lifecycle methods', () => {
expect(Inner.prototype.componentDidUpdate).to.have.been.called;
expect(outerChildText).to.equal(`Outer: ${newValue.toString()}`);
});

it('should not interfere with setState callbacks', () => {
let invocation;

class Child extends Component {
componentDidMount() {
this.props.setValue(10);
}
render() {
return <p>Hello world</p>;
}
}

class App extends Component {
constructor(props) {
super(props);
this.state = {
show: false,
count: null
};
}

componentDidMount() {
// eslint-disable-next-line
this.setState({ show: true });
}

componentDidUpdate() {}

render() {
if (this.state.show) {
return (
<Child
setValue={i =>
this.setState({ count: i }, () => {
invocation = this.state;
})
}
/>
);
}
return null;
}
}

render(<App />, scratch);

rerender();
expect(invocation.count).to.equal(10);
});
});
});

0 comments on commit d30a0ed

Please sign in to comment.