From 2233b2bb01a546cbaf325e950849bb5d34f06149 Mon Sep 17 00:00:00 2001 From: billybimbob Date: Thu, 3 Nov 2022 22:41:45 +0000 Subject: [PATCH 1/7] add use watcher hook --- packages/preact/src/index.ts | 6 ++++++ packages/react/src/index.ts | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/packages/preact/src/index.ts b/packages/preact/src/index.ts index a3398ceda..f878aef2c 100644 --- a/packages/preact/src/index.ts +++ b/packages/preact/src/index.ts @@ -353,6 +353,12 @@ export function useSignalEffect(cb: () => void | (() => void)) { }, []); } +export function useWatcher(value: T) { + const watcher = useSignal(value); + watcher.value = value; + return watcher as ReadonlySignal; +} + /** * @todo Determine which Reactive implementation we'll be using. * @internal diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index a5665e422..f9f29050c 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -234,3 +234,9 @@ export function useSignalEffect(cb: () => void | (() => void)) { }); }, Empty); } + +export function useWatcher(value: T) { + const watcher = useSignal(value); + watcher.value; + return watcher as ReadonlySignal; +} From 4ce10589db6c18956d4db6a1c480918e3b0d8dc9 Mon Sep 17 00:00:00 2001 From: billybimbob Date: Thu, 3 Nov 2022 23:03:03 +0000 Subject: [PATCH 2/7] add testcases --- packages/preact/test/index.test.tsx | 43 ++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/packages/preact/test/index.test.tsx b/packages/preact/test/index.test.tsx index f53b77962..451d63eee 100644 --- a/packages/preact/test/index.test.tsx +++ b/packages/preact/test/index.test.tsx @@ -1,4 +1,4 @@ -import { signal, useComputed } from "@preact/signals"; +import { signal, useComputed, useWatcher } from "@preact/signals"; import { createElement, render } from "preact"; import { setupRerender, act } from "preact/test-utils"; @@ -336,4 +336,45 @@ describe("@preact/signals", () => { }); }); }); + + describe("use watcher hook", () => { + it("should set the initial value of the checked property", () => { + function App({ checked = false }) { + const $checked = useWatcher(checked); + // @ts-ignore + return ; + } + + render(, scratch); + expect(scratch.firstChild).to.have.property("checked", true); + }); + + it("should update the checked property on change", () => { + function App({ checked = false }) { + const $checked = useWatcher(checked); + // @ts-ignore + return ; + } + + render(, scratch); + expect(scratch.firstChild).to.have.property("checked", true); + + render(, scratch); + expect(scratch.firstChild).to.have.property("checked", false); + }); + + it("should update computed signal", () => { + function App({ value = 0 }) { + const $value = useWatcher(value); + const timesTwo = useComputed(() => $value.value * 2); + return

{timesTwo}

; + } + + render(, scratch); + expect(scratch.textContent).to.equal("2"); + + render(, scratch); + expect(scratch.textContent).to.equal("8"); + }); + }); }); From 5af09ce54068215fd0c4880e778933e2a4fac92c Mon Sep 17 00:00:00 2001 From: billybimbob Date: Fri, 4 Nov 2022 20:48:33 +0000 Subject: [PATCH 3/7] fix react update cascades --- packages/preact/test/index.test.tsx | 15 ++++++ packages/react/src/index.ts | 20 +++++++- packages/react/test/index.test.tsx | 76 ++++++++++++++++++++++++++++- 3 files changed, 108 insertions(+), 3 deletions(-) diff --git a/packages/preact/test/index.test.tsx b/packages/preact/test/index.test.tsx index 451d63eee..cb5b1e269 100644 --- a/packages/preact/test/index.test.tsx +++ b/packages/preact/test/index.test.tsx @@ -376,5 +376,20 @@ describe("@preact/signals", () => { render(, scratch); expect(scratch.textContent).to.equal("8"); }); + + it("should not cascade rerenders", () => { + const spy = sinon.spy(); + function App({ value = 0 }) { + const $value = useWatcher(value); + const timesTwo = useComputed(() => $value.value * 2); + spy(); + return

{timesTwo.value}

; + } + + render(, scratch); + render(, scratch); + + expect(spy).to.be.calledTwice; + }); }); }); diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index f9f29050c..7f137c128 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -97,6 +97,8 @@ function WrapWithProxy(Component: FunctionComponent) { return WrappedComponent; } +let renderDepth = 0; + /** * A redux-like store whose store value is a positive 32bit integer (a 'version'). * @@ -115,10 +117,24 @@ function createEffectStore() { let version = 0; let onChangeNotifyReact: (() => void) | undefined; - let unsubscribe = effect(function (this: Effect) { + const unsubscribe = effect(function (this: Effect) { updater = this; }); + + const oldStart = updater._start; + + updater._start = function () { + renderDepth++; + const finish = oldStart.call(updater); + return function () { + finish(); + renderDepth--; + }; + }; + updater._callback = function () { + // only notify React of state changes when not rendering in a component + if (renderDepth > 0) return; version = (version + 1) | 0; if (onChangeNotifyReact) onChangeNotifyReact(); }; @@ -237,6 +253,6 @@ export function useSignalEffect(cb: () => void | (() => void)) { export function useWatcher(value: T) { const watcher = useSignal(value); - watcher.value; + watcher.value = value; return watcher as ReadonlySignal; } diff --git a/packages/react/test/index.test.tsx b/packages/react/test/index.test.tsx index 31a66525c..3d54e1e57 100644 --- a/packages/react/test/index.test.tsx +++ b/packages/react/test/index.test.tsx @@ -1,7 +1,7 @@ // @ts-ignore-next-line globalThis.IS_REACT_ACT_ENVIRONMENT = true; -import { signal, useComputed } from "@preact/signals-react"; +import { signal, useComputed, useWatcher } from "@preact/signals-react"; import { createElement, useMemo, memo, StrictMode } from "react"; import { createRoot, Root } from "react-dom/client"; import { renderToStaticMarkup } from "react-dom/server"; @@ -222,4 +222,78 @@ describe("@preact/signals-react", () => { } }); }); + + describe("use watcher hook", () => { + it("should set the initial value of the checked property", () => { + function App({ value = 0 }) { + const $value = useWatcher(value); + return {$value}; + } + + render(); + expect(scratch.textContent).to.equal("1"); + }); + + it("should update the checked property on change", () => { + function App({ value = 0 }) { + const $value = useWatcher(value); + return {$value}; + } + + render(); + expect(scratch.textContent).to.equal("1"); + + render(); + expect(scratch.textContent).to.equal("4"); + }); + + it("should update computed signal", () => { + function App({ value = 0 }) { + const $value = useWatcher(value); + const timesTwo = useComputed(() => $value.value * 2); + return {timesTwo}; + } + + render(); + expect(scratch.textContent).to.equal("2"); + + render(); + expect(scratch.textContent).to.equal("8"); + }); + + it("should consistently rerender in strict mode", () => { + function Test({ value }: { value: number }) { + const $value = useWatcher(value); + return {$value}; + } + + function App({ value = 0 }) { + return ( + + + + ); + } + + for (let i = 0; i < 3; ++i) { + render(); + expect(scratch.textContent).is.equal(`${i}`); + } + }); + + it("should not cascade rerenders", () => { + const spy = sinon.spy(); + function App({ value = 0 }) { + const $value = useWatcher(value); + const timesTwo = useComputed(() => $value.value * 2); + spy(); + return

{timesTwo.value}

; + } + + render(); + render(); + + expect(spy).to.be.calledTwice; + }); + }); }); From 74a68bf300558abdc642b0565ad5fca3ad5b1ddc Mon Sep 17 00:00:00 2001 From: billybimbob Date: Fri, 4 Nov 2022 20:48:57 +0000 Subject: [PATCH 4/7] refactor preact adapter --- packages/preact/src/index.ts | 76 ++++++++++++++++--------------- packages/preact/src/internal.d.ts | 2 +- 2 files changed, 40 insertions(+), 38 deletions(-) diff --git a/packages/preact/src/index.ts b/packages/preact/src/index.ts index f878aef2c..313c2155d 100644 --- a/packages/preact/src/index.ts +++ b/packages/preact/src/index.ts @@ -54,7 +54,7 @@ function createUpdater(update: () => void) { // if (typeof value !== "object" || value == null) return false; // if (value instanceof Signal) return true; // // @TODO: uncomment this when we land Reactive (ideally behind a brand check) -// // for (let i in value) if (value[i] instanceof Signal) return true; +// // for (const i in value) if (value[i] instanceof Signal) return true; // return false; // } @@ -67,10 +67,9 @@ function Text(this: AugmentedComponent, { data }: { data: Signal }) { // Store the props.data signal in another signal so that // passing a new signal reference re-runs the text computed: - const currentSignal = useSignal(data); - currentSignal.value = data; + const currentSignal = useWatcher(data); - const s = useMemo(() => { + const $s = useMemo(() => { // mark the parent component as having computeds so it gets optimized let v = this.__v; while ((v = v.__!)) { @@ -82,17 +81,16 @@ function Text(this: AugmentedComponent, { data }: { data: Signal }) { // Replace this component's vdom updater with a direct text one: this._updater!._callback = () => { - (this.base as Text).data = s.peek(); + (this.base as Text).data = $s.peek(); }; return computed(() => { - let data = currentSignal.value; - let s = data.value; + const s = currentSignal.value.value; return s === 0 ? 0 : s === true ? "" : s || ""; }); }, []); - return s.value; + return $s.value; } Text.displayName = "_st"; @@ -114,13 +112,13 @@ Object.defineProperties(Signal.prototype, { /** Inject low-level property/attribute bindings for Signals into Preact's diff */ hook(OptionsTypes.DIFF, (old, vnode) => { if (typeof vnode.type === "string") { - let signalProps: Record | undefined; + let signalProps: typeof vnode.__np; - let props = vnode.props; - for (let i in props) { + const props = vnode.props; + for (const i in props) { if (i === "children") continue; - let value = props[i]; + const value = props[i]; if (value instanceof Signal) { if (!signalProps) vnode.__np = signalProps = {}; signalProps[i] = value; @@ -136,9 +134,9 @@ hook(OptionsTypes.DIFF, (old, vnode) => { hook(OptionsTypes.RENDER, (old, vnode) => { setCurrentUpdater(); - let updater; + let updater: Effect | undefined; - let component = vnode.__c; + const component = vnode.__c; if (component) { component._updateFlags &= ~HAS_PENDING_UPDATE; @@ -168,18 +166,16 @@ hook(OptionsTypes.DIFFED, (old, vnode) => { setCurrentUpdater(); currentComponent = undefined; - let dom: Element; - // vnode._dom is undefined during string rendering, // so we use this to skip prop subscriptions during SSR. - if (typeof vnode.type === "string" && (dom = vnode.__e as Element)) { - let props = vnode.__np; - let renderedProps = vnode.props; - if (props) { + if (typeof vnode.type === "string") { + const props = vnode.__np; + const dom = vnode.__e as Element | undefined; + if (props && dom) { let updaters = dom._updaters; if (updaters) { - for (let prop in updaters) { - let updater = updaters[prop]; + for (const prop in updaters) { + const updater = updaters[prop]; if (updater !== undefined && !(prop in props)) { updater._dispose(); // @todo we could just always invoke _dispose() here @@ -187,15 +183,21 @@ hook(OptionsTypes.DIFFED, (old, vnode) => { } } } else { - updaters = {}; - dom._updaters = updaters; + dom._updaters = updaters = {}; } - for (let prop in props) { + + const renderedProps = vnode.props; + + for (const prop in props) { let updater = updaters[prop]; - let signal = props[prop]; + const signal = props[prop]; if (updater === undefined) { - updater = createPropUpdater(dom, prop, signal, renderedProps); - updaters[prop] = updater; + updaters[prop] = updater = createPropUpdater( + dom, + prop, + signal, + renderedProps + ); } else { updater._update(signal, renderedProps); } @@ -244,20 +246,20 @@ function createPropUpdater( /** Unsubscribe from Signals when unmounting components/vnodes */ hook(OptionsTypes.UNMOUNT, (old, vnode: VNode) => { if (typeof vnode.type === "string") { - let dom = vnode.__e as Element | undefined; + const dom = vnode.__e as Element | undefined; // vnode._dom is undefined during string rendering if (dom) { const updaters = dom._updaters; if (updaters) { dom._updaters = undefined; - for (let prop in updaters) { - let updater = updaters[prop]; + for (const prop in updaters) { + const updater = updaters[prop]; if (updater) updater._dispose(); } } } } else { - let component = vnode.__c; + const component = vnode.__c; if (component) { const updater = component._updater; if (updater) { @@ -319,13 +321,13 @@ Component.prototype.shouldComponentUpdate = function ( if (this._updateFlags & (HAS_PENDING_UPDATE | HAS_HOOK_STATE)) return true; // @ts-ignore - for (let i in state) return true; + for (const i in state) return true; // if any non-Signal props changed, update: - for (let i in props) { + for (const i in props) { if (i !== "__source" && props[i] !== this.props[i]) return true; } - for (let i in this.props) if (!(i in props)) return true; + for (const i in this.props) if (!(i in props)) return true; // this is a purely Signal-driven component, don't update: return false; @@ -407,7 +409,7 @@ export function update( if (obj instanceof Signal) { obj.value = peekValue(update); } else { - for (let i in update) { + for (const i in update) { if (i in obj) { obj[i].value = peekValue(update[i]); } else { @@ -417,7 +419,7 @@ export function update( } } if (overwrite) { - for (let i in obj) { + for (const i in obj) { if (!(i in update)) { obj[i].value = undefined; } diff --git a/packages/preact/src/internal.d.ts b/packages/preact/src/internal.d.ts index 436ac029a..a62b373bc 100644 --- a/packages/preact/src/internal.d.ts +++ b/packages/preact/src/internal.d.ts @@ -31,7 +31,7 @@ export interface VNode

extends preact.VNode

{ /** The DOM node for this VNode */ __e?: Element | Text; /** Props that had Signal values before diffing (used after diffing to subscribe) */ - __np?: Record | null; + __np?: Record | null; } export const enum OptionsTypes { From b20ac818a26a8827b18f397f2ae7f281c101c43d Mon Sep 17 00:00:00 2001 From: billybimbob Date: Fri, 4 Nov 2022 21:19:06 +0000 Subject: [PATCH 5/7] add sibling testcases --- packages/preact/test/index.test.tsx | 49 +++++++++++++++++++++++++++++ packages/react/test/index.test.tsx | 49 +++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+) diff --git a/packages/preact/test/index.test.tsx b/packages/preact/test/index.test.tsx index cb5b1e269..3af60d030 100644 --- a/packages/preact/test/index.test.tsx +++ b/packages/preact/test/index.test.tsx @@ -391,5 +391,54 @@ describe("@preact/signals", () => { expect(spy).to.be.calledTwice; }); + + it("should update all silblings", () => { + function Test({ value }: { value: number }) { + const $value = useWatcher(value); + return {$value.value}; + } + + function App({ value = 0 }) { + return ( +

+ + +
+ ); + } + + const firstChild = () => scratch.firstChild?.firstChild; + + render(, scratch); + expect(firstChild()?.textContent).to.be.equal("1"); + expect(firstChild()?.nextSibling?.textContent).to.be.equal("1"); + + render(, scratch); + expect(firstChild()?.textContent).to.be.equal("4"); + expect(firstChild()?.nextSibling?.textContent).to.be.equal("4"); + }); + + it("should not rerender siblings", () => { + const spy = sinon.spy(); + function Test({ value }: { value: number }) { + const $value = useWatcher(value); + spy(); + return {$value.value}; + } + + function App({ value = 0 }) { + return ( +
+ + +
+ ); + } + + render(, scratch); + render(, scratch); + + expect(spy).to.be.callCount(4); + }); }); }); diff --git a/packages/react/test/index.test.tsx b/packages/react/test/index.test.tsx index 3d54e1e57..0aa1d9fc7 100644 --- a/packages/react/test/index.test.tsx +++ b/packages/react/test/index.test.tsx @@ -295,5 +295,54 @@ describe("@preact/signals-react", () => { expect(spy).to.be.calledTwice; }); + + it("should update all silblings", () => { + function Test({ value }: { value: number }) { + const $value = useWatcher(value); + return {$value.value}; + } + + function App({ value = 0 }) { + return ( +
+ + +
+ ); + } + + const firstChild = () => scratch.firstChild?.firstChild; + + render(); + expect(firstChild()?.textContent).to.be.equal("1"); + expect(firstChild()?.nextSibling?.textContent).to.be.equal("1"); + + render(); + expect(firstChild()?.textContent).to.be.equal("4"); + expect(firstChild()?.nextSibling?.textContent).to.be.equal("4"); + }); + + it("should not rerender siblings", () => { + const spy = sinon.spy(); + function Test({ value }: { value: number }) { + const $value = useWatcher(value); + spy(); + return {$value.value}; + } + + function App({ value = 0 }) { + return ( +
+ + +
+ ); + } + + render(); + render(); + + expect(spy).to.be.callCount(4); + }); }); }); From 26338319b468b8cf5eacda572d847987f9ae0fe0 Mon Sep 17 00:00:00 2001 From: billybimbob Date: Fri, 4 Nov 2022 21:48:05 +0000 Subject: [PATCH 6/7] revert const refactors --- packages/preact/src/index.ts | 49 ++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/packages/preact/src/index.ts b/packages/preact/src/index.ts index 313c2155d..3c1cf965e 100644 --- a/packages/preact/src/index.ts +++ b/packages/preact/src/index.ts @@ -54,7 +54,7 @@ function createUpdater(update: () => void) { // if (typeof value !== "object" || value == null) return false; // if (value instanceof Signal) return true; // // @TODO: uncomment this when we land Reactive (ideally behind a brand check) -// // for (const i in value) if (value[i] instanceof Signal) return true; +// // for (let i in value) if (value[i] instanceof Signal) return true; // return false; // } @@ -114,11 +114,11 @@ hook(OptionsTypes.DIFF, (old, vnode) => { if (typeof vnode.type === "string") { let signalProps: typeof vnode.__np; - const props = vnode.props; - for (const i in props) { + let props = vnode.props; + for (let i in props) { if (i === "children") continue; - const value = props[i]; + let value = props[i]; if (value instanceof Signal) { if (!signalProps) vnode.__np = signalProps = {}; signalProps[i] = value; @@ -136,7 +136,7 @@ hook(OptionsTypes.RENDER, (old, vnode) => { let updater: Effect | undefined; - const component = vnode.__c; + let component = vnode.__c; if (component) { component._updateFlags &= ~HAS_PENDING_UPDATE; @@ -166,16 +166,18 @@ hook(OptionsTypes.DIFFED, (old, vnode) => { setCurrentUpdater(); currentComponent = undefined; + let dom: Element; + // vnode._dom is undefined during string rendering, // so we use this to skip prop subscriptions during SSR. - if (typeof vnode.type === "string") { - const props = vnode.__np; - const dom = vnode.__e as Element | undefined; - if (props && dom) { + if (typeof vnode.type === "string" && (dom = vnode.__e as Element)) { + let props = vnode.__np; + let renderedProps = vnode.props; + if (props) { let updaters = dom._updaters; if (updaters) { - for (const prop in updaters) { - const updater = updaters[prop]; + for (let prop in updaters) { + let updater = updaters[prop]; if (updater !== undefined && !(prop in props)) { updater._dispose(); // @todo we could just always invoke _dispose() here @@ -185,12 +187,9 @@ hook(OptionsTypes.DIFFED, (old, vnode) => { } else { dom._updaters = updaters = {}; } - - const renderedProps = vnode.props; - - for (const prop in props) { + for (let prop in props) { let updater = updaters[prop]; - const signal = props[prop]; + let signal = props[prop]; if (updater === undefined) { updaters[prop] = updater = createPropUpdater( dom, @@ -246,20 +245,20 @@ function createPropUpdater( /** Unsubscribe from Signals when unmounting components/vnodes */ hook(OptionsTypes.UNMOUNT, (old, vnode: VNode) => { if (typeof vnode.type === "string") { - const dom = vnode.__e as Element | undefined; + let dom = vnode.__e as Element | undefined; // vnode._dom is undefined during string rendering if (dom) { const updaters = dom._updaters; if (updaters) { dom._updaters = undefined; - for (const prop in updaters) { - const updater = updaters[prop]; + for (let prop in updaters) { + let updater = updaters[prop]; if (updater) updater._dispose(); } } } } else { - const component = vnode.__c; + let component = vnode.__c; if (component) { const updater = component._updater; if (updater) { @@ -321,13 +320,13 @@ Component.prototype.shouldComponentUpdate = function ( if (this._updateFlags & (HAS_PENDING_UPDATE | HAS_HOOK_STATE)) return true; // @ts-ignore - for (const i in state) return true; + for (let i in state) return true; // if any non-Signal props changed, update: - for (const i in props) { + for (let i in props) { if (i !== "__source" && props[i] !== this.props[i]) return true; } - for (const i in this.props) if (!(i in props)) return true; + for (let i in this.props) if (!(i in props)) return true; // this is a purely Signal-driven component, don't update: return false; @@ -409,7 +408,7 @@ export function update( if (obj instanceof Signal) { obj.value = peekValue(update); } else { - for (const i in update) { + for (let i in update) { if (i in obj) { obj[i].value = peekValue(update[i]); } else { @@ -419,7 +418,7 @@ export function update( } } if (overwrite) { - for (const i in obj) { + for (let i in obj) { if (!(i in update)) { obj[i].value = undefined; } From 7169cb8f73357bf94abf6200f8d514fedbeed2eb Mon Sep 17 00:00:00 2001 From: billybimbob Date: Thu, 5 Jan 2023 15:39:06 +0000 Subject: [PATCH 7/7] Add changeset --- .changeset/loud-dolls-confess.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/loud-dolls-confess.md diff --git a/.changeset/loud-dolls-confess.md b/.changeset/loud-dolls-confess.md new file mode 100644 index 000000000..06f77dc2f --- /dev/null +++ b/.changeset/loud-dolls-confess.md @@ -0,0 +1,6 @@ +--- +"@preact/signals": major +"@preact/signals-react": major +--- + +Add useWatcher hook