diff --git a/.changeset/spread-view-layer-effects.md b/.changeset/spread-view-layer-effects.md new file mode 100644 index 000000000..b69aed70c --- /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 ef2b6ca6a..03630bbb9 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 9c1863516..35bd140c1 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 11f5ecd7a..7b31c1d5c 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(); + }); +});