diff --git a/.changeset/ninety-beans-compare.md b/.changeset/ninety-beans-compare.md new file mode 100644 index 00000000..f9322d8c --- /dev/null +++ b/.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. diff --git a/packages/preact/src/index.ts b/packages/preact/src/index.ts index 35c0582b..3a1b0fec 100644 --- a/packages/preact/src/index.ts +++ b/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(compute: () => T) { return useMemo(() => computed(() => $compute.current()), []); } -let oldNotify: (this: Effect) => void, - queue: Array = [], - isFlushing = false; +let oldNotifyEffects: (this: Effect) => void, + effectsQueue: Array = []; + +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(); }); }, []); diff --git a/packages/preact/test/index.test.tsx b/packages/preact/test/index.test.tsx index 7a8efa82..a74ca53b 100644 --- a/packages/preact/test/index.test.tsx +++ b/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({sig}, 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 {x}; } - render(, scratch); + + act(() => { + render(, scratch); + }); spy.resetHistory(); const text = scratch.firstChild!.firstChild!; expect(text).to.have.property("data", "test"); const sig2 = signal("different"); - render(, scratch); + act(() => { + render(, 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 =
a
; - + act(() => { + sig.value =
a
; + }); 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 =
c
; - rerender(); + act(() => { + sig.value =
c
; + }); await sleep(); text = scratch.firstChild!.firstChild!; expect(text).to.be.an.instanceOf(HTMLDivElement); expect(text.textContent).to.equal("c"); - sig.value = d; + act(() => { + sig.value = d; + }); 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(, 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,7 +498,9 @@ 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"); @@ -494,7 +508,9 @@ describe("@preact/signals", () => { 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 (

@@ -728,7 +745,6 @@ describe("@preact/signals", () => { render(, 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(, 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); });