refactor: Switch to Object.is for hook args (#4663)

* Remove unused imports

* refactor: Switch to Object.is for hook args

* refactor: Copy to `useReducer` & store `Object` accessor

* test: Add tests for `useEffect` & `useState` w/ `NaN`

Co-authored-by: jayrobin <[email protected]>

---------

Co-authored-by: jdecroock <[email protected]>
Co-authored-by: jayrobin <[email protected]>

Author: 3 people

hooks/src/index.js

@@ -1,5 +1,7 @@
 import { options as _options } from 'preact';
 
+const ObjectIs = Object.is;
+
 /** @type {number} */
 let currentIndex;
 
@@ -193,7 +195,7 @@ export function useReducer(reducer, initialState, init) {
 					: hookState._value[0];
 				const nextValue = hookState._reducer(currentValue, action);
 
-				if (currentValue !== nextValue) {
+				if (!ObjectIs(currentValue, nextValue)) {
 					hookState._nextValue = [nextValue, hookState._value[1]];
 					hookState._component.setState({});
 				}
@@ -258,7 +260,8 @@ export function useReducer(reducer, initialState, init) {
 						const currentValue = hookItem._value[0];
 						hookItem._value = hookItem._nextValue;
 						hookItem._nextValue = undefined;
-						if (currentValue !== hookItem._value[0]) shouldUpdate = true;
+						if (!ObjectIs(currentValue, hookItem._value[0]))
+							shouldUpdate = true;
 					}
 				});
 
@@ -540,7 +543,7 @@ function argsChanged(oldArgs, newArgs) {
 	return (
 		!oldArgs ||
 		oldArgs.length !== newArgs.length ||
-		newArgs.some((arg, index) => arg !== oldArgs[index])
+		newArgs.some((arg, index) => !ObjectIs(arg, oldArgs[index]))
 	);
 }
 

hooks/test/browser/useEffect.test.js

@@ -636,4 +636,26 @@ describe('useEffect', () => {
 		expect(calls.length).to.equal(1);
 		expect(calls).to.deep.equal(['doing effecthi']);
 	});
+
+	it('should not rerun when receiving NaN on subsequent renders', () => {
+		const calls = [];
+		const Component = ({ value }) => {
+			const [count, setCount] = useState(0);
+			useEffect(() => {
+				calls.push('doing effect' + count);
+				setCount(count + 1);
+				return () => {
+					calls.push('cleaning up' + count);
+				};
+			}, [value]);
+			return <p>{count}</p>;
+		};
+		const App = () => <Component value={NaN} />;
+
+		act(() => {
+			render(<App />, scratch);
+		});
+		expect(calls.length).to.equal(1);
+		expect(calls).to.deep.equal(['doing effect0']);
+	});
 });

hooks/test/browser/useState.test.js

@@ -373,6 +373,32 @@ describe('useState', () => {
 		expect(scratch.innerHTML).to.equal('<p>hello world!!!</p>');
 	});
 
+	it('should limit rerenders when setting state to NaN', () => {
+		const calls = [];
+		const App = ({ i }) => {
+			calls.push('rendering' + i);
+			const [greeting, setGreeting] = useState(0);
+
+			if (i === 2) {
+				setGreeting(NaN);
+			}
+
+			return <p>{greeting}</p>;
+		};
+
+		act(() => {
+			render(<App i={1} />, scratch);
+		});
+		expect(calls.length).to.equal(1);
+		expect(calls).to.deep.equal(['rendering1']);
+
+		act(() => {
+			render(<App i={2} />, scratch);
+		});
+		expect(calls.length).to.equal(3);
+		expect(calls.slice(1).every(c => c === 'rendering2')).to.equal(true);
+	});
+
 	describe('Global sCU', () => {
 		let prevScu;
 		beforeAll(() => {