Skip to content

Commit

Permalink
Made <Match> conditions only evaluate when needed
Browse files Browse the repository at this point in the history
This removes a bug where the `when` condition of a `<Match>` was evaluated twice immediately after creation, when the condition was true, children was a function and keyed was not specified. It also removes any unnecessary conditions evaluations by creating a memo on every `when` in a `Switch`.

For example, if a `<Switch>` has two `<Match>`es with `when={a()}` and `when={b()}` respectively, then:
- `b()` is never called if `a()` is truthy (which was true also before this change),
- `a()` is never called when `b()` changes (which is new).

solidjs#2406
  • Loading branch information
TPReal committed Jan 24, 2025
1 parent c0cc51f commit f2a1e9d
Show file tree
Hide file tree
Showing 2 changed files with 151 additions and 33 deletions.
76 changes: 43 additions & 33 deletions packages/solid/src/render/flow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
Accessor,
Setter,
onCleanup,
MemoOptions,
IS_DEV
} from "../reactive/signal.js";
import { mapArray, indexArray } from "../reactive/array.js";
Expand Down Expand Up @@ -149,7 +148,7 @@ export function Show<T>(props: {
) as unknown as JSX.Element;
}

type EvalConditions = readonly [number, unknown?, MatchProps<unknown>?];
type EvalConditions = readonly [number, Accessor<unknown>, MatchProps<unknown>];

/**
* Switches between content based on mutually exclusive conditions
Expand All @@ -166,47 +165,58 @@ type EvalConditions = readonly [number, unknown?, MatchProps<unknown>?];
* @description https://docs.solidjs.com/reference/components/switch-and-match
*/
export function Switch(props: { fallback?: JSX.Element; children: JSX.Element }): JSX.Element {
let keyed = false;
const equals: MemoOptions<EvalConditions>["equals"] = (a, b) =>
(keyed ? a[1] === b[1] : !a[1] === !b[1]) && a[2] === b[2];
const conditions = children(() => props.children) as unknown as () => MatchProps<unknown>[],
evalConditions = createMemo(
(): EvalConditions => {
let conds = conditions();
if (!Array.isArray(conds)) conds = [conds];
for (let i = 0; i < conds.length; i++) {
const c = conds[i].when;
if (c) {
keyed = !!conds[i].keyed;
return [i, c, conds[i]];
}
}
return [-1];
},
undefined,
IS_DEV ? { equals, name: "eval conditions" } : { equals }
);
const chs = children(() => props.children);
const switchFunc = createMemo(() => {
const ch = chs() as unknown as MatchProps<unknown> | MatchProps<unknown>[];
const mps = Array.isArray(ch) ? ch : [ch];
let func: Accessor<EvalConditions | undefined> = () => undefined;
for (let i = 0; i < mps.length; i++) {
const index = i;
const mp = mps[i];
const prevFunc = func;
const conditionValue = createMemo(
() => (prevFunc() ? undefined : mp.when),
undefined,
IS_DEV ? { name: "condition value" } : undefined
);
const condition = mp.keyed
? conditionValue
: createMemo(
conditionValue,
undefined,
IS_DEV
? {
equals: (a, b) => !a === !b,
name: "condition"
}
: { equals: (a, b) => !a === !b }
);
func = () => prevFunc() || (condition() ? [index, conditionValue, mp] : undefined);
}
return func;
});
return createMemo(
() => {
const [index, when, cond] = evalConditions();
if (index < 0) return props.fallback;
const c = cond!.children;
const fn = typeof c === "function" && c.length > 0;
const sel = switchFunc()();
if (!sel) return props.fallback;
const [index, conditionValue, mp] = sel;
const child = mp.children;
const fn = typeof child === "function" && child.length > 0;
return fn
? untrack(() =>
(c as any)(
keyed
? when
(child as any)(
mp.keyed
? (conditionValue() as any)
: () => {
if (untrack(evalConditions)[0] !== index) throw narrowedError("Match");
return cond!.when;
if (untrack(switchFunc)()?.[0] !== index) throw narrowedError("Match");
return conditionValue();
}
)
)
: c;
: child;
},
undefined,
IS_DEV ? { name: "value" } : undefined
IS_DEV ? { name: "eval conditions" } : undefined
) as unknown as JSX.Element;
}

Expand Down
108 changes: 108 additions & 0 deletions packages/solid/web/test/switch.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,114 @@ describe("Testing non-keyed function handler Switch control flow", () => {
test("dispose", () => disposer());
});

describe("Testing Switch conditions evaluation counts", () => {
let div!: HTMLDivElement, disposer: () => void;
function makeCondition() {
const [get, set] = createSignal(0);
const result = {
get,
set,
evalCount: 0,
getAndCount: () => {
result.evalCount++;
return get();
}
};
return result;
}
const a = makeCondition(),
b = makeCondition(),
c = makeCondition();
const Component = () => (
<div ref={div}>
<Switch fallback={"fallback"}>
<Match when={a.getAndCount()}>a={a.get()}</Match>
<Match when={b.getAndCount()}>{b => <>b={b()}</>}</Match>
<Match when={c.getAndCount()} keyed>
{c => <>c={c}</>}
</Match>
</Switch>
</div>
);

test("Create Switch control flow", () => {
createRoot(dispose => {
disposer = dispose;
<Component />;
});

expect(div.innerHTML).toBe("fallback");
expect(a.evalCount).toBe(1);
expect(b.evalCount).toBe(1);
expect(c.evalCount).toBe(1);
});

test("Toggle conditions", () => {
c.set(5);
expect(div.innerHTML).toBe("c=5");
expect(a.evalCount).toBe(1);
expect(b.evalCount).toBe(1);
expect(c.evalCount).toBe(2);
a.set(1);
expect(div.innerHTML).toBe("a=1");
expect(a.evalCount).toBe(2);
expect(b.evalCount).toBe(1);
expect(c.evalCount).toBe(2);
b.set(3);
expect(div.innerHTML).toBe("a=1");
expect(a.evalCount).toBe(2);
expect(b.evalCount).toBe(1); // did not evaluate
expect(c.evalCount).toBe(2);
b.set(2);
expect(div.innerHTML).toBe("a=1");
expect(a.evalCount).toBe(2);
expect(b.evalCount).toBe(1); // did not evaluate
expect(c.evalCount).toBe(2);
a.set(0);
expect(div.innerHTML).toBe("b=2");
expect(a.evalCount).toBe(3);
expect(b.evalCount).toBe(2); // evaluated now
expect(c.evalCount).toBe(2);
b.set(3);
expect(div.innerHTML).toBe("b=3");
expect(a.evalCount).toBe(3);
expect(b.evalCount).toBe(3);
expect(c.evalCount).toBe(2);
c.set(3);
expect(div.innerHTML).toBe("b=3");
expect(a.evalCount).toBe(3);
expect(b.evalCount).toBe(3);
expect(c.evalCount).toBe(2); // did not evaluate
a.set(1);
expect(div.innerHTML).toBe("a=1");
expect(a.evalCount).toBe(4);
expect(b.evalCount).toBe(3);
expect(c.evalCount).toBe(2);
b.set(1);
expect(div.innerHTML).toBe("a=1");
expect(a.evalCount).toBe(4);
expect(b.evalCount).toBe(3); // did not evaluate
expect(c.evalCount).toBe(2);
b.set(0);
expect(div.innerHTML).toBe("a=1");
expect(a.evalCount).toBe(4);
expect(b.evalCount).toBe(3); // did not evaluate
expect(c.evalCount).toBe(2);
a.set(0);
expect(div.innerHTML).toBe("c=3");
expect(a.evalCount).toBe(5);
expect(b.evalCount).toBe(4); // evaluated now, as b changed since its last evaluation
expect(c.evalCount).toBe(3); // evaluated now
c.set(0);
expect(div.innerHTML).toBe("fallback");
expect(a.evalCount).toBe(5);
expect(b.evalCount).toBe(4);
expect(c.evalCount).toBe(4);
});

test("dispose", () => disposer());
});

describe("Testing non-keyed function handler Switch control flow with dangling callback", () => {
let div!: HTMLDivElement, disposer: () => void;
const [a, setA] = createSignal(0),
Expand Down

0 comments on commit f2a1e9d

Please sign in to comment.