Merge pull request #4048 from dmitrage/may-the-force-be-with-you

preactjs · Jun 19, 2023 · 523bbe9 · 523bbe9
2 parents d887539 + f1eed0e
commit 523bbe9
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 7 deletions.
diff --git a/src/diff/index.js b/src/diff/index.js
@@ -134,14 +134,14 @@ export function diff(
 				}
 
 				if (
-					(!c._force &&
-						c.shouldComponentUpdate != null &&
+					!c._force &&
+					((c.shouldComponentUpdate != null &&
 						c.shouldComponentUpdate(
 							newProps,
 							c._nextState,
 							componentContext
 						) === false) ||
-					newVNode._original === oldVNode._original
+						newVNode._original === oldVNode._original)
 				) {
 					// More info about this here: https://gist.github.com/JoviDeCroock/bec5f2ce93544d2e6070ef8e0036e4e8
 					if (newVNode._original !== oldVNode._original) {
@@ -154,8 +154,6 @@ export function diff(
 						c._dirty = false;
 					}
 
-					// In cases of bailing due to strict-equality we have to reset force as well
-					c._force = false;
 					newVNode._dom = oldVNode._dom;
 					newVNode._children = oldVNode._children;
 					newVNode._children.forEach(vnode => {
@@ -188,6 +186,7 @@ export function diff(
 			c.context = componentContext;
 			c.props = newProps;
 			c._parentDom = parentDom;
+			c._force = false;
 
 			let renderHook = options._render,
 				count = 0;
@@ -255,8 +254,6 @@ export function diff(
 			if (clearProcessingException) {
 				c._pendingError = c._processingException = null;
 			}
-
-			c._force = false;
 		} else if (
 			excessDomChildren == null &&
 			newVNode._original === oldVNode._original

diff --git a/test/browser/components.test.js b/test/browser/components.test.js
@@ -2660,5 +2660,58 @@ describe('Components', () => {
 
 			expect(scratch.innerHTML).to.equal('<div>bar</div>');
 		});
+
+		it('should skip shouldComponentUpdate when called during render', () => {
+			let isSCUCalled = false;
+			class App extends Component {
+				shouldComponentUpdate() {
+					isSCUCalled = true;
+					return false;
+				}
+				render() {
+					const isUpdated = this.isUpdated;
+					if (!isUpdated) {
+						this.isUpdated = true;
+						this.forceUpdate();
+					}
+					return <div>Updated: {isUpdated ? 'yes' : 'no'}</div>;
+				}
+			}
+			render(<App />, scratch);
+			rerender();
+			expect(isSCUCalled).to.be.false;
+			expect(scratch.innerHTML).to.equal('<div>Updated: yes</div>');
+		});
+
+		it('should break through strict equality optimization', () => {
+			let isSCUCalled = false;
+
+			class Child extends Component {
+				componentDidMount() {
+					this.props.parent.forceUpdate();
+					this.forceUpdate();
+					this.isUpdated = true;
+				}
+				shouldComponentUpdate() {
+					isSCUCalled = true;
+					return false;
+				}
+				render() {
+					return <div>Updated: {this.isUpdated ? 'yes' : 'no'}</div>;
+				}
+			}
+
+			class App extends Component {
+				children = (<Child parent={this} />);
+				render() {
+					return this.children;
+				}
+			}
+
+			render(<App />, scratch);
+			rerender();
+			expect(isSCUCalled).to.be.false;
+			expect(scratch.innerHTML).to.equal('<div>Updated: yes</div>');
+		});
 	});
 });