diff --git a/.changeset/healthy-lamps-teach.md b/.changeset/healthy-lamps-teach.md new file mode 100644 index 000000000..1ce2937cf --- /dev/null +++ b/.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. diff --git a/packages/preact/README.md b/packages/preact/README.md index 8e4444213..ee94d98c8 100644 --- a/packages/preact/README.md +++ b/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
Result: {computed}
; -> } -> ``` - ### 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. diff --git a/packages/preact/src/index.ts b/packages/preact/src/index.ts index 5cbe4e94c..021249a01 100644 --- a/packages/preact/src/index.ts +++ b/packages/preact/src/index.ts @@ -423,18 +423,11 @@ export function useSignal(value?: T, options?: SignalOptions) { )[0]; } -export function useComputed( - compute: () => T, - options?: SignalOptions -): ReadonlySignal { - const [$fn, $computed] = useMemo(() => { - const $fn = signal(compute); - return [$fn, computed(() => $fn.value(), options)] as const; - }, []); - +export function useComputed(compute: () => T, options?: SignalOptions) { + const $compute = useRef(compute); + $compute.current = compute; (currentComponent as AugmentedComponent)._updateFlags |= HAS_COMPUTEDS; - $fn.value = compute; - return $computed; + return useMemo(() => computed(() => $compute.current(), options), []); } function safeRaf(callback: () => void) { diff --git a/packages/preact/test/index.test.tsx b/packages/preact/test/index.test.tsx index 6f57d91fa..eeb4c2178 100644 --- a/packages/preact/test/index.test.tsx +++ b/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 {c.value}; - } - - render(, scratch); - expect(scratch.textContent).to.equal("1"); - - render(, 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 {c.value}; - } - - render(, 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; - }); - }); }); diff --git a/packages/react/README.md b/packages/react/README.md index 92763283f..61b593030 100644 --- a/packages/react/README.md +++ b/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
Result: {computed.value}
; -> } -> ``` - ### 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.