Backports x (#3707)

* backport #3615

* backport #3633

* backport #3643

* backport #3655

* backport #3663

* backport #3690

* backport useState optim

Author: JoviDeCroock

compat/src/index.js

@@ -137,19 +137,39 @@ export function useTransition() {
 // styles/... before it attaches
 export const useInsertionEffect = useLayoutEffect;
 
+/**
+ * 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
+ */
 export function useSyncExternalStore(subscribe, getSnapshot) {
-	const [state, setState] = useState(getSnapshot);
+	const value = getSnapshot();
+
+	const [{ _instance }, forceUpdate] = useState({
+		_instance: { _value: value, _getSnapshot: getSnapshot }
+	});
 
-	// TODO: in suspense for data we could have a discrepancy here because Preact won't re-init the "useState"
-	// when this unsuspends which could lead to stale state as the subscription is torn down.
+	useLayoutEffect(() => {
+		_instance._value = value;
+		_instance._getSnapshot = getSnapshot;
+
+		if (_instance._value !== getSnapshot()) {
+			forceUpdate({ _instance });
+		}
+	}, [subscribe, value, getSnapshot]);
 
 	useEffect(() => {
+		if (_instance._value !== _instance._getSnapshot()) {
+			forceUpdate({ _instance });
+		}
+
 		return subscribe(() => {
-			setState(getSnapshot());
+			if (_instance._value !== _instance._getSnapshot()) {
+				forceUpdate({ _instance });
+			}
 		});
-	}, [subscribe, getSnapshot]);
+	}, [subscribe]);
 
-	return state;
+	return value;
 }
 
 export * from 'preact/hooks';

compat/src/render.js

@@ -10,7 +10,7 @@ import { IS_NON_DIMENSIONAL } from './util';
 
 export const REACT_ELEMENT_TYPE = Symbol.for('react.element');
 
-const CAMEL_PROPS = /^(?:accent|alignment|arabic|baseline|cap|clip(?!PathU)|color|dominant|fill|flood|font|glyph(?!R)|horiz|marker(?!H|W|U)|overline|paint|shape|stop|strikethrough|stroke|text(?!L)|underline|unicode|units|v|vector|vert|word|writing|x(?!C))[A-Z]/;
+const CAMEL_PROPS = /^(?:accent|alignment|arabic|baseline|cap|clip(?!PathU)|color|dominant|fill|flood|font|glyph(?!R)|horiz|image|letter|lighting|marker(?!H|W|U)|overline|paint|pointer|shape|stop|strikethrough|stroke|text(?!L)|transform|underline|unicode|units|v|vector|vert|word|writing|x(?!C))[A-Z]/;
 const IS_DOM = typeof document !== 'undefined';
 
 // type="file|checkbox|radio".
@@ -161,7 +161,7 @@ options.vnode = vnode => {
 			} else if (/^on(Ani|Tra|Tou|BeforeInp|Compo)/.test(i)) {
 				i = i.toLowerCase();
 			} else if (nonCustomElement && CAMEL_PROPS.test(i)) {
-				i = i.replace(/[A-Z0-9]/, '-$&').toLowerCase();
+				i = i.replace(/[A-Z0-9]/g, '-$&').toLowerCase();
 			} else if (value === null) {
 				value = undefined;
 			}

compat/test/browser/hooks.test.js

@@ -4,7 +4,9 @@ import React, {
 	useInsertionEffect,
 	useSyncExternalStore,
 	useTransition,
-	render
+	render,
+	useState,
+	useCallback
 } from 'preact/compat';
 import { setupRerender, act } from 'preact/test-utils';
 import { setupScratch, teardown } from '../../../test/_util/helpers';
@@ -91,7 +93,7 @@ describe('React-18-hooks', () => {
 			});
 			expect(scratch.innerHTML).to.equal('<p>hello world</p>');
 			expect(subscribe).to.be.calledOnce;
-			expect(getSnapshot).to.be.calledOnce;
+			expect(getSnapshot).to.be.calledThrice;
 		});
 
 		it('subscribes and rerenders when called', () => {
@@ -119,13 +121,100 @@ describe('React-18-hooks', () => {
 			});
 			expect(scratch.innerHTML).to.equal('<p>hello world</p>');
 			expect(subscribe).to.be.calledOnce;
-			expect(getSnapshot).to.be.calledOnce;
+			expect(getSnapshot).to.be.calledThrice;
 
 			called = true;
 			flush();
 			rerender();
 
 			expect(scratch.innerHTML).to.equal('<p>hello new world</p>');
 		});
+
+		it('should not call function values on subscription', () => {
+			let flush;
+			const subscribe = sinon.spy(cb => {
+				flush = cb;
+				return () => {};
+			});
+
+			const func = () => 'value: ' + i++;
+
+			let i = 0;
+			const getSnapshot = sinon.spy(() => {
+				return func;
+			});
+
+			const App = () => {
+				const value = useSyncExternalStore(subscribe, getSnapshot);
+				return <p>{value()}</p>;
+			};
+
+			act(() => {
+				render(<App />, scratch);
+			});
+			expect(scratch.innerHTML).to.equal('<p>value: 0</p>');
+			expect(subscribe).to.be.calledOnce;
+			expect(getSnapshot).to.be.calledThrice;
+
+			flush();
+			rerender();
+
+			expect(scratch.innerHTML).to.equal('<p>value: 0</p>');
+		});
+
+		it('should work with changing getSnapshot', () => {
+			let flush;
+			const subscribe = sinon.spy(cb => {
+				flush = cb;
+				return () => {};
+			});
+
+			let i = 0;
+			const App = () => {
+				const value = useSyncExternalStore(subscribe, () => {
+					return i;
+				});
+				return <p>value: {value}</p>;
+			};
+
+			act(() => {
+				render(<App />, scratch);
+			});
+			expect(scratch.innerHTML).to.equal('<p>value: 0</p>');
+			expect(subscribe).to.be.calledOnce;
+
+			i++;
+			flush();
+			rerender();
+
+			expect(scratch.innerHTML).to.equal('<p>value: 1</p>');
+		});
+
+		it('works with useCallback', () => {
+			let toggle;
+			const App = () => {
+				const [state, setState] = useState(true);
+				toggle = setState.bind(this, () => false);
+
+				const value = useSyncExternalStore(
+					useCallback(() => {
+						return () => {};
+					}, [state]),
+					() => (state ? 'yep' : 'nope')
+				);
+
+				return <p>{value}</p>;
+			};
+
+			act(() => {
+				render(<App />, scratch);
+			});
+			expect(scratch.innerHTML).to.equal('<p>yep</p>');
+
+			toggle();
+			rerender();
+
+			expect(scratch.innerHTML).to.equal('<p>nope</p>');
+		});
 	});
 });

compat/test/browser/svg.test.js

@@ -73,13 +73,22 @@ describe('svg', () => {
 				clipPath="value"
 				clipRule="value"
 				clipPathUnits="value"
+				colorInterpolationFilters="auto"
+				fontSizeAdjust="value"
 				glyphOrientationHorizontal="value"
+				glyphOrientationVertical="value"
 				shapeRendering="crispEdges"
 				glyphRef="value"
+				horizAdvX="value"
+				horizOriginX="value"
 				markerStart="value"
 				markerHeight="value"
 				markerUnits="value"
 				markerWidth="value"
+				unitsPerEm="value"
+				vertAdvY="value"
+				vertOriginX="value"
+				vertOriginY="value"
 				x1="value"
 				xChannelSelector="value"
 			/>,
@@ -88,7 +97,7 @@ describe('svg', () => {
 
 		expect(serializeHtml(scratch)).to.eql(
 			sortAttributes(
-				'<svg clip-path="value" clip-rule="value" clipPathUnits="value" glyph-orientationhorizontal="value" shape-rendering="crispEdges" glyphRef="value" marker-start="value" markerHeight="value" markerUnits="value" markerWidth="value" x1="value" xChannelSelector="value"></svg>'
+				'<svg clip-path="value" clip-rule="value" clipPathUnits="value" color-interpolation-filters="auto" font-size-adjust="value" glyph-orientation-horizontal="value" glyph-orientation-vertical="value" shape-rendering="crispEdges" glyphRef="value" horiz-adv-x="value" horiz-origin-x="value" marker-start="value" markerHeight="value" markerUnits="value" markerWidth="value" units-per-em="value" vert-adv-y="value" vert-origin-x="value" vert-origin-y="value" x1="value" xChannelSelector="value"></svg>'
 			)
 		);
 	});

follow-ups.md

@@ -21,6 +21,7 @@ PR's that weren't backported yet, do they work?
 - https://github.com/preactjs/preact/pull/3280 Not merged yet need some input
 - https://github.com/preactjs/preact/pull/3222 Same as above
 - Make this work https://github.com/preactjs/preact/pull/3306
+- https://github.com/preactjs/preact/pull/3696
 
 ## Root node follow ups
 

hooks/src/index.js

@@ -14,7 +14,7 @@ let previousInternal;
 /** @type {number} */
 let currentHook = 0;
 
-/** @type {Array<import('./internal').Component>} */
+/** @type {Array<import('./internal').Internal>} */
 let afterPaintEffects = [];
 
 let EMPTY = [];
@@ -180,16 +180,49 @@ export function useReducer(reducer, initialState, init) {
 		];
 
 		hookState._internal = currentInternal;
-		currentInternal._component.shouldComponentUpdate = () => {
-			if (!hookState._nextValue) return true;
-
-			const currentValue = hookState._value[0];
-			hookState._value = hookState._nextValue;
-			hookState._nextValue = undefined;
-
-			return currentValue !== hookState._value[0];
-		};
+		if (!currentInternal.data._hasScuFromHooks) {
+			currentInternal.data._hasScuFromHooks = true;
+			const prevScu = currentInternal._component.shouldComponentUpdate;
+
+			// This SCU has the purpose of bailing out after repeated updates
+			// to stateful hooks.
+			// we store the next value in _nextValue[0] and keep doing that for all
+			// state setters, if we have next states and
+			// all next states within a component end up being equal to their original state
+			// we are safe to bail out for this specific component.
+			currentInternal._component.shouldComponentUpdate = function(p, s, c) {
+				if (!hookState._internal.data.__hooks) return true;
+
+				const stateHooks = hookState._internal.data.__hooks._list.filter(
+					x => x._internal
+				);
+				const allHooksEmpty = stateHooks.every(x => !x._nextValue);
+				// When we have no updated hooks in the component we invoke the previous SCU or
+				// traverse the VDOM tree further.
+				if (allHooksEmpty) {
+					return prevScu ? prevScu.call(this, p, s, c) : true;
+				}
 
+				// We check whether we have components with a nextValue set that
+				// have values that aren't equal to one another this pushes
+				// us to update further down the tree
+				let shouldUpdate = false;
+				stateHooks.forEach(hookItem => {
+					if (hookItem._nextValue) {
+						const currentValue = hookItem._value[0];
+						hookItem._value = hookItem._nextValue;
+						hookItem._nextValue = undefined;
+						if (currentValue !== hookItem._value[0]) shouldUpdate = true;
+					}
+				});
+
+				return shouldUpdate
+					? prevScu
+						? prevScu.call(this, p, s, c)
+						: true
+					: false;
+			};
+		}
 	}
 
 	return hookState._nextValue || hookState._value;
@@ -359,6 +392,7 @@ export function useErrorBoundary(cb) {
 function flushAfterPaintEffects() {
 	let internal;
 	while ((internal = afterPaintEffects.shift())) {
+		if (!internal.data.__hooks) continue;
 		if (~internal.flags & MODE_UNMOUNTING) {
 			try {
 				internal.data.__hooks._pendingEffects.forEach(invokeCleanup);