From 07fa8c2225c793e4bc69f7f65155244c390585b7 Mon Sep 17 00:00:00 2001 From: Brenley Dueck Date: Sun, 1 Nov 2026 11:15:15 -0600 Subject: [PATCH] perf(web): resolve condition memos inline in a spread's collect A spread's attribute effect reruns every getter it reads, so a compiled `memo(() => !!cond)()` inside one was rebuilt on each run. Outside hydration it now resolves inline, and the condition's inputs become dependencies of the attribute effect. --- .changeset/spread-view-layer-effects.md | 7 +++ packages/web/src/client.ts | 67 +++++++++++++++---------- packages/web/src/render.ts | 12 +++++ packages/web/test/spread-nodes.spec.tsx | 63 ++++++++++++++++++++++- 4 files changed, 122 insertions(+), 27 deletions(-) create mode 100644 .changeset/spread-view-layer-effects.md diff --git a/.changeset/spread-view-layer-effects.md b/.changeset/spread-view-layer-effects.md new file mode 100644 index 0000000000..b69aed70c1 --- /dev/null +++ b/.changeset/spread-view-layer-effects.md @@ -0,0 +1,7 @@ +--- +"@solidjs/web": patch +--- + +perf(web): condition memos in a spread's getters resolve inline + +A spread's attribute effect reruns every getter it reads, so a compiled condition memo inside one of those getters (`memo(() => !!cond)()`) was rebuilt on each run. Outside hydration it now resolves inline, and the condition's inputs become dependencies of the attribute effect. On the polymorphic chain bench, update goes from 6.2 to 5.2 ms with mount unchanged. diff --git a/packages/web/src/client.ts b/packages/web/src/client.ts index ef2b6ca6ad..03630bbb93 100644 --- a/packages/web/src/client.ts +++ b/packages/web/src/client.ts @@ -35,7 +35,15 @@ import { SOURCE_PROXY, SOURCE_MEMO } from "solid-js/internal"; -import { effect, memo, setSpreadName, spreadLoc, spreadName, tagElement } from "./render.js"; +import { + effect, + memo, + setCollector, + setSpreadName, + spreadLoc, + spreadName, + tagElement +} from "./render.js"; import { JSX } from "../jsx/jsx.js"; @@ -1225,32 +1233,39 @@ export function spread(node, props, skipChildren, skip, name, loc) { return prevProps; } effect(() => { - const source = resolveSource(props); - const newProps = {}; - // A merge() proxy is read through its SOURCES, not through the proxy: a - // spread mixed with other attributes compiles to - // `spread(el, merge(statics, () => rest))`, and going through the proxy - // costs merge's `keys()` (a Set plus an own-enumerable scan of every - // source) and then, per key, a right-to-left `in` walk of the sources. - // The union of own string keys with later sources overriding earlier - // — Object.assign order, merge's own contract — is all a spread needs. - // An omit() proxy likewise is read through its VIEW RECORD — its source - // walked directly with the hidden keys filtered — never through its - // traps (a descriptor trap per key, allocating, on every rerun). - // - // A view over plain objects only has a RESOLVED TABLE — key → owning - // leaf, shadowing already applied — built once; on every rerun this - // effect then does exactly what it did over an eager copy: one read per - // key, no re-enumeration and no per-key walk of the later sources. - const table = resolvedTable(source); - if (table !== undefined) return collectTable(newProps, table, skip); - if (source != null) { - const view = viewOf(source); - if (view instanceof OmitView) collectProps(newProps, view, SOURCE_OMIT, skip); - else if (view !== undefined) collectSources(newProps, view.sources, view.kinds, skip); - else collectProps(newProps, source, $PROXY in source ? SOURCE_PROXY : SOURCE_PLAIN, skip); + // Condition memos the getters create resolve inline (see `setCollector`); + // hydration keeps them, for their ids. + const outer = setCollector(sharedConfig.hydrating ? null : getOwner()); + try { + const source = resolveSource(props); + const newProps = {}; + // A merge() proxy is read through its SOURCES, not through the proxy: a + // spread mixed with other attributes compiles to + // `spread(el, merge(statics, () => rest))`, and going through the proxy + // costs merge's `keys()` (a Set plus an own-enumerable scan of every + // source) and then, per key, a right-to-left `in` walk of the sources. + // The union of own string keys with later sources overriding earlier + // — Object.assign order, merge's own contract — is all a spread needs. + // An omit() proxy likewise is read through its VIEW RECORD — its source + // walked directly with the hidden keys filtered — never through its + // traps (a descriptor trap per key, allocating, on every rerun). + // + // A view over plain objects only has a RESOLVED TABLE — key → owning + // leaf, shadowing already applied — built once; on every rerun this + // effect then does exactly what it did over an eager copy: one read per + // key, no re-enumeration and no per-key walk of the later sources. + const table = resolvedTable(source); + if (table !== undefined) return collectTable(newProps, table, skip); + if (source != null) { + const view = viewOf(source); + if (view instanceof OmitView) collectProps(newProps, view, SOURCE_OMIT, skip); + else if (view !== undefined) collectSources(newProps, view.sources, view.kinds, skip); + else collectProps(newProps, source, $PROXY in source ? SOURCE_PROXY : SOURCE_PLAIN, skip); + } + return newProps; + } finally { + setCollector(outer); } - return newProps; }, apply); if ("_SOLID_DEV_") unscopedByDesign = node; if (!skipChildren && !(skip !== undefined && skip("children"))) { diff --git a/packages/web/src/render.ts b/packages/web/src/render.ts index 9c1863516e..35bd140c1d 100644 --- a/packages/web/src/render.ts +++ b/packages/web/src/render.ts @@ -92,5 +92,17 @@ export function effect( // onto the parent's live counter and breaks async-hole parity (pinned by the // async-cond-before-for harness scenario). export function memo(fn: () => T): () => T { + if (collector !== null && getOwner() === collector) return fn; return createMemo(() => fn(), syncOptions); } + +// The spread effect collecting its props right now (client.ts `spread`), set +// outside hydration only. A condition memo created directly under it — a +// compiled getter's `memo(() => !!cond)()` — would be rebuilt on every rerun, +// and the effect already reruns every getter, so it resolves inline instead. +let collector: unknown = null; +export function setCollector(owner: unknown): unknown { + const outer = collector; + collector = owner; + return outer; +} diff --git a/packages/web/test/spread-nodes.spec.tsx b/packages/web/test/spread-nodes.spec.tsx index 11f5ecd7a8..7b31c1d5c6 100644 --- a/packages/web/test/spread-nodes.spec.tsx +++ b/packages/web/test/spread-nodes.spec.tsx @@ -12,7 +12,7 @@ * inline (no memo, no hydration id) and honors a `skip` predicate. */ import { describe, expect, test, vi } from "vitest"; -import { render, spread } from "@solidjs/web"; +import { memo, render, spread } from "@solidjs/web"; import { createRoot, createSignal, createStore, flush, getOwner, merge, omit } from "solid-js"; const mount = (el: () => any) => { @@ -470,3 +470,64 @@ describe("existing contracts", () => { m.dispose(); }); }); + +describe("compiled condition getters in a spread", () => { + // What the compiler emits for `type={native() ? p.type : undefined}` on a + // component, reached through merge/omit layers and spread onto the element. + function conditionProps() { + const [as, setAs] = createSignal("button"); + const [kind, setKind] = createSignal("submit"); + const [label, setLabel] = createSignal("a"); + const native = () => as() === "button"; + const props = merge( + { type: "button" }, + { + get type() { + return memo(() => !!native())() ? kind() : undefined; + }, + get title() { + return label(); + } + } + ); + return { props, setAs, setKind, setLabel }; + } + + test("resolve inline: reruns leave no memo under the attribute effect", () => { + const { props, setLabel } = conditionProps(); + let owner: any; + const dispose = createRoot(d => { + owner = getOwner(); + spread(document.createElement("button"), props); + return d; + }); + flush(); + setLabel("b"); + flush(); + expect(ownerTotal(owner)).toBe(1); + dispose(); + }); + + test("still track the condition and the chosen branch", () => { + const { props, setAs, setKind, setLabel } = conditionProps(); + const m = mount(() => { + const el = document.createElement("button"); + spread(el, props); + return el; + }); + expect(m.el().getAttribute("type")).toBe("submit"); + setKind("reset"); + flush(); + expect(m.el().getAttribute("type")).toBe("reset"); + setAs("a"); + flush(); + expect(m.el().hasAttribute("type")).toBe(false); + setLabel("b"); + flush(); + expect(m.el().title).toBe("b"); + setAs("button"); + flush(); + expect(m.el().getAttribute("type")).toBe("reset"); + m.dispose(); + }); +});