Revert computed changes in react/preact (#777)

* Revert "Add note about useComputed (#775)"

This reverts commit f32e357.

* Revert "preact/signals: Recompute useComputed on rerender (#754)"

This reverts commit 5db1295.

* Add changeset

Author: JoviDeCroock

.changeset/healthy-lamps-teach.md

@@ -0,0 +1,6 @@
+---
+"@preact/signals": minor
+"@preact/signals-react": minor
+---
+
+Revert the changes to `useComputed`, sincere apologies for the inconvenience we've discussed this at length and are going to side on the perf side.

packages/preact/README.md

@@ -72,35 +72,6 @@ function Counter() {
 }
 ```
 
-> **Optimizing `useComputed` performance**
->
-> The `useComputed` hook follows the convention of keeping closures fresh. By default, the callback function passed to `useComputed` will re-run on every component render to ensure it always has access to the latest values from the component's scope. However, if the computed value doesn't change, updates won't propagate to dependent nodes in the signal graph.
->
-> For expensive computations, you can optimize performance by memoizing the callback with `useCallback`:
->
-> ```js
-> import { useSignal, useComputed } from "@preact/signals";
-> import { useCallback } from "preact/hooks";
->
-> function Counter() {
-> 	const count = useSignal(0);
->
-> 	// Memoize the callback to avoid re-running expensive calculations
-> 	const expensiveComputation = useCallback(() => {
-> 		let result = count.value;
-> 		for (let i = 0; i < 10000000; i++) {
-> 			result += 1;
-> 		}
-> 		return result;
-> 		// Empty deps means callback never changes, alternatively add count here so that if the identity of the signal changes this re-runs
-> 	}, []);
->
-> 	const computed = useComputed(expensiveComputation);
->
-> 	return <div>Result: {computed}</div>;
-> }
-> ```
-
 ### Rendering optimizations
 
 The Preact adapter ships with several optimizations it can apply out of the box to skip virtual-dom rendering entirely. If you pass a signal directly into JSX, it will bind directly to the DOM `Text` node that is created and update that whenever the signal changes.

packages/preact/src/index.ts

@@ -423,18 +423,11 @@ export function useSignal<T>(value?: T, options?: SignalOptions<T>) {
 	)[0];
 }
 
-export function useComputed<T>(
-	compute: () => T,
-	options?: SignalOptions<T>
-): ReadonlySignal<T> {
-	const [$fn, $computed] = useMemo(() => {
-		const $fn = signal(compute);
-		return [$fn, computed(() => $fn.value(), options)] as const;
-	}, []);
-
+export function useComputed<T>(compute: () => T, options?: SignalOptions<T>) {
+	const $compute = useRef(compute);
+	$compute.current = compute;
 	(currentComponent as AugmentedComponent)._updateFlags |= HAS_COMPUTEDS;
-	$fn.value = compute;
-	return $computed;
+	return useMemo(() => computed<T>(() => $compute.current(), options), []);
 }
 
 function safeRaf(callback: () => void) {

packages/preact/test/index.test.tsx

@@ -15,13 +15,7 @@ import {
 	Component,
 } from "preact";
 import type { ComponentChildren, FunctionComponent, VNode } from "preact";
-import {
-	useContext,
-	useEffect,
-	useRef,
-	useState,
-	useCallback,
-} from "preact/hooks";
+import { useContext, useEffect, useRef, useState } from "preact/hooks";
 import { setupRerender, act } from "preact/test-utils";
 
 const sleep = (ms?: number) => new Promise(r => setTimeout(r, ms));
@@ -1076,57 +1070,4 @@ describe("@preact/signals", () => {
 			expect(spy).to.have.been.calledWith("willmount:1");
 		});
 	});
-
-	describe("useComputed", () => {
-		it("should recompute and update dependency list when the compute function changes", async () => {
-			const s1 = signal(1);
-			const s2 = signal("a");
-
-			function App({ x }: { x: Signal }) {
-				const fn = useCallback(() => {
-					return x.value;
-				}, [x]);
-
-				const c = useComputed(fn);
-				return <span>{c.value}</span>;
-			}
-
-			render(<App x={s1} />, scratch);
-			expect(scratch.textContent).to.equal("1");
-
-			render(<App x={s2} />, scratch);
-			expect(scratch.textContent).to.equal("a");
-
-			s1.value = 2;
-			rerender();
-			expect(scratch.textContent).to.equal("a");
-
-			s2.value = "b";
-			rerender();
-			expect(scratch.textContent).to.equal("b");
-		});
-
-		it("should not recompute when the compute function doesn't change and dependency values don't change", async () => {
-			const s1 = signal(1);
-			const spy = sinon.spy();
-
-			function App({ x }: { x: Signal }) {
-				const fn = useCallback(() => {
-					spy();
-					return x.value;
-				}, [x]);
-
-				const c = useComputed(fn);
-				return <span>{c.value}</span>;
-			}
-
-			render(<App x={s1} />, scratch);
-			expect(scratch.textContent).to.equal("1");
-			expect(spy).to.have.been.calledOnce;
-
-			rerender();
-			expect(scratch.textContent).to.equal("1");
-			expect(spy).to.have.been.calledOnce;
-		});
-	});
 });

packages/react/README.md

@@ -103,34 +103,6 @@ function Counter() {
 }
 ```
 
-> **Optimizing `useComputed` performance**
->
-> The `useComputed` hook follows the convention of keeping closures fresh. By default, the callback function passed to `useComputed` will re-run on every component render to ensure it always has access to the latest values from the component's scope. However, if the computed value doesn't change, updates won't propagate to dependent nodes in the signal graph.
->
-> For expensive computations, you can optimize performance by memoizing the callback with `useCallback`:
->
-> ```js
-> import { useSignal, useComputed } from "@preact/signals-react";
-> import { useCallback } from "react";
->
-> function Counter() {
-> 	const count = useSignal(0);
->
-> 	// Memoize the callback to avoid re-running expensive calculations
-> 	const expensiveComputation = useCallback(() => {
-> 		for (let i = 0; i < 10000000; i++) {
-> 			result += 1;
-> 		}
-> 		return result;
-> 		// Empty deps means callback never changes, alternatively add count here so that if the identity of the signal changes this re-runs
-> 	}, []);
->
-> 	const computed = useComputed(expensiveComputation);
->
-> 	return <div>Result: {computed.value}</div>;
-> }
-> ```
-
 ### Using signals with React's SSR APIs
 
 Components rendered using SSR APIs (e.g. `renderToString`) in a server environment (i.e. an environment without a global `window` object) will not track signals used during render. Components generally don't rerender when using SSR APIs so tracking signal usage is useless since changing these signals can't trigger a rerender.