Raf aligned dom updates

.changeset/ninety-beans-compare.md

@@ -0,0 +1,16 @@
+---
+"@preact/signals": major
+---
+
+Defer all DOM updates by an animation frame, this should make it so
+that any previously synchronous DOM update will be instead delayed by an
+animation frame. This allows Preact to first perform its own render
+cycle and then our direct DOM updates to occur. These will now
+be performed in a batched way which is more performant as the browser
+is prepared to handle these during the animation frame.
+
+This does impact how Preact based signals are tested, when
+you perform a signal update, you'll need to wrap it in `act`. In a way
+this was always the case, as a signal update that resulted in
+a Preact state update would require it to be wrapped in `act`, but
+now this is the norm.

packages/preact/src/index.ts

@@ -79,32 +79,21 @@ function SignalValue(this: AugmentedComponent, { data }: { data: Signal }) {
 	const currentSignal = useSignal(data);
 	currentSignal.value = data;
 
-	const s = useMemo(() => {
-		// mark the parent component as having computeds so it gets optimized
-		let v = this.__v;
-		while ((v = v.__!)) {
-			if (v.__c) {
-				v.__c._updateFlags |= HAS_COMPUTEDS;
-				break;
-			}
-		}
-
-		this._updater!._callback = () => {
-			if (isValidElement(s.peek()) || this.base?.nodeType !== 3) {
-				this._updateFlags |= HAS_PENDING_UPDATE;
-				this.setState({});
-				return;
-			}
+	const s = useComputed(() => {
+		let data = currentSignal.value;
+		let s = data.value;
+		return s === 0 ? 0 : s === true ? "" : s || "";
+	});
 
-			(this.base as Text).data = s.peek();
-		};
+	const isText = useComputed(() => {
+		return !isValidElement(s.peek()) || this.base?.nodeType === 3;
+	});
 
-		return computed(() => {
-			let data = currentSignal.value;
-			let s = data.value;
-			return s === 0 ? 0 : s === true ? "" : s || "";
-		});
-	}, []);
+	useSignalEffect(() => {
+		if (isText.value) {
+			(this.base as Text).data = data.peek();
+		}
+	});
 
 	return s.value;
 }
@@ -238,7 +227,9 @@ function createPropUpdater(
 			changeSignal.value = newSignal;
 			props = newProps;
 		},
-		_dispose: effect(() => {
+		_dispose: effect(function (this: Effect) {
+			if (!oldNotifyEffects) oldNotifyEffects = this._notify;
+			this._notify = notifyEffects;
 			const value = changeSignal.value.value;
 			// If Preact just rendered this value, don't render it again:
 			if (props[prop] === value) return;
@@ -358,28 +349,26 @@ export function useComputed<T>(compute: () => T) {
 	return useMemo(() => computed<T>(() => $compute.current()), []);
 }
 
-let oldNotify: (this: Effect) => void,
-	queue: Array<Effect> = [],
-	isFlushing = false;
+let oldNotifyEffects: (this: Effect) => void,
+	effectsQueue: Array<Effect> = [];
+
+const defer =
+	typeof requestAnimationFrame === "undefined"
+		? setTimeout
+		: requestAnimationFrame;
 
-function flush() {
+function flushEffects() {
 	batch(() => {
-		let flushing = [...queue];
-		isFlushing = false;
-		queue.length = 0;
-		for (let i = 0; i < flushing.length; i++) {
-			oldNotify.call(flushing[i]);
+		let inst: Effect | undefined;
+		while ((inst = effectsQueue.shift())) {
+			oldNotifyEffects.call(inst);
 		}
 	});
 }
 
-function notify(this: Effect) {
-	queue.push(this);
-	if (!isFlushing) {
-		isFlushing = true;
-		(typeof requestAnimationFrame === "undefined"
-			? setTimeout
-			: requestAnimationFrame)(flush);
+function notifyEffects(this: Effect) {
+	if (effectsQueue.push(this) === 1) {
+		(options.requestAnimationFrame || defer)(flushEffects);
 	}
 }
 
@@ -389,8 +378,8 @@ export function useSignalEffect(cb: () => void | (() => void)) {
 
 	useEffect(() => {
 		return effect(function (this: Effect) {
-			if (!oldNotify) oldNotify = this._notify;
-			this._notify = notify;
+			if (!oldNotifyEffects) oldNotifyEffects = this._notify;
+			this._notify = notifyEffects;
 			return callback.current();
 		});
 	}, []);

packages/preact/test/index.test.tsx

@@ -13,15 +13,6 @@ import { useContext, useRef, useState } from "preact/hooks";
 import { setupRerender, act } from "preact/test-utils";
 
 const sleep = (ms?: number) => new Promise(r => setTimeout(r, ms));
-const defer =
-	typeof requestAnimationFrame === "undefined"
-		? setTimeout
-		: requestAnimationFrame;
-const afterFrame = () => {
-	return new Promise(res => {
-		defer(res);
-	});
-};
 
 describe("@preact/signals", () => {
 	let scratch: HTMLDivElement;
@@ -63,14 +54,16 @@ describe("@preact/signals", () => {
 			expect(text).to.have.property("data", "test");
 		});
 
-		it("should update Signal-based SignalValue (no parent component)", () => {
+		it("should update Signal-based SignalValue (no parent component)", async () => {
 			const sig = signal("test");
 			render(<span>{sig}</span>, scratch);
 
 			const text = scratch.firstChild!.firstChild!;
 			expect(text).to.have.property("data", "test");
 
-			sig.value = "changed";
+			act(() => {
+				sig.value = "changed";
+			});
 
 			// should not remount/replace SignalValue
 			expect(scratch.firstChild!.firstChild!).to.equal(text);
@@ -91,7 +84,9 @@ describe("@preact/signals", () => {
 			const text = scratch.firstChild!.firstChild!;
 			expect(text).to.have.property("data", "test");
 
-			sig.value = "changed";
+			act(() => {
+				sig.value = "changed";
+			});
 
 			// should not remount/replace SignalValue
 			expect(scratch.firstChild!.firstChild!).to.equal(text);
@@ -109,14 +104,19 @@ describe("@preact/signals", () => {
 				spy();
 				return <span>{x}</span>;
 			}
-			render(<App x={sig} />, scratch);
+
+			act(() => {
+				render(<App x={sig} />, scratch);
+			});
 			spy.resetHistory();
 
 			const text = scratch.firstChild!.firstChild!;
 			expect(text).to.have.property("data", "test");
 
 			const sig2 = signal("different");
-			render(<App x={sig2} />, scratch);
+			act(() => {
+				render(<App x={sig2} />, scratch);
+			});
 			expect(spy).to.have.been.called;
 			spy.resetHistory();
 
@@ -128,14 +128,18 @@ describe("@preact/signals", () => {
 			await sleep();
 			expect(spy).not.to.have.been.called;
 
-			sig.value = "changed old signal";
+			act(() => {
+				sig.value = "changed old signal";
+			});
 
 			await sleep();
 			expect(spy).not.to.have.been.called;
 			// the text should _not_ have changed:
 			expect(text).to.have.property("data", "different");
 
-			sig2.value = "changed";
+			act(() => {
+				sig2.value = "changed";
+			});
 
 			expect(scratch.firstChild!.firstChild!).to.equal(text);
 			expect(text).to.have.property("data", "changed");
@@ -177,11 +181,10 @@ describe("@preact/signals", () => {
 			expect(text).to.be.an.instanceOf(HTMLSpanElement);
 			expect(text).to.have.property("firstChild").that.is.an.instanceOf(Text);
 
-			sig.value = <div>a</div>;
-
+			act(() => {
+				sig.value = <div>a</div>;
+			});
 			expect(spy).not.to.have.been.calledOnce;
-
-			rerender();
 			scratch.firstChild!.firstChild!.textContent!.should.equal("a");
 		});
 
@@ -198,23 +201,30 @@ describe("@preact/signals", () => {
 			expect(text.textContent).to.equal("test");
 			expect(text).to.be.an.instanceOf(HTMLSpanElement);
 			expect(text).to.have.property("firstChild").that.is.an.instanceOf(Text);
-			sig.value = "a";
-			rerender();
+
+			act(() => {
+				sig.value = "a";
+			});
 			text = scratch.firstChild!.firstChild!;
 			expect(text.nodeType).to.equal(Node.TEXT_NODE);
 			expect(text.textContent).to.equal("a");
 
-			sig.value = "b";
+			act(() => {
+				sig.value = "b";
+			});
 			expect(text.textContent).to.equal("b");
 
-			sig.value = <div>c</div>;
-			rerender();
+			act(() => {
+				sig.value = <div>c</div>;
+			});
 			await sleep();
 			text = scratch.firstChild!.firstChild!;
 
 			expect(text).to.be.an.instanceOf(HTMLDivElement);
 			expect(text.textContent).to.equal("c");
-			sig.value = <span>d</span>;
+			act(() => {
+				sig.value = <span>d</span>;
+			});
 			rerender();
 			await sleep();
 
@@ -461,14 +471,16 @@ describe("@preact/signals", () => {
 			expect(s.value).to.equal(true);
 		});
 
-		it("should update the checked property on change", () => {
+		it("should update the checked property on change", async () => {
 			const s = signal(true);
 			// @ts-ignore
 			render(<input checked={s} />, scratch);
 
 			expect(scratch.firstChild).to.have.property("checked", true);
 
-			s.value = false;
+			act(() => {
+				s.value = false;
+			});
 
 			expect(scratch.firstChild).to.have.property("checked", false);
 		});
@@ -486,15 +498,19 @@ describe("@preact/signals", () => {
 
 			expect(scratch.firstChild).to.have.property("value", "initial");
 
-			s.value = "updated";
+			act(() => {
+				s.value = "updated";
+			});
 
 			expect(scratch.firstChild).to.have.property("value", "updated");
 
 			// ensure the component was never re-rendered: (even after a tick)
 			await sleep();
 			expect(spy).not.to.have.been.called;
 
-			s.value = "second update";
+			act(() => {
+				s.value = "second update";
+			});
 
 			expect(scratch.firstChild).to.have.property("value", "second update");
 
@@ -522,7 +538,9 @@ describe("@preact/signals", () => {
 			await sleep();
 			expect(spy).not.to.have.been.called;
 
-			style.value = "left: 20px;";
+			act(() => {
+				style.value = "left: 20px;";
+			});
 
 			expect(div.style).to.have.property("left", "20px");
 
@@ -679,7 +697,6 @@ describe("@preact/signals", () => {
 			});
 			expect(scratch.textContent).to.equal("foo");
 			// expect(spy).not.to.have.been.called;
-			await afterFrame();
 			expect(spy).to.have.been.calledOnceWith(
 				"foo",
 				scratch.firstElementChild,
@@ -690,11 +707,9 @@ describe("@preact/signals", () => {
 
 			act(() => {
 				sig.value = "bar";
-				rerender();
 			});
 
 			expect(scratch.textContent).to.equal("bar");
-			await afterFrame();
 
 			expect(spy).to.have.been.calledOnceWith(
 				"bar",
@@ -715,7 +730,9 @@ describe("@preact/signals", () => {
 					const id = ref.current.getAttribute("data-render-id");
 					const value = sig.value;
 					spy(value, ref.current, id);
-					return () => cleanup(value, ref.current, id);
+					return () => {
+						cleanup(value, ref.current, id);
+					};
 				});
 				return (
 					<p ref={ref} data-render-id={count++}>
@@ -728,7 +745,6 @@ describe("@preact/signals", () => {
 				render(<App />, scratch);
 			});
 
-			await afterFrame();
 			expect(cleanup).not.to.have.been.called;
 			expect(spy).to.have.been.calledOnceWith(
 				"foo",
@@ -739,16 +755,12 @@ describe("@preact/signals", () => {
 
 			act(() => {
 				sig.value = "bar";
-				rerender();
 			});
 
 			expect(scratch.textContent).to.equal("bar");
-			await afterFrame();
 
 			const child = scratch.firstElementChild;
-
 			expect(cleanup).to.have.been.calledOnceWith("foo", child, "0");
-
 			expect(spy).to.have.been.calledOnceWith("bar", child, "1");
 		});
 
@@ -771,8 +783,6 @@ describe("@preact/signals", () => {
 				render(<App />, scratch);
 			});
 
-			await afterFrame();
-
 			const child = scratch.firstElementChild;
 
 			expect(cleanup).not.to.have.been.called;
@@ -783,8 +793,6 @@ describe("@preact/signals", () => {
 				render(null, scratch);
 			});
 
-			await afterFrame();
-
 			expect(spy).not.to.have.been.called;
 			expect(cleanup).to.have.been.calledOnceWith("foo", child);
 		});