Skip to content

Commit

Permalink
Capture expected behavior of passing signals through context (#425)
Browse files Browse the repository at this point in the history
* Attempt to add repro of test for using a signal that only every written and reads are always on computed

* Modify test to be signal model ref passed through context pattern
  • Loading branch information
andrewiggins authored Oct 5, 2023
1 parent 3b3496d commit de68f7c
Show file tree
Hide file tree
Showing 2 changed files with 216 additions and 2 deletions.
109 changes: 108 additions & 1 deletion packages/preact/test/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@ import {
useSignalEffect,
Signal,
signal,
useSignal,
} from "@preact/signals";
import type { ReadonlySignal } from "@preact/signals";
import { createElement, createRef, render, createContext } from "preact";
import { useContext, useState } from "preact/hooks";
import type { ComponentChildren, FunctionComponent } from "preact";
import { useContext, useRef, useState } from "preact/hooks";
import { setupRerender, act } from "preact/test-utils";

const sleep = (ms?: number) => new Promise(r => setTimeout(r, ms));
Expand Down Expand Up @@ -270,6 +273,110 @@ describe("@preact/signals", () => {
expect(spy).to.be.calledOnce;
});

it("should minimize rerenders when passing signals through context", () => {
function spyOn<P = { children?: ComponentChildren }>(
c: FunctionComponent<P>
) {
return sinon.spy(c);
}

// Manually read signal value below so we can watch whether components rerender
const Origin = spyOn(function Origin() {
const origin = useContext(URLModelContext).origin;
return <span>{origin.value}</span>;
});

const Pathname = spyOn(function Pathname() {
const pathname = useContext(URLModelContext).pathname;
return <span>{pathname.value}</span>;
});

const Search = spyOn(function Search() {
const search = useContext(URLModelContext).search;
return <span>{search.value}</span>;
});

// Never reads signal value during render so should never rerender
const UpdateURL = spyOn(function UpdateURL() {
const update = useContext(URLModelContext).update;
return (
<button
onClick={() => {
update(newURL => {
newURL.search = newURL.search === "?a=1" ? "?a=2" : "?a=1";
});
}}
>
update
</button>
);
});

interface URLModel {
origin: ReadonlySignal<string>;
pathname: ReadonlySignal<string>;
search: ReadonlySignal<string>;
update(updater: (newURL: URL) => void): void;
}

// Also never reads signal value during render so should never rerender
const URLModelContext = createContext<URLModel>(null as any);
const URLModelProvider = spyOn(function SignalProvider({ children }) {
const url = useSignal(new URL("https://domain.com/test?a=1"));
const modelRef = useRef<URLModel | null>(null);

if (modelRef.current == null) {
modelRef.current = {
origin: computed(() => url.value.origin),
pathname: computed(() => url.value.pathname),
search: computed(() => url.value.search),
update(updater) {
const newURL = new URL(url.value);
updater(newURL);
url.value = newURL;
},
};
}

return (
<URLModelContext.Provider value={modelRef.current}>
{children}
</URLModelContext.Provider>
);
});

function App() {
return (
<URLModelProvider>
<p>
<Origin />
<Pathname />
<Search />
</p>
<UpdateURL />
</URLModelProvider>
);
}

render(<App />, scratch);

const url = scratch.querySelector("p")!;
expect(url.textContent).to.equal("https://domain.com/test?a=1");
expect(URLModelProvider).to.be.calledOnce;
expect(Origin).to.be.calledOnce;
expect(Pathname).to.be.calledOnce;
expect(Search).to.be.calledOnce;

scratch.querySelector("button")!.click();
rerender();

expect(url.textContent).to.equal("https://domain.com/test?a=2");
expect(URLModelProvider).to.be.calledOnce;
expect(Origin).to.be.calledOnce;
expect(Pathname).to.be.calledOnce;
expect(Search).to.be.calledTwice;
});

it("should not subscribe to computed signals only created and not used", () => {
const sig = signal(0);
const childSpy = sinon.spy();
Expand Down
109 changes: 108 additions & 1 deletion packages/react/test/browser/updates.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import {
useComputed,
useSignalEffect,
useSignal,
Signal,
} from "@preact/signals-react";
import type { Signal, ReadonlySignal } from "@preact/signals-react";
import {
createElement,
Fragment,
Expand All @@ -21,7 +21,9 @@ import {
useState,
useContext,
createContext,
useRef,
} from "react";
import type { FunctionComponent } from "react";

import { renderToStaticMarkup } from "react-dom/server";
import {
Expand Down Expand Up @@ -509,6 +511,111 @@ describe("@preact/signals-react updating", () => {
expect(scratch.innerHTML).to.equal("<div>1 1</div>");
});

it("should minimize rerenders when passing signals through context", async () => {
function spyOn<P = { children?: React.ReactNode }>(
c: FunctionComponent<P>
) {
return sinon.spy(c);
}

// Manually read signal value below so we can watch whether components rerender
const Origin = spyOn(function Origin() {
const origin = useContext(URLModelContext).origin;
return <span>{origin.value}</span>;
});

const Pathname = spyOn(function Pathname() {
const pathname = useContext(URLModelContext).pathname;
return <span>{pathname.value}</span>;
});

const Search = spyOn(function Search() {
const search = useContext(URLModelContext).search;
return <span>{search.value}</span>;
});

// Never reads signal value during render so should never rerender
const UpdateURL = spyOn(function UpdateURL() {
const update = useContext(URLModelContext).update;
return (
<button
onClick={() => {
update(newURL => {
newURL.search = newURL.search === "?a=1" ? "?a=2" : "?a=1";
});
}}
>
update
</button>
);
});

interface URLModel {
origin: ReadonlySignal<string>;
pathname: ReadonlySignal<string>;
search: ReadonlySignal<string>;
update(updater: (newURL: URL) => void): void;
}

// Also never reads signal value during render so should never rerender
const URLModelContext = createContext<URLModel>(null as any);
const URLModelProvider = spyOn(function SignalProvider({ children }) {
const url = useSignal(new URL("https://domain.com/test?a=1"));
const modelRef = useRef<URLModel | null>(null);

if (modelRef.current == null) {
modelRef.current = {
origin: computed(() => url.value.origin),
pathname: computed(() => url.value.pathname),
search: computed(() => url.value.search),
update(updater) {
const newURL = new URL(url.value);
updater(newURL);
url.value = newURL;
},
};
}

return (
<URLModelContext.Provider value={modelRef.current}>
{children}
</URLModelContext.Provider>
);
});

function App() {
return (
<URLModelProvider>
<p>
<Origin />
<Pathname />
<Search />
</p>
<UpdateURL />
</URLModelProvider>
);
}

await render(<App />);

const url = scratch.querySelector("p")!;
expect(url.textContent).to.equal("https://domain.com/test?a=1");
expect(URLModelProvider).to.be.calledOnce;
expect(Origin).to.be.calledOnce;
expect(Pathname).to.be.calledOnce;
expect(Search).to.be.calledOnce;

await act(() => {
scratch.querySelector("button")!.click();
});

expect(url.textContent).to.equal("https://domain.com/test?a=2");
expect(URLModelProvider).to.be.calledOnce;
expect(Origin).to.be.calledOnce;
expect(Pathname).to.be.calledOnce;
expect(Search).to.be.calledTwice;
});

it("should not subscribe to computed signals only created and not used", async () => {
const sig = signal(0);
const childSpy = sinon.spy();
Expand Down

0 comments on commit de68f7c

Please sign in to comment.