Merge pull request #219 from eddyw/use-sync-external-store

Author: marvinhagemeister

.changeset/brown-wombats-brush.md

@@ -0,0 +1,5 @@
+---
+"@preact/signals-react": minor
+---
+
+Replace useReducer with useSyncExternalStore

packages/react/package.json

@@ -38,15 +38,17 @@
 		"prepublishOnly": "cd ../.. && pnpm build:react"
 	},
 	"dependencies": {
-		"@preact/signals-core": "workspace:^1.2.1"
+		"@preact/signals-core": "workspace:^1.2.1",
+		"use-sync-external-store": "^1.2.0"
 	},
 	"peerDependencies": {
 		"react": "17.x || 18.x"
 	},
 	"devDependencies": {
-		"react": "^18.2.0",
-		"react-dom": "^18.2.0",
 		"@types/react": "^18.0.18",
-		"@types/react-dom": "^18.0.6"
+		"@types/react-dom": "^18.0.6",
+		"@types/use-sync-external-store": "^0.0.3",
+		"react": "^18.2.0",
+		"react-dom": "^18.2.0"
 	}
 }

packages/react/src/index.ts

@@ -2,11 +2,13 @@ import {
 	useRef,
 	useMemo,
 	useEffect,
-	// @ts-ignore-next-line
-	// eslint-disable-next-line @typescript-eslint/no-unused-vars
-	__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED as internals,
+	Component,
+	type FunctionComponent,
 } from "react";
 import React from "react";
+import jsxRuntime from "react/jsx-runtime";
+import jsxRuntimeDev from "react/jsx-dev-runtime";
+import { useSyncExternalStore } from "use-sync-external-store/shim";
 import {
 	signal,
 	computed,
@@ -15,64 +17,183 @@ import {
 	Signal,
 	type ReadonlySignal,
 } from "@preact/signals-core";
-import { Effect, ReactDispatcher } from "./internal";
+import type { Effect, JsxRuntimeModule } from "./internal";
 
 export { signal, computed, batch, effect, Signal, type ReadonlySignal };
 
-/**
- * Install a middleware into React.createElement to replace any Signals in props with their value.
- * @todo this likely needs to be duplicated for jsx()...
- */
-const createElement = React.createElement;
-// @ts-ignore-next-line
-React.createElement = function (type, props) {
-	if (typeof type === "string" && props) {
-		for (let i in props) {
-			let v = props[i];
-			if (i !== "children" && v instanceof Signal) {
-				// createPropUpdater(props, i, v);
-				props[i] = v.value;
-			}
+const Empty = [] as const;
+const ReactElemType = Symbol.for("react.element"); // https://github.com/facebook/react/blob/346c7d4c43a0717302d446da9e7423a8e28d8996/packages/shared/ReactSymbols.js#L15
+const ReactMemoType = Symbol.for("react.memo"); // https://github.com/facebook/react/blob/346c7d4c43a0717302d446da9e7423a8e28d8996/packages/shared/ReactSymbols.js#L30
+const ProxyInstance = new Map<FunctionComponent<any>, FunctionComponent<any>>();
+const SupportsProxy = typeof Proxy === "function";
+
+const ProxyHandlers = {
+	/**
+	 * This is a function call trap for functional components.
+	 * When this is called, we know it means React did run 'Component()',
+	 * that means we can use any hooks here to setup our effect and store.
+	 *
+	 * With the native Proxy, all other calls such as access/setting to/of properties will
+	 * be forwarded to the target Component, so we don't need to copy the Component's
+	 * own or inherited properties.
+	 *
+	 * @see https://github.com/facebook/react/blob/2d80a0cd690bb5650b6c8a6c079a87b5dc42bd15/packages/react-reconciler/src/ReactFiberHooks.old.js#L460
+	 */
+	apply(Component: FunctionComponent, thisArg: any, argumentsList: any) {
+		const store = useMemo(createEffectStore, Empty);
+
+		useSyncExternalStore(store.subscribe, store.getSnapshot, store.getSnapshot);
+
+		const stop = store.updater._start();
+
+		try {
+			const children = Component.apply(thisArg, argumentsList);
+			return children;
+		} catch (e) {
+			// Re-throwing promises that'll be handled by suspense
+			// or an actual error.
+			throw e;
+		} finally {
+			// Stop effects in either case before return or throw,
+			// Otherwise the effect will leak.
+			stop();
 		}
-	}
-	// @ts-ignore-next-line
-	return createElement.apply(this, arguments);
+	},
 };
 
-/*
-// This breaks React's controlled components implementation
-function createPropUpdater(props: any, prop: string, signal: Signal) {
-	let ref = props.ref;
-	if (!ref) ref = props.ref = React.createRef();
-	effect(() => {
-		if (props) props[prop] = signal.value;
-		let el = ref.current;
-		if (!el) return; // unsubscribe
-		(el as any)[prop] = signal.value;
-	});
-	props = null;
+function ProxyFunctionalComponent(Component: FunctionComponent<any>) {
+	return ProxyInstance.get(Component) || WrapWithProxy(Component);
 }
-*/
+function WrapWithProxy(Component: FunctionComponent<any>) {
+	if (SupportsProxy) {
+		const ProxyComponent = new Proxy(Component, ProxyHandlers);
 
-let finishUpdate: (() => void) | undefined;
-const updaterForComponent = new WeakMap<() => void, Effect>();
+		ProxyInstance.set(Component, ProxyComponent);
+		ProxyInstance.set(ProxyComponent, ProxyComponent);
 
-function setCurrentUpdater(updater?: Effect) {
-	// end tracking for the current update:
-	if (finishUpdate) finishUpdate();
-	// start tracking the new update:
-	finishUpdate = updater && updater._start();
+		return ProxyComponent;
+	}
+
+	/**
+	 * Emulate a Proxy if environment doesn't support it.
+	 *
+	 * @TODO - unlike Proxy, it's not possible to access the type/Component's
+	 * static properties this way. Not sure if we want to copy all statics here.
+	 * Omitting this for now.
+	 *
+	 * @example - works with Proxy, doesn't with wrapped function.
+	 * ```
+	 * const el = <SomeFunctionalComponent />
+	 * el.type.someOwnOrInheritedProperty;
+	 * el.type.defaultProps;
+	 * ```
+	 */
+	const WrappedComponent = function () {
+		return ProxyHandlers.apply(Component, undefined, arguments);
+	};
+	ProxyInstance.set(Component, WrappedComponent);
+	ProxyInstance.set(WrappedComponent, WrappedComponent);
+
+	return WrappedComponent;
 }
 
-function createUpdater(update: () => void) {
+/**
+ * A redux-like store whose store value is a positive 32bit integer (a 'version').
+ *
+ * React subscribes to this store and gets a snapshot of the current 'version',
+ * whenever the 'version' changes, we tell React it's time to update the component (call 'onStoreChange').
+ *
+ * How we achieve this is by creating a binding with an 'effect', when the `effect._callback' is called,
+ * we update our store version and tell React to re-render the component ([1] We don't really care when/how React does it).
+ *
+ * [1]
+ * @see https://reactjs.org/docs/hooks-reference.html#usesyncexternalstore
+ * @see https://github.com/reactjs/rfcs/blob/main/text/0214-use-sync-external-store.md
+ */
+function createEffectStore() {
 	let updater!: Effect;
-	effect(function (this: Effect) {
+	let version = 0;
+	let onChangeNotifyReact: (() => void) | undefined;
+
+	let unsubscribe = effect(function (this: Effect) {
 		updater = this;
 	});
-	updater._callback = update;
-	return updater;
+	updater._callback = function () {
+		version = (version + 1) | 0;
+		if (onChangeNotifyReact) onChangeNotifyReact();
+	};
+
+	return {
+		updater,
+		subscribe(onStoreChange: () => void) {
+			onChangeNotifyReact = onStoreChange;
+
+			return function () {
+				/**
+				 * Rotate to next version when unsubscribing to ensure that components are re-run
+				 * when subscribing again.
+				 *
+				 * In StrictMode, 'memo'-ed components seem to keep a stale snapshot version, so
+				 * don't re-run after subscribing again if the version is the same as last time.
+				 *
+				 * Because we unsubscribe from the effect, the version may not change. We simply
+				 * set a new initial version in case of stale snapshots here.
+				 */
+				version = (version + 1) | 0;
+				onChangeNotifyReact = undefined;
+				unsubscribe();
+			};
+		},
+		getSnapshot() {
+			return version;
+		},
+	};
+}
+
+function WrapJsx<T>(jsx: T): T {
+	if (typeof jsx !== "function") return jsx;
+
+	return function (type: any, props: any, ...rest: any[]) {
+		if (typeof type === "function" && !(type instanceof Component)) {
+			return jsx.call(jsx, ProxyFunctionalComponent(type), props, ...rest);
+		}
+
+		if (type && typeof type === "object" && type.$$typeof === ReactMemoType) {
+			type.type = ProxyFunctionalComponent(type.type);
+			return jsx.call(jsx, type, props, ...rest);
+		}
+
+		if (typeof type === "string" && props) {
+			for (let i in props) {
+				let v = props[i];
+				if (i !== "children" && v instanceof Signal) {
+					props[i] = v.value;
+				}
+			}
+		}
+
+		return jsx.call(jsx, type, props, ...rest);
+	} as any as T;
 }
 
+const JsxPro: JsxRuntimeModule = jsxRuntime;
+const JsxDev: JsxRuntimeModule = jsxRuntimeDev;
+
+/**
+ * createElement _may_ be called by jsx runtime as a fallback in certain cases,
+ * so we need to wrap it regardless.
+ *
+ * The jsx exports depend on the `NODE_ENV` var to ensure the users' bundler doesn't
+ * include both, so one of them will be set with `undefined` values.
+ */
+React.createElement = WrapJsx(React.createElement);
+JsxDev.jsx && /*   */ (JsxDev.jsx = WrapJsx(JsxDev.jsx));
+JsxPro.jsx && /*   */ (JsxPro.jsx = WrapJsx(JsxPro.jsx));
+JsxDev.jsxs && /*  */ (JsxDev.jsxs = WrapJsx(JsxDev.jsxs));
+JsxPro.jsxs && /*  */ (JsxPro.jsxs = WrapJsx(JsxPro.jsxs));
+JsxDev.jsxDEV && /**/ (JsxDev.jsxDEV = WrapJsx(JsxDev.jsxDEV));
+JsxPro.jsxDEV && /**/ (JsxPro.jsxDEV = WrapJsx(JsxPro.jsxDEV));
+
 /**
  * A wrapper component that renders a Signal's value directly as a Text node.
  */
@@ -81,11 +202,9 @@ function Text({ data }: { data: Signal }) {
 }
 
 // Decorate Signals so React renders them as <Text> components.
-//@ts-ignore-next-line
-const $$typeof = createElement("a").$$typeof;
 Object.defineProperties(Signal.prototype, {
-	$$typeof: { configurable: true, value: $$typeof },
-	type: { configurable: true, value: Text },
+	$$typeof: { configurable: true, value: ReactElemType },
+	type: { configurable: true, value: ProxyFunctionalComponent(Text) },
 	props: {
 		configurable: true,
 		get() {
@@ -95,59 +214,14 @@ Object.defineProperties(Signal.prototype, {
 	ref: { configurable: true, value: null },
 });
 
-// Track the current dispatcher (roughly equiv to current component impl)
-let lock = false;
-const UPDATE = () => ({});
-let currentDispatcher: ReactDispatcher;
-Object.defineProperty(internals.ReactCurrentDispatcher, "current", {
-	get() {
-		return currentDispatcher;
-	},
-	set(api) {
-		currentDispatcher = api;
-		if (lock) return;
-		if (api && !isInvalidHookAccessor(api)) {
-			// prevent re-injecting useReducer when the Dispatcher
-			// context changes to run the reducer callback:
-			lock = true;
-			const rerender = api.useReducer(UPDATE, {})[1];
-			lock = false;
-
-			let updater = updaterForComponent.get(rerender);
-			if (!updater) {
-				updater = createUpdater(rerender);
-				updaterForComponent.set(rerender, updater);
-			} else {
-				updater._callback = rerender;
-			}
-			setCurrentUpdater(updater);
-		} else {
-			setCurrentUpdater();
-		}
-	},
-});
-
-// We inject a useReducer into every function component via CurrentDispatcher.
-// This prevents injecting into anything other than a function component render.
-const invalidHookAccessors = new Map();
-function isInvalidHookAccessor(api: ReactDispatcher) {
-	const cached = invalidHookAccessors.get(api);
-	if (cached !== undefined) return cached;
-	// we only want the real implementation, not the warning ones
-	const invalid =
-		api.useCallback.length < 2 || /Invalid/.test(api.useCallback as any);
-	invalidHookAccessors.set(api, invalid);
-	return invalid;
-}
-
 export function useSignal<T>(value: T) {
-	return useMemo(() => signal<T>(value), []);
+	return useMemo(() => signal<T>(value), Empty);
 }
 
 export function useComputed<T>(compute: () => T) {
 	const $compute = useRef(compute);
 	$compute.current = compute;
-	return useMemo(() => computed<T>(() => $compute.current()), []);
+	return useMemo(() => computed<T>(() => $compute.current()), Empty);
 }
 
 export function useSignalEffect(cb: () => void | (() => void)) {
@@ -158,5 +232,5 @@ export function useSignalEffect(cb: () => void | (() => void)) {
 		return effect(() => {
 			return callback.current();
 		});
-	}, []);
+	}, Empty);
 }

packages/react/src/internal.d.ts

@@ -7,8 +7,10 @@ export interface Effect {
 	_dispose(): void;
 }
 
-export interface ReactDispatcher {
-	useCallback(): unknown;
-}
-
 export type Updater = Signal<unknown>;
+
+export interface JsxRuntimeModule {
+	jsx?(type: any, ...rest: any[]): unknown;
+	jsxs?(type: any, ...rest: any[]): unknown;
+	jsxDEV?(type: any, ...rest: any[]): unknown;
+}