From 943b61cfbdd6583e3e8a066ee056b31a5563c376 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Thu, 3 Oct 2024 09:19:15 +0200 Subject: [PATCH 1/6] Raf aligned dom updates --- .changeset/ninety-beans-compare.md | 5 +++ packages/preact/src/index.ts | 57 ++++++++++++++++---------- packages/preact/test/index.test.tsx | 32 ++++++++++----- packages/react/test/shared/updates.tsx | 7 ++++ 4 files changed, 70 insertions(+), 31 deletions(-) create mode 100644 .changeset/ninety-beans-compare.md diff --git a/.changeset/ninety-beans-compare.md b/.changeset/ninety-beans-compare.md new file mode 100644 index 00000000..de5b0a0f --- /dev/null +++ b/.changeset/ninety-beans-compare.md @@ -0,0 +1,5 @@ +--- +"@preact/signals": minor +--- + +Defer all DOM updates by an animation frame as well diff --git a/packages/preact/src/index.ts b/packages/preact/src/index.ts index 35c0582b..5f23095b 100644 --- a/packages/preact/src/index.ts +++ b/packages/preact/src/index.ts @@ -99,7 +99,9 @@ function SignalValue(this: AugmentedComponent, { data }: { data: Signal }) { (this.base as Text).data = s.peek(); }; - return computed(() => { + return computed(function (this: Effect) { + if (!oldNotifyComputeds) oldNotifyComputeds = this._notify; + this._notify = notifyComputeds; let data = currentSignal.value; let s = data.value; return s === 0 ? 0 : s === true ? "" : s || ""; @@ -238,7 +240,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 +362,39 @@ 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 = [], + oldNotifyComputeds: (this: Effect) => void, + computedsQueue: Array = []; -function flush() { +const defer = + typeof requestAnimationFrame === "undefined" + ? setTimeout + : requestAnimationFrame; + +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 notifyEffects(this: Effect) { + if (effectsQueue.push(this) === 1) { + defer(flushEffects); + } +} + +function flushComputeds() { + batch(() => { + let inst: Effect | undefined; + while ((inst = computedsQueue.shift())) oldNotifyComputeds.call(inst); }); } -function notify(this: Effect) { - queue.push(this); - if (!isFlushing) { - isFlushing = true; - (typeof requestAnimationFrame === "undefined" - ? setTimeout - : requestAnimationFrame)(flush); +function notifyComputeds(this: Effect) { + if (computedsQueue.push(this) === 1) { + defer(flushComputeds); } } @@ -389,8 +404,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..c7986938 100644 --- a/packages/preact/test/index.test.tsx +++ b/packages/preact/test/index.test.tsx @@ -63,7 +63,7 @@ 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); @@ -71,6 +71,7 @@ describe("@preact/signals", () => { expect(text).to.have.property("data", "test"); sig.value = "changed"; + await afterFrame(); // should not remount/replace SignalValue expect(scratch.firstChild!.firstChild!).to.equal(text); @@ -92,6 +93,7 @@ describe("@preact/signals", () => { expect(text).to.have.property("data", "test"); sig.value = "changed"; + await afterFrame(); // should not remount/replace SignalValue expect(scratch.firstChild!.firstChild!).to.equal(text); @@ -120,6 +122,7 @@ describe("@preact/signals", () => { expect(spy).to.have.been.called; spy.resetHistory(); + await afterFrame(); // should not remount/replace SignalValue expect(scratch.firstChild!.firstChild!).to.equal(text); // should update the text in-place @@ -136,6 +139,7 @@ describe("@preact/signals", () => { expect(text).to.have.property("data", "different"); sig2.value = "changed"; + await afterFrame(); expect(scratch.firstChild!.firstChild!).to.equal(text); expect(text).to.have.property("data", "changed"); @@ -178,10 +182,9 @@ describe("@preact/signals", () => { expect(text).to.have.property("firstChild").that.is.an.instanceOf(Text); sig.value =
a
; - - expect(spy).not.to.have.been.calledOnce; - + await afterFrame(); rerender(); + expect(spy).not.to.have.been.calledOnce; scratch.firstChild!.firstChild!.textContent!.should.equal("a"); }); @@ -199,15 +202,18 @@ describe("@preact/signals", () => { expect(text).to.be.an.instanceOf(HTMLSpanElement); expect(text).to.have.property("firstChild").that.is.an.instanceOf(Text); sig.value = "a"; + await afterFrame(); rerender(); text = scratch.firstChild!.firstChild!; expect(text.nodeType).to.equal(Node.TEXT_NODE); expect(text.textContent).to.equal("a"); sig.value = "b"; + await afterFrame(); expect(text.textContent).to.equal("b"); sig.value =
c
; + await afterFrame(); rerender(); await sleep(); text = scratch.firstChild!.firstChild!; @@ -215,6 +221,7 @@ describe("@preact/signals", () => { expect(text).to.be.an.instanceOf(HTMLDivElement); expect(text.textContent).to.equal("c"); sig.value = d; + await afterFrame(); rerender(); await sleep(); @@ -461,7 +468,7 @@ 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); @@ -469,6 +476,7 @@ describe("@preact/signals", () => { expect(scratch.firstChild).to.have.property("checked", true); s.value = false; + await afterFrame(); expect(scratch.firstChild).to.have.property("checked", false); }); @@ -487,6 +495,7 @@ describe("@preact/signals", () => { expect(scratch.firstChild).to.have.property("value", "initial"); s.value = "updated"; + await afterFrame(); expect(scratch.firstChild).to.have.property("value", "updated"); @@ -495,6 +504,7 @@ describe("@preact/signals", () => { expect(spy).not.to.have.been.called; s.value = "second update"; + await afterFrame(); expect(scratch.firstChild).to.have.property("value", "second update"); @@ -523,6 +533,7 @@ describe("@preact/signals", () => { expect(spy).not.to.have.been.called; style.value = "left: 20px;"; + await afterFrame(); expect(div.style).to.have.property("left", "20px"); @@ -695,6 +706,7 @@ describe("@preact/signals", () => { expect(scratch.textContent).to.equal("bar"); await afterFrame(); + await afterFrame(); expect(spy).to.have.been.calledOnceWith( "bar", @@ -715,7 +727,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 (

@@ -737,18 +751,16 @@ describe("@preact/signals", () => { ); spy.resetHistory(); - act(() => { + act(async () => { sig.value = "bar"; rerender(); }); - expect(scratch.textContent).to.equal("bar"); await afterFrame(); + expect(scratch.textContent).to.equal("bar"); const child = scratch.firstElementChild; - expect(cleanup).to.have.been.calledOnceWith("foo", child, "0"); - expect(spy).to.have.been.calledOnceWith("bar", child, "1"); }); diff --git a/packages/react/test/shared/updates.tsx b/packages/react/test/shared/updates.tsx index 2e90f6b5..e9cae8f4 100644 --- a/packages/react/test/shared/updates.tsx +++ b/packages/react/test/shared/updates.tsx @@ -161,6 +161,7 @@ export function updateSignalsTests(usingTransform = false) { await act(() => { sig.value = "bar"; }); + await afterFrame(); expect(scratch.textContent).to.equal("bar"); }); @@ -282,6 +283,7 @@ export function updateSignalsTests(usingTransform = false) { await act(() => { sig.value = "bar"; }); + await afterFrame(); expect(scratch.textContent).to.equal("bar"); }); @@ -302,6 +304,7 @@ export function updateSignalsTests(usingTransform = false) { await act(() => { sig.value = "bar"; }); + await afterFrame(); expect(scratch.textContent).to.equal("bar"); }); @@ -322,6 +325,7 @@ export function updateSignalsTests(usingTransform = false) { await act(async () => { sig.value = i; }); + await afterFrame(); expect(scratch.textContent).to.equal("" + i); } }); @@ -343,6 +347,7 @@ export function updateSignalsTests(usingTransform = false) { await act(async () => { sig.value = i; }); + await afterFrame(); expect(scratch.textContent).to.equal("" + i); } }); @@ -411,6 +416,7 @@ export function updateSignalsTests(usingTransform = false) { await act(async () => { count.value += 1; }); + await afterFrame(); expect(scratch.innerHTML).to.equal( `

-2${count.value * 2}
` ); @@ -617,6 +623,7 @@ export function updateSignalsTests(usingTransform = false) { await act(() => { scratch.querySelector("button")!.click(); }); + await afterFrame(); expect(url.textContent).to.equal("https://domain.com/test?a=2"); expect(URLModelProvider).to.be.calledOnce; From b8974063fc9c86f8b68cf24441fdcabe82af0deb Mon Sep 17 00:00:00 2001 From: jdecroock Date: Sat, 5 Oct 2024 09:29:26 +0200 Subject: [PATCH 2/6] Test approach leveraging our existing options.debounceRendering This all works fine, however it creates a discrepancy between browser usage and test usage which we want to avoid. The discrepancy is made clear in the failing tests. What we want to happen is for Preact to be first allowed to perform its render cycle, the effects that follow and only then should we clean up stragglers by performing the direct DOM updates. Currently what happens is that the ordering is "random" based on how the signals were registered. --- packages/preact/src/index.ts | 4 +- packages/preact/test/index.test.tsx | 90 ++++++++++++++++------------- 2 files changed, 51 insertions(+), 43 deletions(-) diff --git a/packages/preact/src/index.ts b/packages/preact/src/index.ts index 5f23095b..02f3e832 100644 --- a/packages/preact/src/index.ts +++ b/packages/preact/src/index.ts @@ -381,7 +381,7 @@ function flushEffects() { function notifyEffects(this: Effect) { if (effectsQueue.push(this) === 1) { - defer(flushEffects); + (options.debounceRendering || defer)(flushEffects); } } @@ -394,7 +394,7 @@ function flushComputeds() { function notifyComputeds(this: Effect) { if (computedsQueue.push(this) === 1) { - defer(flushComputeds); + (options.debounceRendering || defer)(flushComputeds); } } diff --git a/packages/preact/test/index.test.tsx b/packages/preact/test/index.test.tsx index c7986938..ff85d85c 100644 --- a/packages/preact/test/index.test.tsx +++ b/packages/preact/test/index.test.tsx @@ -23,7 +23,7 @@ const afterFrame = () => { }); }; -describe("@preact/signals", () => { +describe.only("@preact/signals", () => { let scratch: HTMLDivElement; let rerender: () => void; @@ -70,8 +70,9 @@ describe("@preact/signals", () => { const text = scratch.firstChild!.firstChild!; expect(text).to.have.property("data", "test"); - sig.value = "changed"; - await afterFrame(); + act(() => { + sig.value = "changed"; + }); // should not remount/replace SignalValue expect(scratch.firstChild!.firstChild!).to.equal(text); @@ -92,8 +93,9 @@ describe("@preact/signals", () => { const text = scratch.firstChild!.firstChild!; expect(text).to.have.property("data", "test"); - sig.value = "changed"; - await afterFrame(); + act(() => { + sig.value = "changed"; + }); // should not remount/replace SignalValue expect(scratch.firstChild!.firstChild!).to.equal(text); @@ -111,18 +113,22 @@ 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(); - await afterFrame(); // should not remount/replace SignalValue expect(scratch.firstChild!.firstChild!).to.equal(text); // should update the text in-place @@ -131,15 +137,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"; - await afterFrame(); + act(() => { + sig2.value = "changed"; + }); expect(scratch.firstChild!.firstChild!).to.equal(text); expect(text).to.have.property("data", "changed"); @@ -181,9 +190,9 @@ describe("@preact/signals", () => { expect(text).to.be.an.instanceOf(HTMLSpanElement); expect(text).to.have.property("firstChild").that.is.an.instanceOf(Text); - sig.value =
a
; - await afterFrame(); - rerender(); + act(() => { + sig.value =
a
; + }); expect(spy).not.to.have.been.calledOnce; scratch.firstChild!.firstChild!.textContent!.should.equal("a"); }); @@ -201,26 +210,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"; - await afterFrame(); - 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"; - await afterFrame(); + act(() => { + sig.value = "b"; + }); expect(text.textContent).to.equal("b"); - sig.value =
c
; - await afterFrame(); - 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; + }); await afterFrame(); rerender(); await sleep(); @@ -475,8 +488,9 @@ describe("@preact/signals", () => { expect(scratch.firstChild).to.have.property("checked", true); - s.value = false; - await afterFrame(); + act(() => { + s.value = false; + }); expect(scratch.firstChild).to.have.property("checked", false); }); @@ -494,8 +508,9 @@ describe("@preact/signals", () => { expect(scratch.firstChild).to.have.property("value", "initial"); - s.value = "updated"; - await afterFrame(); + act(() => { + s.value = "updated"; + }); expect(scratch.firstChild).to.have.property("value", "updated"); @@ -503,8 +518,9 @@ describe("@preact/signals", () => { await sleep(); expect(spy).not.to.have.been.called; - s.value = "second update"; - await afterFrame(); + act(() => { + s.value = "second update"; + }); expect(scratch.firstChild).to.have.property("value", "second update"); @@ -532,8 +548,9 @@ describe("@preact/signals", () => { await sleep(); expect(spy).not.to.have.been.called; - style.value = "left: 20px;"; - await afterFrame(); + act(() => { + style.value = "left: 20px;"; + }); expect(div.style).to.have.property("left", "20px"); @@ -701,12 +718,9 @@ describe("@preact/signals", () => { act(() => { sig.value = "bar"; - rerender(); }); expect(scratch.textContent).to.equal("bar"); - await afterFrame(); - await afterFrame(); expect(spy).to.have.been.calledOnceWith( "bar", @@ -751,12 +765,10 @@ describe("@preact/signals", () => { ); spy.resetHistory(); - act(async () => { + act(() => { sig.value = "bar"; - rerender(); }); - await afterFrame(); expect(scratch.textContent).to.equal("bar"); const child = scratch.firstElementChild; @@ -783,8 +795,6 @@ describe("@preact/signals", () => { render(, scratch); }); - await afterFrame(); - const child = scratch.firstElementChild; expect(cleanup).not.to.have.been.called; @@ -795,8 +805,6 @@ describe("@preact/signals", () => { render(null, scratch); }); - await afterFrame(); - expect(spy).not.to.have.been.called; expect(cleanup).to.have.been.calledOnceWith("foo", child); }); From 3519fb5cbf80fa81feba518c67e3f6c7455b24aa Mon Sep 17 00:00:00 2001 From: jdecroock Date: Sat, 5 Oct 2024 09:44:11 +0200 Subject: [PATCH 3/6] options.requestAnimationFrame solves it --- .changeset/ninety-beans-compare.md | 13 ++++++++++++- packages/preact/src/index.ts | 4 ++-- packages/preact/test/index.test.tsx | 14 +------------- packages/react/test/shared/updates.tsx | 7 ------- 4 files changed, 15 insertions(+), 23 deletions(-) diff --git a/.changeset/ninety-beans-compare.md b/.changeset/ninety-beans-compare.md index de5b0a0f..fd3e8b1a 100644 --- a/.changeset/ninety-beans-compare.md +++ b/.changeset/ninety-beans-compare.md @@ -2,4 +2,15 @@ "@preact/signals": minor --- -Defer all DOM updates by an animation frame as well +Defer all DOM updates by an animation frame, this should make it so +that any used to be synchronous DOM update will be 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 have one way to how Preact based signals are tested, when +we perform a signal update we have 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 02f3e832..7a408d6b 100644 --- a/packages/preact/src/index.ts +++ b/packages/preact/src/index.ts @@ -381,7 +381,7 @@ function flushEffects() { function notifyEffects(this: Effect) { if (effectsQueue.push(this) === 1) { - (options.debounceRendering || defer)(flushEffects); + (options.requestAnimationFrame || defer)(flushEffects); } } @@ -394,7 +394,7 @@ function flushComputeds() { function notifyComputeds(this: Effect) { if (computedsQueue.push(this) === 1) { - (options.debounceRendering || defer)(flushComputeds); + (options.requestAnimationFrame || defer)(flushComputeds); } } diff --git a/packages/preact/test/index.test.tsx b/packages/preact/test/index.test.tsx index ff85d85c..a74ca53b 100644 --- a/packages/preact/test/index.test.tsx +++ b/packages/preact/test/index.test.tsx @@ -13,17 +13,8 @@ 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.only("@preact/signals", () => { +describe("@preact/signals", () => { let scratch: HTMLDivElement; let rerender: () => void; @@ -234,7 +225,6 @@ describe.only("@preact/signals", () => { act(() => { sig.value = d; }); - await afterFrame(); rerender(); await sleep(); @@ -707,7 +697,6 @@ describe.only("@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, @@ -756,7 +745,6 @@ describe.only("@preact/signals", () => { render(, scratch); }); - await afterFrame(); expect(cleanup).not.to.have.been.called; expect(spy).to.have.been.calledOnceWith( "foo", diff --git a/packages/react/test/shared/updates.tsx b/packages/react/test/shared/updates.tsx index e9cae8f4..2e90f6b5 100644 --- a/packages/react/test/shared/updates.tsx +++ b/packages/react/test/shared/updates.tsx @@ -161,7 +161,6 @@ export function updateSignalsTests(usingTransform = false) { await act(() => { sig.value = "bar"; }); - await afterFrame(); expect(scratch.textContent).to.equal("bar"); }); @@ -283,7 +282,6 @@ export function updateSignalsTests(usingTransform = false) { await act(() => { sig.value = "bar"; }); - await afterFrame(); expect(scratch.textContent).to.equal("bar"); }); @@ -304,7 +302,6 @@ export function updateSignalsTests(usingTransform = false) { await act(() => { sig.value = "bar"; }); - await afterFrame(); expect(scratch.textContent).to.equal("bar"); }); @@ -325,7 +322,6 @@ export function updateSignalsTests(usingTransform = false) { await act(async () => { sig.value = i; }); - await afterFrame(); expect(scratch.textContent).to.equal("" + i); } }); @@ -347,7 +343,6 @@ export function updateSignalsTests(usingTransform = false) { await act(async () => { sig.value = i; }); - await afterFrame(); expect(scratch.textContent).to.equal("" + i); } }); @@ -416,7 +411,6 @@ export function updateSignalsTests(usingTransform = false) { await act(async () => { count.value += 1; }); - await afterFrame(); expect(scratch.innerHTML).to.equal( `
-2${count.value * 2}
` ); @@ -623,7 +617,6 @@ export function updateSignalsTests(usingTransform = false) { await act(() => { scratch.querySelector("button")!.click(); }); - await afterFrame(); expect(url.textContent).to.equal("https://domain.com/test?a=2"); expect(URLModelProvider).to.be.calledOnce; From e5f503f033bbbfc28ae048df9497ba495db4c5db Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Mon, 7 Oct 2024 09:09:09 +0200 Subject: [PATCH 4/6] Improve changelog post Co-authored-by: Ryan Christian <33403762+rschristian@users.noreply.github.com> --- .changeset/ninety-beans-compare.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.changeset/ninety-beans-compare.md b/.changeset/ninety-beans-compare.md index fd3e8b1a..d7e1bf42 100644 --- a/.changeset/ninety-beans-compare.md +++ b/.changeset/ninety-beans-compare.md @@ -3,14 +3,14 @@ --- Defer all DOM updates by an animation frame, this should make it so -that any used to be synchronous DOM update will be delayed by an +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 have one way to how Preact based signals are tested, when -we perform a signal update we have 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 +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. From f8e5d098555cb9c5cf5cf2391976d759e0471848 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Mon, 7 Oct 2024 13:04:33 +0200 Subject: [PATCH 5/6] Update .changeset/ninety-beans-compare.md --- .changeset/ninety-beans-compare.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/ninety-beans-compare.md b/.changeset/ninety-beans-compare.md index d7e1bf42..f9322d8c 100644 --- a/.changeset/ninety-beans-compare.md +++ b/.changeset/ninety-beans-compare.md @@ -1,5 +1,5 @@ --- -"@preact/signals": minor +"@preact/signals": major --- Defer all DOM updates by an animation frame, this should make it so From 9804e9fd41157796db3079d8f765dd6fad99487f Mon Sep 17 00:00:00 2001 From: jdecroock Date: Sat, 5 Oct 2024 09:44:11 +0200 Subject: [PATCH 6/6] Skip changing the SignalValue notifier when it's an object (JSX) --- packages/preact/src/index.ts | 60 ++++++++++-------------------------- 1 file changed, 17 insertions(+), 43 deletions(-) diff --git a/packages/preact/src/index.ts b/packages/preact/src/index.ts index 7a408d6b..3a1b0fec 100644 --- a/packages/preact/src/index.ts +++ b/packages/preact/src/index.ts @@ -79,34 +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(function (this: Effect) { - if (!oldNotifyComputeds) oldNotifyComputeds = this._notify; - this._notify = notifyComputeds; - 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; } @@ -363,9 +350,7 @@ export function useComputed(compute: () => T) { } let oldNotifyEffects: (this: Effect) => void, - effectsQueue: Array = [], - oldNotifyComputeds: (this: Effect) => void, - computedsQueue: Array = []; + effectsQueue: Array = []; const defer = typeof requestAnimationFrame === "undefined" @@ -375,7 +360,9 @@ const defer = function flushEffects() { batch(() => { let inst: Effect | undefined; - while ((inst = effectsQueue.shift())) oldNotifyEffects.call(inst); + while ((inst = effectsQueue.shift())) { + oldNotifyEffects.call(inst); + } }); } @@ -385,19 +372,6 @@ function notifyEffects(this: Effect) { } } -function flushComputeds() { - batch(() => { - let inst: Effect | undefined; - while ((inst = computedsQueue.shift())) oldNotifyComputeds.call(inst); - }); -} - -function notifyComputeds(this: Effect) { - if (computedsQueue.push(this) === 1) { - (options.requestAnimationFrame || defer)(flushComputeds); - } -} - export function useSignalEffect(cb: () => void | (() => void)) { const callback = useRef(cb); callback.current = cb;