Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

15 changes: 13 additions & 2 deletions packages/preact/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { options, Component } from "preact";
import { options, Component, isValidElement } from "preact";
XantreDev marked this conversation as resolved.
Show resolved Hide resolved
import { useRef, useMemo, useEffect } from "preact/hooks";
import {
signal,
Expand Down Expand Up @@ -80,9 +80,20 @@ function Text(this: AugmentedComponent, { data }: { data: Signal }) {
}
}

const defaultUpdaterCallback = this._updater!._callback;

// Replace this component's vdom updater with a direct text one:
this._updater!._callback = () => {
XantreDev marked this conversation as resolved.
Show resolved Hide resolved
(this.base as Text).data = s.peek();
const node = this.base;
// if we need to update the text directly, we should ensure that current node is a text node
// if it is not, we should promise preact to create it via rerendering the whole component
if (isValidElement(s.peek()) || node?.nodeType !== 3) {
defaultUpdaterCallback();

return;
}

(node as Text).data = s.peek();
};

return computed(() => {
Expand Down
127 changes: 110 additions & 17 deletions packages/preact/test/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,93 @@ describe("@preact/signals", () => {
await sleep();
expect(spy).not.to.have.been.called;
});

it("should support rendering JSX in Text 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 Text 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 Text 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);

{
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 = "a";
rerender();
{
XantreDev marked this conversation as resolved.
Show resolved Hide resolved
const 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();
{
const 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();

const text = scratch.firstChild!.firstChild!;
expect(text).to.be.an.instanceOf(HTMLSpanElement);
expect(text.textContent).to.equal("d");
}
});
});

describe("Component bindings", () => {
Expand Down Expand Up @@ -418,38 +505,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", () => {
XantreDev marked this conversation as resolved.
Show resolved Hide resolved
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 () => {
Expand Down