Removed difference in behaviour of @preact/signals and @preact/signal… (

#387)

* Removed difference in behaviour of @preact/signals and @preact/signals-react

* added more explicit jsx checking

* renamed Text to SignalValue in preact adapter

* renamed Text to SignalValue in react adapter

* resolved pr comment

* pr fixes: stay with useMemo

* removed unnecessary field

* changeset added

* removed internal changeset

we don't need to talk about these to the outside world

* Update .changeset/tender-pianos-love.md

---------

Co-authored-by: Jovi De Croock <[email protected]>

.changeset/tender-pianos-love.md

@@ -0,0 +1,5 @@
+---
+"@preact/signals": minor
+---
+
+Removed difference in behaviour between adapters, signals that use a JSX value will correctly re-render the whole component rather than attempting the JSX-Text optimization.

packages/preact/src/index.ts

@@ -1,4 +1,4 @@
-import { options, Component } from "preact";
+import { options, Component, isValidElement } from "preact";
 import { useRef, useMemo, useEffect } from "preact/hooks";
 import {
 	signal,
@@ -71,7 +71,7 @@ function createUpdater(update: () => void) {
  * A wrapper component that renders a Signal directly as a Text node.
  * @todo: in Preact 11, just decorate Signal with `type:null`
  */
-function Text(this: AugmentedComponent, { data }: { data: Signal }) {
+function SignalValue(this: AugmentedComponent, { data }: { data: Signal }) {
 	// hasComputeds.add(this);
 
 	// Store the props.data signal in another signal so that
@@ -89,8 +89,13 @@ function Text(this: AugmentedComponent, { data }: { data: Signal }) {
 			}
 		}
 
-		// Replace this component's vdom updater with a direct text one:
 		this._updater!._callback = () => {
+			if (isValidElement(s.peek()) || this.base?.nodeType !== 3) {
+				this._updateFlags |= HAS_PENDING_UPDATE;
+				this.setState({});
+				return;
+			}
+
 			(this.base as Text).data = s.peek();
 		};
 
@@ -103,11 +108,11 @@ function Text(this: AugmentedComponent, { data }: { data: Signal }) {
 
 	return s.value;
 }
-Text.displayName = "_st";
+SignalValue.displayName = "_st";
 
 Object.defineProperties(Signal.prototype, {
 	constructor: { configurable: true, value: undefined },
-	type: { configurable: true, value: Text },
+	type: { configurable: true, value: SignalValue },
 	props: {
 		configurable: true,
 		get() {

packages/preact/test/index.test.tsx

@@ -34,15 +34,15 @@ describe("@preact/signals", () => {
 		});
 	});
 
-	describe("Text bindings", () => {
+	describe("SignalValue bindings", () => {
 		it("should render text without signals", () => {
 			render(<span>test</span>, scratch);
 			const span = scratch.firstChild;
 			const text = span?.firstChild;
 			expect(text).to.have.property("data", "test");
 		});
 
-		it("should render Signals as Text", () => {
+		it("should render Signals as SignalValue", () => {
 			const sig = signal("test");
 			render(<span>{sig}</span>, scratch);
 			const span = scratch.firstChild;
@@ -51,7 +51,7 @@ describe("@preact/signals", () => {
 			expect(text).to.have.property("data", "test");
 		});
 
-		it("should update Signal-based Text (no parent component)", () => {
+		it("should update Signal-based SignalValue (no parent component)", () => {
 			const sig = signal("test");
 			render(<span>{sig}</span>, scratch);
 
@@ -60,13 +60,13 @@ describe("@preact/signals", () => {
 
 			sig.value = "changed";
 
-			// should not remount/replace Text
+			// should not remount/replace SignalValue
 			expect(scratch.firstChild!.firstChild!).to.equal(text);
 			// should update the text in-place
 			expect(text).to.have.property("data", "changed");
 		});
 
-		it("should update Signal-based Text (in a parent component)", async () => {
+		it("should update Signal-based SignalValue (in a parent component)", async () => {
 			const sig = signal("test");
 			const spy = sinon.spy();
 			function App({ x }: { x: typeof sig }) {
@@ -81,7 +81,7 @@ describe("@preact/signals", () => {
 
 			sig.value = "changed";
 
-			// should not remount/replace Text
+			// should not remount/replace SignalValue
 			expect(scratch.firstChild!.firstChild!).to.equal(text);
 			// should update the text in-place
 			expect(text).to.have.property("data", "changed");
@@ -90,7 +90,7 @@ describe("@preact/signals", () => {
 			expect(spy).not.to.have.been.called;
 		});
 
-		it("should support swapping Signals in Text positions", async () => {
+		it("should support swapping Signals in SignalValue positions", async () => {
 			const sig = signal("test");
 			const spy = sinon.spy();
 			function App({ x }: { x: typeof sig }) {
@@ -108,7 +108,7 @@ describe("@preact/signals", () => {
 			expect(spy).to.have.been.called;
 			spy.resetHistory();
 
-			// should not remount/replace Text
+			// should not remount/replace SignalValue
 			expect(scratch.firstChild!.firstChild!).to.equal(text);
 			// should update the text in-place
 			expect(text).to.have.property("data", "different");
@@ -131,6 +131,85 @@ describe("@preact/signals", () => {
 			await sleep();
 			expect(spy).not.to.have.been.called;
 		});
+
+		it("should support rendering JSX in SignalValue positions", async () => {
+			const sig = signal(<span>test</span>);
+			function App({ x }: { x: typeof sig }) {
+				return <span>{x}</span>;
+			}
+
+			render(<App x={sig} />, scratch);
+
+			const text = scratch.firstChild!.firstChild!;
+
+			expect(text.textContent).to.equal("test");
+			expect(text).to.be.an.instanceOf(HTMLSpanElement);
+			expect(text).to.have.property("firstChild").that.is.an.instanceOf(Text);
+		});
+
+		it("JSX in SignalValue should be reactive", async () => {
+			const sig = signal(<span>test</span>);
+			const spy = sinon.spy();
+			function App({ x }: { x: typeof sig }) {
+				spy();
+				return <span>{x}</span>;
+			}
+
+			render(<App x={sig} />, scratch);
+			expect(spy).to.have.been.calledOnce;
+			spy.resetHistory();
+
+			const text = scratch.firstChild!.firstChild!;
+
+			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 = <div>a</div>;
+
+			expect(spy).not.to.have.been.calledOnce;
+
+			rerender();
+			scratch.firstChild!.firstChild!.textContent!.should.equal("a");
+		});
+
+		it("should support swapping between JSX and string in SignalValue positions", async () => {
+			const sig = signal<JSX.Element | string>(<span>test</span>);
+			function App({ x }: { x: typeof sig }) {
+				return <span>{x}</span>;
+			}
+
+			render(<App x={sig} />, scratch);
+
+			let text = scratch.firstChild!.firstChild!;
+
+			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();
+			text = scratch.firstChild!.firstChild!;
+			expect(text.nodeType).to.equal(Node.TEXT_NODE);
+			expect(text.textContent).to.equal("a");
+
+			sig.value = "b";
+			expect(text.textContent).to.equal("b");
+
+			sig.value = <div>c</div>;
+			rerender();
+			await sleep();
+			text = scratch.firstChild!.firstChild!;
+
+			expect(text).to.be.an.instanceOf(HTMLDivElement);
+			expect(text.textContent).to.equal("c");
+			sig.value = <span>d</span>;
+			rerender();
+			await sleep();
+
+			text = scratch.firstChild!.firstChild!;
+			expect(text).to.be.an.instanceOf(HTMLSpanElement);
+			expect(text.textContent).to.equal("d");
+		});
 	});
 
 	describe("Component bindings", () => {
@@ -418,38 +497,44 @@ describe("@preact/signals", () => {
 		});
 	});
 
-	describe('hooks mixed with signals', () => {
-		it('signals should not stop context from propagating', () => {
-			const ctx = createContext({ test: 'should-not-exist' });
+	describe("hooks mixed with signals", () => {
+		it("signals should not stop context from propagating", () => {
+			const ctx = createContext({ test: "should-not-exist" });
 			let update: any;
 
 			function Provider(props: any) {
-				const [test, setTest] = useState('foo');
-				update = setTest
-				return (
-					<ctx.Provider value={{ test }}>{props.children}</ctx.Provider>
-				);
+				const [test, setTest] = useState("foo");
+				update = setTest;
+				return <ctx.Provider value={{ test }}>{props.children}</ctx.Provider>;
 			}
 
-			const s = signal('baz')
+			const s = signal("baz");
 			function Test() {
 				const value = useContext(ctx);
-				return <p>{value.test} {s.value}</p>
+				return (
+					<p>
+						{value.test} {s.value}
+					</p>
+				);
 			}
 
 			function App() {
-				return <Provider><Test /></Provider>
+				return (
+					<Provider>
+						<Test />
+					</Provider>
+				);
 			}
 
 			render(<App />, scratch);
 
-			expect(scratch.innerHTML).to.equal('<p>foo baz</p>')
+			expect(scratch.innerHTML).to.equal("<p>foo baz</p>");
 			act(() => {
-				update('bar')
-			})
-			expect(scratch.innerHTML).to.equal('<p>bar baz</p>')
-		})
-	})
+				update("bar");
+			});
+			expect(scratch.innerHTML).to.equal("<p>bar baz</p>");
+		});
+	});
 
 	describe("useSignalEffect()", () => {
 		it("should be invoked after commit", async () => {

packages/preact/test/ssr.test.tsx

@@ -15,7 +15,7 @@ describe("@preact/signals", () => {
 			expect(() => renderToString(<App />)).not.to.throw();
 		});
 
-		describe("Text bindings", () => {
+		describe("SignalValue bindings", () => {
 			it("should render strings", () => {
 				const s = signal("hello");
 				expect(renderToString(<p>{s}</p>)).to.equal(`<p>hello</p>`);

packages/react/runtime/src/index.ts

@@ -119,16 +119,16 @@ export function useSignals(): () => void {
 }
 
 /**
- * A wrapper component that renders a Signal's value directly as a Text node.
+ * A wrapper component that renders a Signal's value directly as a Text node or JSX.
  */
-function Text({ data }: { data: Signal }) {
+function SignalValue({ data }: { data: Signal }) {
 	return data.value;
 }
 
-// Decorate Signals so React renders them as <Text> components.
+// Decorate Signals so React renders them as <SignalValue> components.
 Object.defineProperties(Signal.prototype, {
 	$$typeof: { configurable: true, value: ReactElemType },
-	type: { configurable: true, value: Text },
+	type: { configurable: true, value: SignalValue },
 	props: {
 		configurable: true,
 		get() {

packages/react/test/browser/updates.test.tsx

@@ -57,15 +57,15 @@ describe("@preact/signals-react updating", () => {
 		checkHangingAct();
 	});
 
-	describe("Text bindings", () => {
+	describe("SignalValue bindings", () => {
 		it("should render text without signals", async () => {
 			await render(<span>test</span>);
 			const span = scratch.firstChild;
 			const text = span?.firstChild;
 			expect(text).to.have.property("data", "test");
 		});
 
-		it("should render Signals as Text", async () => {
+		it("should render Signals as SignalValue", async () => {
 			const sig = signal("test");
 			await render(<span>{sig}</span>);
 			const span = scratch.firstChild;
@@ -74,7 +74,7 @@ describe("@preact/signals-react updating", () => {
 			expect(text).to.have.property("data", "test");
 		});
 
-		it("should render computed as Text", async () => {
+		it("should render computed as SignalValue", async () => {
 			const sig = signal("test");
 			const comp = computed(() => `${sig} ${sig}`);
 			await render(<span>{comp}</span>);
@@ -84,7 +84,7 @@ describe("@preact/signals-react updating", () => {
 			expect(text).to.have.property("data", "test test");
 		});
 
-		it("should update Signal-based Text (no parent component)", async () => {
+		it("should update Signal-based SignalValue (no parent component)", async () => {
 			const sig = signal("test");
 			await render(<span>{sig}</span>);
 
@@ -95,13 +95,13 @@ describe("@preact/signals-react updating", () => {
 				sig.value = "changed";
 			});
 
-			// should not remount/replace Text
+			// should not remount/replace SignalValue
 			expect(scratch.firstChild!.firstChild!).to.equal(text);
 			// should update the text in-place
 			expect(text).to.have.property("data", "changed");
 		});
 
-		it("should update Signal-based Text (in a parent component)", async () => {
+		it("should update Signal-based SignalValue (in a parent component)", async () => {
 			const sig = signal("test");
 			function App({ x }: { x: typeof sig }) {
 				return <span>{x}</span>;
@@ -115,11 +115,31 @@ describe("@preact/signals-react updating", () => {
 				sig.value = "changed";
 			});
 
-			// should not remount/replace Text
+			// should not remount/replace SignalValue
 			expect(scratch.firstChild!.firstChild!).to.equal(text);
 			// should update the text in-place
 			expect(text).to.have.property("data", "changed");
 		});
+
+		it("should work with JSX inside signal", async () => {
+			const sig = signal(<b>test</b>);
+			function App({ x }: { x: typeof sig }) {
+				return <span>{x}</span>;
+			}
+			await render(<App x={sig} />);
+
+			let text = scratch.firstChild!.firstChild!;
+			expect(text).to.be.instanceOf(HTMLElement);
+			expect(text.firstChild).to.have.property("data", "test");
+
+			await act(() => {
+				sig.value = <div>changed</div>;
+			});
+
+			text = scratch.firstChild!.firstChild!;
+			expect(text).to.be.instanceOf(HTMLDivElement);
+			expect(text.firstChild).to.have.property("data", "changed");
+		});
 	});
 
 	describe("Component bindings", () => {

packages/react/test/shared/mounting.tsx

@@ -29,13 +29,13 @@ export function mountSignalsTests(
 			expect(html).to.equal("<span>test</span>");
 		});
 
-		it("should render Signals as Text", async () => {
+		it("should render Signals as SignalValue", async () => {
 			const sig = signal("test");
 			const html = await render(<span>{sig}</span>);
 			expect(html).to.equal("<span>test</span>");
 		});
 
-		it("should render computed as Text", async () => {
+		it("should render computed as SignalValue", async () => {
 			const sig = signal("test");
 			const comp = computed(() => `${sig} ${sig}`);
 			const html = await render(<span>{comp}</span>);