Backport x v2 (#3780)

JoviDeCroock · web-flow · commit cdfbc2eda502 · 2022-10-27T09:01:28.000+02:00
diff --git a/compat/src/index.d.ts b/compat/src/index.d.ts
@@ -134,10 +134,23 @@ declare namespace React {
 	): C;
 
 	export interface ForwardFn<P = {}, T = any> {
-		(props: P, ref: Ref<T>): preact.ComponentChild;
+		(props: P, ref: ForwardedRef<T>): preact.ComponentChild;
 		displayName?: string;
 	}
 
+	interface MutableRefObject<T> {
+		current: T;
+	}
+
+	export type ForwardedRef<T> =
+		| ((instance: T | null) => void)
+		| MutableRefObject<T | null>
+		| null;
+
+	export type PropsWithChildren<P = unknown> = P & {
+		children?: preact.ComponentChild | undefined;
+	};
+
 	export function forwardRef<R, P = {}>(
 		fn: ForwardFn<P, R>
 	): preact.FunctionalComponent<Omit<P, 'ref'> & { ref?: preact.Ref<R> }>;
diff --git a/compat/src/index.js b/compat/src/index.js
@@ -137,6 +137,16 @@ export function useTransition() {
 // styles/... before it attaches
 export const useInsertionEffect = useLayoutEffect;
 
+/**
+ * Check if two values are the same value
+ * @param {*} x
+ * @param {*} y
+ * @returns {boolean}
+ */
+function is(x, y) {
+	return (x === y && (x !== 0 || 1 / x === 1 / y)) || (x !== x && y !== y);
+}
+
 /**
  * This is taken from https://github.com/facebook/react/blob/main/packages/use-sync-external-store/src/useSyncExternalStoreShimClient.js#L84
  * on a high level this cuts out the warnings, ... and attempts a smaller implementation
@@ -152,18 +162,18 @@ export function useSyncExternalStore(subscribe, getSnapshot) {
 		_instance._value = value;
 		_instance._getSnapshot = getSnapshot;
 
-		if (_instance._value !== getSnapshot()) {
+		if (!is(_instance._value, getSnapshot())) {
 			forceUpdate({ _instance });
 		}
 	}, [subscribe, value, getSnapshot]);
 
 	useEffect(() => {
-		if (_instance._value !== _instance._getSnapshot()) {
+		if (!is(_instance._value, _instance._getSnapshot())) {
 			forceUpdate({ _instance });
 		}
 
 		return subscribe(() => {
-			if (_instance._value !== _instance._getSnapshot()) {
+			if (!is(_instance._value, _instance._getSnapshot())) {
 				forceUpdate({ _instance });
 			}
 		});
diff --git a/compat/test/browser/hooks.test.js b/compat/test/browser/hooks.test.js
@@ -130,6 +130,40 @@ describe('React-18-hooks', () => {
 			expect(scratch.innerHTML).to.equal('<p>hello new world</p>');
 		});
 
+		it('getSnapshot can return NaN without causing infinite loop', () => {
+			let flush;
+			const subscribe = sinon.spy(cb => {
+				flush = cb;
+				return () => {};
+			});
+			let called = false;
+			const getSnapshot = sinon.spy(() => {
+				if (called) {
+					return NaN;
+				}
+
+				return 1;
+			});
+
+			const App = () => {
+				const value = useSyncExternalStore(subscribe, getSnapshot);
+				return <p>{value}</p>;
+			};
+
+			act(() => {
+				render(<App />, scratch);
+			});
+			expect(scratch.innerHTML).to.equal('<p>1</p>');
+			expect(subscribe).to.be.calledOnce;
+			expect(getSnapshot).to.be.calledThrice;
+
+			called = true;
+			flush();
+			rerender();
+
+			expect(scratch.innerHTML).to.equal('<p>NaN</p>');
+		});
+
 		it('should not call function values on subscription', () => {
 			let flush;
 			const subscribe = sinon.spy(cb => {
diff --git a/debug/src/index.d.ts b/debug/src/index.d.ts
@@ -0,0 +1,4 @@
+/**
+ * Reset the history of which prop type warnings have been logged.
+ */
+export function resetPropWarnings(): void;
diff --git a/hooks/src/index.js b/hooks/src/index.js
@@ -216,7 +216,7 @@ export function useReducer(reducer, initialState, init) {
 					}
 				});
 
-				return shouldUpdate
+				return shouldUpdate || hookState._internal.props !== p
 					? prevScu
 						? prevScu.call(this, p, s, c)
 						: true
diff --git a/hooks/test/browser/useState.test.js b/hooks/test/browser/useState.test.js
@@ -346,4 +346,29 @@ describe('useState', () => {
 
 		expect(renderSpy).to.be.calledTwice;
 	});
+
+	// see preactjs/preact#3731
+	it('respects updates initiated from the parent', () => {
+		let setChild, setParent;
+		const Child = props => {
+			const [, setState] = useState(false);
+			setChild = setState;
+			return <p>{props.text}</p>;
+		};
+
+		const Parent = () => {
+			const [state, setState] = useState('hello world');
+			setParent = setState;
+			return <Child text={state} />;
+		};
+
+		render(<Parent />, scratch);
+		expect(scratch.innerHTML).to.equal('<p>hello world</p>');
+
+		setParent('hello world!!!');
+		setChild(true);
+		setChild(false);
+		rerender();
+		expect(scratch.innerHTML).to.equal('<p>hello world!!!</p>');
+	});
 });
diff --git a/mangle.json b/mangle.json
@@ -52,6 +52,7 @@
       "$_children": "__k",
       "$_pendingSuspensionCount": "__u",
       "$_childDidSuspend": "__c",
+      "$_stateCallbacks": "_sb",
       "$_onResolve": "__R",
       "$_suspended": "__e",
       "$_dom": "__e",
diff --git a/package.json b/package.json
@@ -44,6 +44,7 @@
 			"umd": "./compat/dist/compat.umd.js"
 		},
 		"./debug": {
+      "types": "./debug/src/index.d.ts",
 			"module": "./debug/dist/debug.mjs",
 			"import": "./debug/dist/debug.mjs",
 			"require": "./debug/dist/debug.js",
diff --git a/src/component.js b/src/component.js
@@ -53,7 +53,7 @@ Component.prototype.setState = function(update, callback) {
 
 	const internal = this._internal;
 	if (update != null && internal) {
-		if (callback) internal._commitCallbacks.push(callback.bind(this));
+		if (callback) internal._stateCallbacks.push(callback.bind(this));
 		internal.rerender(internal);
 	}
 };
diff --git a/src/diff/mount.js b/src/diff/mount.js
@@ -367,6 +367,10 @@ function mountComponent(internal, startDom) {
 		if (renderHook) renderHook(internal);
 		if (ENABLE_CLASSES && internal.flags & TYPE_CLASS) {
 			renderResult = c.render(c.props, c.state, c.context);
+			for (let i = 0; i < internal._stateCallbacks.length; i++) {
+				internal._commitCallbacks.push(internal._stateCallbacks[i]);
+			}
+			internal._stateCallbacks = [];
 			// note: disable repeat render invocation for class components
 			break;
 		} else {
diff --git a/src/diff/patch.js b/src/diff/patch.js
@@ -244,6 +244,10 @@ function patchComponent(internal, newVNode) {
 		c.props = newProps;
 		c.state = c._nextState;
 		internal.flags |= SKIP_CHILDREN;
+		for (let i = 0; i < internal._stateCallbacks.length; i++) {
+			internal._commitCallbacks.push(internal._stateCallbacks[i]);
+		}
+		internal._stateCallbacks = [];
 		return;
 	}
 
@@ -265,6 +269,10 @@ function patchComponent(internal, newVNode) {
 		if (renderHook) renderHook(internal);
 		if (ENABLE_CLASSES && internal.flags & TYPE_CLASS) {
 			renderResult = c.render(c.props, c.state, c.context);
+			for (let i = 0; i < internal._stateCallbacks.length; i++) {
+				internal._commitCallbacks.push(internal._stateCallbacks[i]);
+			}
+			internal._stateCallbacks = [];
 			// note: disable repeat render invocation for class components
 			break;
 		} else {
diff --git a/src/diff/props.js b/src/diff/props.js
@@ -94,10 +94,7 @@ export function setProperty(dom, name, value, oldValue, isSvg) {
 
 		if (typeof value === 'function') {
 			// never serialize functions as attribute values
-		} else if (
-			value != null &&
-			(value !== false || (name[0] === 'a' && name[1] === 'r'))
-		) {
+		} else if (value != null && (value !== false || name.indexOf('-') != -1)) {
 			dom.setAttribute(name, value);
 		} else {
 			dom.removeAttribute(name);
diff --git a/src/index.d.ts b/src/index.d.ts
@@ -39,7 +39,7 @@ export type Key = string | number | any;
 
 export type RefObject<T> = { current: T | null };
 export type RefCallback<T> = (instance: T | null) => void;
-export type Ref<T> = RefObject<T> | RefCallback<T>;
+export type Ref<T> = RefObject<T> | RefCallback<T> | null;
 
 export type ComponentChild =
 	| VNode<any>
diff --git a/src/internal.d.ts b/src/internal.d.ts
@@ -164,6 +164,7 @@ export interface Internal<P = {}> {
 	_depth: number | null;
 	/** Callbacks to invoke when this internal commits */
 	_commitCallbacks: Array<() => void>;
+	_stateCallbacks: Array<() => void>; // Only class components
 }
 
 export interface Component<P = {}, S = {}> extends preact.Component<P, S> {
diff --git a/src/jsx.d.ts b/src/jsx.d.ts
@@ -647,8 +647,8 @@ export namespace JSXInternal {
 		challenge?: string;
 		checked?: boolean;
 		cite?: string;
-		class?: string;
-		className?: string;
+		class?: string | undefined;
+		className?: string | undefined;
 		cols?: number;
 		colSpan?: number;
 		content?: string;
diff --git a/src/tree.js b/src/tree.js
@@ -89,6 +89,7 @@ export function createInternal(vnode, parentInternal) {
 		_prevRef: null,
 		data: flags & TYPE_COMPONENT ? {} : null,
 		_commitCallbacks: flags & TYPE_COMPONENT ? [] : null,
+		_stateCallbacks: flags & TYPE_COMPONENT ? [] : null,
 		rerender: enqueueRender,
 		flags,
 		_children: null,
diff --git a/test/browser/fragments.test.js b/test/browser/fragments.test.js
@@ -84,6 +84,57 @@ describe('Fragment', () => {
 		]);
 	});
 
+	it('should not remove keyed elements', () => {
+		let deleteItem = () => {};
+		const Element = ({ item, deleteItem }) => (
+			<Fragment>
+				<div>Item: {item}</div>
+				{''} {/* If you delete this, it works fine. */}
+			</Fragment>
+		);
+
+		class App extends Component {
+			constructor(props) {
+				super(props);
+				this.state = {
+					items: Array(10)
+						.fill()
+						.map((_, i) => i)
+				};
+			}
+
+			render(_props, state) {
+				deleteItem = () => {
+					this.setState({
+						items: this.state.items.filter(i => i !== this.state.items[2])
+					});
+				};
+
+				return state.items.map(item => (
+					<Element item={item} deleteItem={deleteItem} key={item} />
+				));
+			}
+		}
+
+		render(<App />, scratch);
+		expect(scratch.innerHTML).to.equal(
+			'<div>Item: 0</div> <div>Item: 1</div> <div>Item: 2</div> <div>Item: 3</div> <div>Item: 4</div> <div>Item: 5</div> <div>Item: 6</div> <div>Item: 7</div> <div>Item: 8</div> <div>Item: 9</div> '
+		);
+
+		clearLog();
+		deleteItem();
+		rerender();
+
+		expect(scratch.innerHTML).to.equal(
+			'<div>Item: 0</div> <div>Item: 1</div> <div>Item: 3</div> <div>Item: 4</div> <div>Item: 5</div> <div>Item: 6</div> <div>Item: 7</div> <div>Item: 8</div> <div>Item: 9</div> '
+		);
+		expectDomLogToBe([
+			'<div>Item: 2.remove()',
+			'#text.remove()',
+			'#text.remove()'
+		]);
+	});
+
 	it('should render multiple children via noop renderer', () => {
 		render(
 			<Fragment>
@@ -1502,6 +1553,63 @@ describe('Fragment', () => {
 		);
 	});
 
+	it('should swap nested fragments correctly', () => {
+		let swap;
+		class App extends Component {
+			constructor(props) {
+				super(props);
+				this.state = { first: true };
+			}
+
+			render() {
+				if (this.state.first) {
+					return (
+						<Fragment>
+							{
+								<Fragment>
+									<p>1. Original item first paragraph</p>
+								</Fragment>
+							}
+							<p>2. Original item second paragraph</p>
+							<button onClick={(swap = () => this.setState({ first: false }))}>
+								Click me
+							</button>
+						</Fragment>
+					);
+				}
+				return (
+					<Fragment>
+						<p>1. Second item first paragraph</p>
+						<Fragment>
+							<p>2. Second item second paragraph</p>
+							<div />
+						</Fragment>
+						<button onClick={(swap = () => this.setState({ first: true }))}>
+							Click me
+						</button>
+					</Fragment>
+				);
+			}
+		}
+
+		render(<App />, scratch);
+		expect(scratch.innerHTML).to.equal(
+			'<p>1. Original item first paragraph</p><p>2. Original item second paragraph</p><button>Click me</button>'
+		);
+
+		swap();
+		rerender();
+		expect(scratch.innerHTML).to.equal(
+			'<p>1. Second item first paragraph</p><p>2. Second item second paragraph</p><div></div><button>Click me</button>'
+		);
+
+		swap();
+		rerender();
+		expect(scratch.innerHTML).to.equal(
+			'<p>1. Original item first paragraph</p><p>2. Original item second paragraph</p><button>Click me</button>'
+		);
+	});
+
 	it('should preserve state with reordering in multiple levels with lots of Fragment siblings', () => {
 		// Also fails if the # of divs outside the Fragment equals or exceeds
 		// the # inside the Fragment for both conditions
diff --git a/test/browser/lifecycles/componentDidMount.test.js b/test/browser/lifecycles/componentDidMount.test.js
diff --git a/test/browser/lifecycles/componentDidUpdate.test.js b/test/browser/lifecycles/componentDidUpdate.test.js
diff --git a/test/browser/refs.test.js b/test/browser/refs.test.js
diff --git a/test/browser/render.test.js b/test/browser/render.test.js

-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +/**
 + * Reset the history of which prop type warnings have been logged.
 + */
 +export function resetPropWarnings(): void;
Original file line number	Diff line number	Diff line change
`@@ -216,7 +216,7 @@ export function useReducer(reducer, initialState, init) {`
`216`	`216`	`}`
`217`	`217`	`});`
`218`	`218`
`219`		`- return shouldUpdate`
	`219`	`+ return shouldUpdate \|\| hookState._internal.props !== p`
`220`	`220`	`? prevScu`
`221`	`221`	`? prevScu.call(this, p, s, c)`
`222`	`222`	`: true`
Original file line number	Diff line number	Diff line change
`@@ -53,7 +53,7 @@ Component.prototype.setState = function(update, callback) {`
`53`	`53`
`54`	`54`	`const internal = this._internal;`
`55`	`55`	`if (update != null && internal) {`
`56`		`- if (callback) internal._commitCallbacks.push(callback.bind(this));`
	`56`	`+ if (callback) internal._stateCallbacks.push(callback.bind(this));`
`57`	`57`	`internal.rerender(internal);`
`58`	`58`	`}`
`59`	`59`	`};`