Skip to content

feat: add jsxBind function to @preact/signals #724

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/tender-waves-do.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@preact/signals": minor
---

Introduce the `jsxBind` function for inlined `computed` declarations as JSX attributes or JSX children.
24 changes: 23 additions & 1 deletion packages/preact/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,10 @@ 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 SignalValue(this: AugmentedComponent, { data }: { data: Signal }) {
function SignalValue(
this: AugmentedComponent,
{ data }: { data: ReadonlySignal }
) {
// hasComputeds.add(this);

// Store the props.data signal in another signal so that
Expand All @@ -105,6 +108,10 @@ function SignalValue(this: AugmentedComponent, { data }: { data: Signal }) {

const wrappedSignal = computed(() => {
let s = currentSignal.value.value;
// This is possibly an inline computed from jsxBind
if (typeof s === "function") {
s = s();
}
return s === 0 ? 0 : s === true ? "" : s || "";
});

Expand Down Expand Up @@ -172,13 +179,17 @@ 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") {
const oldSignalProps = vnode.__np;
let signalProps: Record<string, any> | undefined;

let props = vnode.props;
for (let i in props) {
if (i === "children") continue;

let value = props[i];
if (value && typeof value === "object" && value.__proto__ === jsxBind) {
value = oldSignalProps?.[i] || computed(value.value);
}
if (value instanceof Signal) {
if (!signalProps) vnode.__np = signalProps = {};
signalProps[i] = value;
Expand Down Expand Up @@ -465,6 +476,17 @@ export function useSignalEffect(
}, []);
}

/**
* Bind the given callback to a JSX attribute or JSX child. This allows for "inline computed"
* signals that derive their value from other signals. Like with `useComputed`, any non-signal
* values used in the callback are captured at the time of binding and won't change after that.
*/
export function jsxBind<T>(cb: () => T): T {
return { value: cb, __proto__: jsxBind } as any;
}

Object.setPrototypeOf(jsxBind, Signal.prototype);

/**
* @todo Determine which Reactive implementation we'll be using.
* @internal
Expand Down
154 changes: 154 additions & 0 deletions packages/preact/test/index.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
jsxBind,
computed,
useComputed,
useSignalEffect,
Expand Down Expand Up @@ -724,6 +725,159 @@ describe("@preact/signals", () => {
});
});

describe("jsxBind", () => {
it("should bind a callback to a JSX attribute", async () => {
const count = signal(0);
const double = signal(2);
const spy = sinon.spy();

function App() {
spy();
return (
<div data-value={jsxBind(() => count.value * double.value)}></div>
);
}

render(<App />, scratch);
expect(spy).to.have.been.calledOnce;
expect(scratch.innerHTML).to.equal('<div data-value="0"></div>');

act(() => {
count.value = 5;
});

// Component should not re-render when only the bound value changes
expect(spy).to.have.been.calledOnce;
expect(scratch.innerHTML).to.equal('<div data-value="10"></div>');

act(() => {
double.value = 3;
});

expect(spy).to.have.been.calledOnce;
expect(scratch.innerHTML).to.equal('<div data-value="15"></div>');
});

it("should bind a callback to a JSX child", async () => {
const firstName = signal("John");
const lastName = signal("Doe");
const spy = sinon.spy();

function App() {
spy();
return (
<div>{jsxBind(() => `${firstName.value} ${lastName.value}`)}</div>
);
}

render(<App />, scratch);
expect(spy).to.have.been.calledOnce;
expect(scratch.innerHTML).to.equal("<div>John Doe</div>");

act(() => {
firstName.value = "Jane";
});

// Component should not re-render when only the bound value changes
expect(spy).to.have.been.calledOnce;
expect(scratch.innerHTML).to.equal("<div>Jane Doe</div>");
});

it("should update bound values without re-rendering the component", async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a test where we first return JSX and then text or something, similar to the normal SignalValue component

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 👍

const count = signal(0);
const enabled = signal(true);
const renderSpy = sinon.spy();
const boundSpy = sinon.spy(() =>
enabled.value ? count.value : "disabled"
);

function App() {
renderSpy();
return (
<button disabled={jsxBind(() => !enabled.value)}>
{jsxBind(boundSpy)}
</button>
);
}

render(<App />, scratch);
expect(renderSpy).to.have.been.calledOnce;
expect(boundSpy).to.have.been.called;
expect(scratch.innerHTML).to.equal("<button>0</button>");

act(() => {
count.value = 5;
});

expect(renderSpy).to.have.been.calledOnce;
expect(boundSpy).to.have.been.calledTwice;
expect(scratch.innerHTML).to.equal("<button>5</button>");

act(() => {
enabled.value = false;
});

expect(renderSpy).to.have.been.calledOnce;
expect(scratch.innerHTML).to.equal(
`<button disabled="">disabled</button>`
);
});

it("can toggle between JSX text and JSX element", async () => {
const bold = signal(false);
const label = signal("Hello");
const renderSpy = sinon.spy();

function App() {
renderSpy();
return (
<div>
{jsxBind(() =>
bold.value ? <strong>{label.value}</strong> : label.value
)}
</div>
);
}

render(<App />, scratch);
expect(renderSpy).to.have.been.calledOnce;
expect(scratch.innerHTML).to.equal("<div>Hello</div>");

// Text-to-text update.
act(() => {
label.value = "Bonjour";
});

expect(renderSpy).to.have.been.calledOnce;
expect(scratch.innerHTML).to.equal("<div>Bonjour</div>");

// Text-to-element update.
act(() => {
bold.value = true;
});

expect(renderSpy).to.have.been.calledOnce;
expect(scratch.innerHTML).to.equal("<div><strong>Bonjour</strong></div>");

// Element-to-element update.
act(() => {
label.value = "Pryvit";
});

expect(renderSpy).to.have.been.calledOnce;
expect(scratch.innerHTML).to.equal("<div><strong>Pryvit</strong></div>");

// Element-to-text update.
act(() => {
label.value = "Hola";
bold.value = false;
});

expect(renderSpy).to.have.been.calledOnce;
expect(scratch.innerHTML).to.equal("<div>Hola</div>");
});
});

describe("hooks mixed with signals", () => {
it("signals should not stop context from propagating", () => {
const ctx = createContext({ test: "should-not-exist" });
Expand Down