Merge pull request #335 from preactjs/react-adapter-update

Revert react integration to tracking current dispatcher

Author: andrewiggins

.changeset/angry-papayas-accept.md

@@ -0,0 +1,5 @@
+---
+"@preact/signals-react": minor
+---
+
+Revert react integration to tracking current dispatcher

package.json

@@ -30,10 +30,10 @@
 	"license": "MIT",
 	"devDependencies": {
 		"@babel/core": "^7.19.1",
+		"@babel/plugin-transform-typescript": "^7.19.1",
 		"@babel/preset-env": "^7.19.1",
 		"@babel/preset-react": "^7.18.6",
 		"@babel/preset-typescript": "^7.18.6",
-		"@babel/plugin-transform-typescript": "^7.19.1",
 		"@changesets/changelog-github": "^0.4.6",
 		"@changesets/cli": "^2.24.2",
 		"@types/chai": "^4.3.3",

packages/react/package.json

@@ -49,6 +49,7 @@
 		"@types/react-dom": "^18.0.6",
 		"@types/use-sync-external-store": "^0.0.3",
 		"react": "^18.2.0",
-		"react-dom": "^18.2.0"
+		"react-dom": "^18.2.0",
+		"react-router-dom": "^6.9.0"
 	}
 }

packages/react/src/index.ts

@@ -2,14 +2,16 @@ import {
 	useRef,
 	useMemo,
 	useEffect,
-	Component,
-	type FunctionComponent,
+	// @ts-ignore-next-line
+	// eslint-disable-next-line @typescript-eslint/no-unused-vars
+	__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED as ReactInternals,
 	type ReactElement,
+	type useCallback,
+	type useReducer,
 } 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/index.js";
 import {
 	signal,
 	computed,
@@ -18,96 +20,34 @@ import {
 	Signal,
 	type ReadonlySignal,
 } from "@preact/signals-core";
+import { useSyncExternalStore } from "use-sync-external-store/shim/index";
 import type { Effect, JsxRuntimeModule } from "./internal";
 
 export { signal, computed, batch, effect, Signal, type ReadonlySignal };
 
 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 ReactForwardRefType = Symbol.for("react.forward_ref"); // https://github.com/facebook/react/blob/346c7d4c43a0717302d446da9e7423a8e28d8996/packages/shared/ReactSymbols.js#L25
-const ProxyInstance = new WeakMap<
-	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<any>,
-		thisArg: any,
-		argumentsList: Parameters<FunctionComponent<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;
-			// eslint-disable-next-line no-useless-catch
-		} 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();
-		}
-	},
-};
 
-function ProxyFunctionalComponent(Component: FunctionComponent<any>) {
-	return ProxyInstance.get(Component) || WrapWithProxy(Component);
+interface ReactDispatcher {
+	useRef: typeof useRef;
+	useCallback: typeof useCallback;
+	useReducer: typeof useReducer;
+	useSyncExternalStore: typeof useSyncExternalStore;
 }
 
-function WrapWithProxy(Component: FunctionComponent<any>) {
-	if (SupportsProxy) {
-		const ProxyComponent = new Proxy(Component, ProxyHandlers);
-
-		ProxyInstance.set(Component, ProxyComponent);
-		ProxyInstance.set(ProxyComponent, ProxyComponent);
-
-		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: FunctionComponent<any> = (...args) => {
-		return ProxyHandlers.apply(Component, undefined, args);
-	};
+let finishUpdate: (() => void) | undefined;
 
-	ProxyInstance.set(Component, WrappedComponent);
-	ProxyInstance.set(WrappedComponent, WrappedComponent);
+function setCurrentUpdater(updater?: Effect) {
+	// end tracking for the current update:
+	if (finishUpdate) finishUpdate();
+	// start tracking the new update:
+	finishUpdate = updater && updater._start();
+}
 
-	return WrappedComponent;
+interface EffectStore {
+	updater: Effect;
+	subscribe(onStoreChange: () => void): () => void;
+	getSnapshot(): number;
 }
 
 /**
@@ -123,7 +63,7 @@ function WrapWithProxy(Component: FunctionComponent<any>) {
  * @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() {
+function createEffectStore(): EffectStore {
 	let updater!: Effect;
 	let version = 0;
 	let onChangeNotifyReact: (() => void) | undefined;
@@ -138,7 +78,7 @@ function createEffectStore() {
 
 	return {
 		updater,
-		subscribe(onStoreChange: () => void) {
+		subscribe(onStoreChange) {
 			onChangeNotifyReact = onStoreChange;
 
 			return function () {
@@ -163,24 +103,144 @@ function createEffectStore() {
 	};
 }
 
-function WrapJsx<T>(jsx: T): T {
-	if (typeof jsx !== "function") return jsx;
+/**
+ * Custom hook to create the effect to track signals used during render and
+ * subscribe to changes to rerender the component when the signals change
+ */
+function usePreactSignalStore(nextDispatcher: ReactDispatcher): EffectStore {
+	const storeRef = nextDispatcher.useRef<EffectStore>();
+	if (storeRef.current == null) {
+		storeRef.current = createEffectStore();
+	}
 
-	return function (type: any, props: any, ...rest: any[]) {
-		if (typeof type === "function" && !(type instanceof Component)) {
-			return jsx.call(jsx, ProxyFunctionalComponent(type), props, ...rest);
+	const store = storeRef.current;
+	useSyncExternalStore(store.subscribe, store.getSnapshot, store.getSnapshot);
+
+	return store;
+}
+
+// To track when we are entering and exiting a component render (i.e. before and
+// after React renders a component), we track how the dispatcher changes.
+// Outside of a component rendering, the dispatcher is set to an instance that
+// errors or warns when any hooks are called. This behavior is prevents hooks
+// from being used outside of components. Right before React renders a
+// component, the dispatcher is set to a valid one. Right after React finishes
+// rendering a component, the dispatcher is set to an erroring one again. This
+// erroring dispatcher is called the `ContextOnlyDispatcher` in React's source.
+//
+// So, we watch the getter and setter on `ReactCurrentDispatcher.current` to
+// monitor the changes to the current ReactDispatcher. When the dispatcher
+// changes from the ContextOnlyDispatcher to a valid dispatcher, we assume we
+// are entering a component render. At this point, we setup our
+// auto-subscriptions for any signals used in the component. We do this by
+// creating an effect and manually starting the effect. We use
+// `useSyncExternalStore` to trigger rerenders on the component when any signals
+// it uses changes.
+//
+// When the dispatcher changes from a valid dispatcher back to the
+// ContextOnlyDispatcher, we assume we are exiting a component render. At this
+// point we stop the effect.
+//
+// Some edge cases to be aware of:
+// - In development, useReducer, useState, and useMemo changes the dispatcher to
+//   a different erroring dispatcher before invoking the reducer and resets it
+//   right after.
+//
+//   The useSyncExternalStore shim will use some of these hooks when we invoke
+//   it while entering a component render. We need to prevent this dispatcher
+//   change caused by these hooks from re-triggering our entering logic (it
+//   would cause an infinite loop if we did not). We do this by using a lock to
+//   prevent the setter from running while we are in the setter.
+//
+//   When a Component's function body invokes useReducer, useState, or useMemo,
+//   this change in dispatcher should not signal that we are exiting a component
+//   render. We ignore this change by detecting these dispatchers as different
+//   from ContextOnlyDispatcher and other valid dispatchers.
+//
+// - The `use` hook will change the dispatcher to from a valid update dispatcher
+//   to a valid mount dispatcher in some cases. Similarly to useReducer
+//   mentioned above, we should not signal that we are exiting a component
+//   during this change. Because these other valid dispatchers do not pass the
+//   ContextOnlyDispatcher check, they do not affect our logic.
+let lock = false;
+let currentDispatcher: ReactDispatcher | null = null;
+Object.defineProperty(ReactInternals.ReactCurrentDispatcher, "current", {
+	get() {
+		return currentDispatcher;
+	},
+	set(nextDispatcher: ReactDispatcher) {
+		if (lock) {
+			currentDispatcher = nextDispatcher;
+			return;
 		}
 
-		if (type && typeof type === "object") {
-			if (type.$$typeof === ReactMemoType) {
-				type.type = ProxyFunctionalComponent(type.type);
-				return jsx.call(jsx, type, props, ...rest);
-			} else if (type.$$typeof === ReactForwardRefType) {
-				type.render = ProxyFunctionalComponent(type.render);
-				return jsx.call(jsx, type, props, ...rest);
-			}
+		const currentDispatcherType = getDispatcherType(currentDispatcher);
+		const nextDispatcherType = getDispatcherType(nextDispatcher);
+
+		// We are entering a component render if the current dispatcher is the
+		// ContextOnlyDispatcher and the next dispatcher is a valid dispatcher.
+		const isEnteringComponentRender =
+			currentDispatcherType === ContextOnlyDispatcherType &&
+			nextDispatcherType === ValidDispatcherType;
+
+		// We are exiting a component render if the current dispatcher is a valid
+		// dispatcher and the next dispatcher is the ContextOnlyDispatcher.
+		const isExitingComponentRender =
+			currentDispatcherType === ValidDispatcherType &&
+			nextDispatcherType === ContextOnlyDispatcherType;
+
+		// Update the current dispatcher now so the hooks inside of the
+		// useSyncExternalStore shim get the right dispatcher.
+		currentDispatcher = nextDispatcher;
+		if (isEnteringComponentRender) {
+			lock = true;
+			const store = usePreactSignalStore(nextDispatcher);
+			lock = false;
+
+			setCurrentUpdater(store.updater);
+		} else if (isExitingComponentRender) {
+			setCurrentUpdater();
 		}
+	},
+});
+
+const ValidDispatcherType = 0;
+const ContextOnlyDispatcherType = 1;
+const ErroringDispatcherType = 2;
+
+// We inject a useSyncExternalStore into every function component via
+// CurrentDispatcher. This prevents injecting into anything other than a
+// function component render.
+const dispatcherTypeCache = new Map<ReactDispatcher, number>();
+function getDispatcherType(dispatcher: ReactDispatcher | null): number {
+	// Treat null the same as the ContextOnlyDispatcher.
+	if (!dispatcher) return ContextOnlyDispatcherType;
+
+	const cached = dispatcherTypeCache.get(dispatcher);
+	if (cached !== undefined) return cached;
+
+	// The ContextOnlyDispatcher sets all the hook implementations to a function
+	// that takes no arguments and throws and error. Check the number of arguments
+	// for this dispatcher's useCallback implementation to determine if it is a
+	// ContextOnlyDispatcher. All other dispatchers, erroring or not, define
+	// functions with arguments and so fail this check.
+	let type: number;
+	if (dispatcher.useCallback.length < 2) {
+		type = ContextOnlyDispatcherType;
+	} else if (/Invalid/.test(dispatcher.useCallback as any)) {
+		type = ErroringDispatcherType;
+	} else {
+		type = ValidDispatcherType;
+	}
+
+	dispatcherTypeCache.set(dispatcher, type);
+	return type;
+}
 
+function WrapJsx<T>(jsx: T): T {
+	if (typeof jsx !== "function") return jsx;
+
+	return function (type: any, props: any, ...rest: any[]) {
 		if (typeof type === "string" && props) {
 			for (let i in props) {
 				let v = props[i];
@@ -228,7 +288,7 @@ function Text({ data }: { data: Signal }) {
 // Decorate Signals so React renders them as <Text> components.
 Object.defineProperties(Signal.prototype, {
 	$$typeof: { configurable: true, value: ReactElemType },
-	type: { configurable: true, value: ProxyFunctionalComponent(Text) },
+	type: { configurable: true, value: Text },
 	props: {
 		configurable: true,
 		get() {