From a4a0e16e9ad937c8cf7a763a53bd479588ba3188 Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Mon, 14 Sep 2026 23:46:10 -0700 Subject: [PATCH 1/3] perf(signals,web,universal,html): merge/omit are always lazy views; consumers read the leaves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit merge() and omit() never copy under Proxy. omit() returns a view record for every input (plain objects included, predicate filters supported); merge() returns an O(1) view over its flattened sources, or the single non-function source itself. omit-over-merge carries one filtered leaf view per source, merge-over-omit takes the record as a leaf, nested omits fold their filters: the headless-UI chain merge → omit → merge → omit collapses to leaf views over the author's objects with no proxy layer between spread and props. Reads: a view over plain leaves resolves a key → owning-leaf table on first read (merged key order), after which get/has/descriptor are one lookup. spread() (web, universal) and ssrElement() read leaves directly and walk the table when present, so an effect rerun is one read per key as it was over the eager copy. Views over a store, a memo source, or any `$PROXY in s` proxy (frames slot props) keep the `in` walk. Truthful descriptors: getOwnPropertyDescriptor through any depth reports a data descriptor only for a data property of a plain leaf, an accessor for a getter, store key, or memo source. hasStaticKeys() says when a key set is fixed; spread() uses both to skip the children effect for static children behind omit/merge layers (#3388 through views). @solidjs/html built props by assigning onto merge()'s result; it now collects its own props and merges once at the end (#3384). Signals-layer chain (depth 1/3/7) vs next: build 7.4/29/131 µs → 2.2/6.4/19; build+reads 8.3/31/95 → 2.5/7.4/20. SSR polymorphic-chain 200 rows ~2.4× faster; DOM update path at parity. Co-authored-by: Claude via Cursor Co-authored-by: Cursor --- .changeset/merge-omit-lazy-views.md | 27 + packages/html/src/tagged-jsx.ts | 20 +- packages/signals/src/store/index.ts | 11 +- packages/signals/src/store/utils.ts | 620 +++++++++++++++--- .../tests/store/utilities-no-proxy.test.ts | 161 +++++ .../signals/tests/store/utilities.test.ts | 341 +++++++++- packages/solid/src/index.ts | 6 + packages/solid/src/server/index.ts | 6 + packages/solid/src/server/signals.ts | 14 +- packages/universal/src/universal.ts | 95 ++- .../universal/test/spread-sources.spec.js | 53 +- packages/web/src/client.ts | 113 +++- packages/web/src/server.ts | 90 ++- .../test/server/ssr-element-sources.spec.tsx | 79 ++- packages/web/test/spread-nodes.spec.tsx | 23 +- packages/web/test/spread-sources.spec.tsx | 93 ++- 16 files changed, 1522 insertions(+), 230 deletions(-) create mode 100644 .changeset/merge-omit-lazy-views.md create mode 100644 packages/signals/tests/store/utilities-no-proxy.test.ts diff --git a/.changeset/merge-omit-lazy-views.md b/.changeset/merge-omit-lazy-views.md new file mode 100644 index 000000000..dcca1f2ca --- /dev/null +++ b/.changeset/merge-omit-lazy-views.md @@ -0,0 +1,27 @@ +--- +"@solidjs/signals": patch +"solid-js": patch +"@solidjs/web": patch +"@solidjs/universal": patch +"@solidjs/html": patch +--- + +`merge()` and `omit()` are always lazy views, and props consumers read their leaves + +`omit(props, ...keys)` returns a live view of `props` for every input — a plain object included — instead of copying it with a `getOwnPropertyDescriptor` + `defineProperty` per prop. A predicate form hides keys by rule without enumerating first: `omit(props, k => k[0] === "$")`. `merge()` no longer builds an eager copy when its sources are plain objects: under `Proxy` it always returns an O(1) view over the flattened sources (a single non-function source is returned as is). + +The two compose flat. An `omit()` over a `merge()` carries one filtered view per flattened merge source, a `merge()` over an `omit()` takes the view record as a leaf, and nested omits fold their filters into one record. A component chain of `merge(defaults) → omit(consumed) → merge(statics) → omit("as")` — the shape headless-UI libraries render every element through — collapses to leaf views over the original objects, each with its accumulated filter, with no proxy layer left between the outermost spread and the author's props. `merge()` keeps the omitted keys hidden by construction (#3014) rather than by treating the omit as opaque. Construction cost drops 3–7× at depth 1–7; the SSR polymorphic-chain bench (#3448) runs ~2.4× faster. + +Reads stay cheap: a view over plain objects resolves a key → owning-leaf table once, on first read, and every `get`/`has`/descriptor is one lookup after that. `spread()` (DOM and universal) and `ssrElement()` read the leaves directly — never through the proxies' traps — and walk that table when there is one, so an effect rerun costs one read per key, as it did over the copy. Both proxies use a class target and one shared handler (no per-instance closures). + +The views tell the truth: `Object.getOwnPropertyDescriptor(view, key)` reports a data descriptor only when the key is a data property of a plain leaf (the compiler's encoding of a static prop) and an accessor for a getter, a store key, or a memo source. Together with the new internal `hasStaticKeys()`, `spread()` now skips the children effect for static children behind `omit`/`merge` layers (#3388 through views). + +Behavior changes: + +- Writes to a `merge()` or `omit()` result are no-ops (they already were for the proxy forms). A caller that needs its own object copies it (`{ ...merged }`), and the copy carries no sources (#3384). `@solidjs/html` now collects its own props and spreads into one `merge()` at the end instead of assigning onto the result. +- A data property on a source is read live through the view rather than snapshotted at `merge()`/`omit()` time. +- Key order of a merged view is the merged order — every key at the position of the last source that carries it — matching `ssrElement`'s array form. +- Sources are treated as own-keyed; a key added to a plain source after merging is not seen (the copy did not see it either). +- Enumerating a view through its traps (`for…in`, `Object.keys`, `{ ...view }`) costs a trap per key, as any proxy does; the internal consumers avoid it. Environments without `Proxy` keep the copy paths. + +Internal helpers for consumers, exported from `solid-js`: `omitView(o)`, `sourceKeys(entry)`, `sourceHas(entry, key)`, `sourceGet(entry, key)`, `hasStaticKeys(o)`, `resolvedTable(o)`. diff --git a/packages/html/src/tagged-jsx.ts b/packages/html/src/tagged-jsx.ts index a3f45a36e..a2d3d0fe1 100644 --- a/packages/html/src/tagged-jsx.ts +++ b/packages/html/src/tagged-jsx.ts @@ -219,35 +219,43 @@ function createHtml() { components: ComponentRegistry, props: Record = {} ) => { + // A merge() result is a read-only view — writes to it are no-ops — so + // own props are collected into plain objects and the spreads interleaved + // as sources, merged once at the end in source order (later wins). + const sources: unknown[] = []; + let own: Record = props; for (const prop of node.props) { switch (prop.type) { case BOOLEAN_PROP: - props[prop.name] = true; + own[prop.name] = true; break; case STATIC_PROP: - props[prop.name] = prop.value; + own[prop.name] = prop.value; break; case EXPRESSION_PROP: - applyGetter(props, prop.name, values[prop.value]); + applyGetter(own, prop.name, values[prop.value]); break; case SPREAD_PROP: const spreadValue = values[prop.value]; if (!spreadValue || typeof spreadValue !== "object") throw new Error("Can only spread objects"); - props = mergeProps(props, spreadValue); + sources.push(own, spreadValue); + own = {}; break; } } // children - childNodes overwrites any props.children if (node.type === COMPONENT_NODE && node.children.length) { - Object.defineProperty(props, "children", { + Object.defineProperty(own, "children", { get() { return renderChildren(node, values, components); } }); } - return props; + if (sources.length === 0) return own; + sources.push(own); + return mergeProps(...sources) as Record; }; const applyGetter = (props: Record, name: string, value: any) => { diff --git a/packages/signals/src/store/index.ts b/packages/signals/src/store/index.ts index 86d7789fa..e48327460 100644 --- a/packages/signals/src/store/index.ts +++ b/packages/signals/src/store/index.ts @@ -12,7 +12,16 @@ export type { export type { Merge, Omit } from "./utils.js"; export { isWrappable, $TRACK, $PROXY, $TARGET } from "./store.js"; -export { mergeSources } from "./utils.js"; +export { + mergeSources, + omitView, + sourceKeys, + sourceHas, + sourceGet, + hasStaticKeys, + resolvedTable, + OmitView +} from "./utils.js"; import type { NoFn, ProjectionOptions, Store, StoreOptions, StoreSetter } from "./store.js"; import type { Refreshable } from "../core/index.js"; diff --git a/packages/signals/src/store/utils.ts b/packages/signals/src/store/utils.ts index b8711d8d2..c9e93083d 100644 --- a/packages/signals/src/store/utils.ts +++ b/packages/signals/src/store/utils.ts @@ -7,37 +7,6 @@ function trueFn() { return true; } -const propTraps: ProxyHandler<{ - get: (k: string | number | symbol) => any; - has: (k: string | number | symbol) => boolean; - keys: () => (string | symbol)[]; -}> = { - get(_, property, receiver) { - if (property === $PROXY) return receiver; - return _.get(property); - }, - has(_, property) { - if (property === $PROXY) return true; - return _.has(property); - }, - set: trueFn, - deleteProperty: trueFn, - getOwnPropertyDescriptor(_, property) { - return { - configurable: true, - enumerable: true, - get() { - return _.get(property); - }, - set: trueFn, - deleteProperty: trueFn - }; - }, - ownKeys(_) { - return _.keys(); - } -}; - type DistributeOverride = T extends undefined ? F : T; type Override = T extends any ? U extends any @@ -80,13 +49,435 @@ function resolveSource(s: any) { } const $SOURCES = Symbol(__DEV__ ? "MERGE_SOURCE" : 0); -/** @internal The flattened sources behind a `merge()` PROXY, or undefined. - * Only the proxy form carries sources: its writes are no-ops, so they are - * the whole truth. The plain-object form is a real object callers may copy - * (descriptor copies, `{...props}`) or mutate afterwards (html's tagged - * templates assign props and a children getter after spreading) — what is - * on the object is the truth there, so it records nothing and every consumer, - * a nested merge included, reads it directly (#3384). */ +const $OMIT = Symbol(__DEV__ ? "OMIT_VIEW" : 0); + +/** @internal The record behind an `omit()` proxy: `source` with `hidden` + * keys removed. It is the proxy's TARGET, so the shared handler reads it as + * plain fields — no per-instance closures — and it is what props consumers + * walk directly (`merge`, `spread`, `ssrElement`): a view never materializes + * a copy, and a consumer that knows the record never goes through its traps + * (a `getOwnPropertyDescriptor` trap per key allocates a descriptor and a + * getter, so enumerating a proxy costs more than the copy it was avoiding). + * `hidden` is a key list or a predicate (`omit(props, k => k[0] === "$")`). + * + * When `source` is a `merge()` proxy the view also carries `entries`: one + * leaf view per flattened merge source, same filter. That is what the proxy + * answers `$SOURCES` with, so a consumer — `merge()` re-merging it, a spread, + * `ssrElement` — flattens `omit(merge(a, b))` to `[a', b']` and a component + * chain of defaults + omit + spread (`merge(omit(merge(omit(props))))`) + * collapses to the leaf objects, each with its accumulated filter, with no + * trap round-trip per layer. A leaf may be merge's memo for a function + * source; it is resolved on access. */ +export class OmitView { + /** see `resolvedTable` */ + table: Map | null | undefined = undefined; + constructor( + public source: any, + public hidden: Hidden, + public entries?: OmitView[] + ) {} +} + +type Hidden = PropertyKey[] | ((key: PropertyKey) => boolean); + +function isHidden(view: OmitView, key: PropertyKey): boolean { + const h = view.hidden; + return typeof h === "function" ? h(key) : h.includes(key); +} + +// Both filters as one. Two key lists stay a key list (one `includes`, no +// closure); a predicate on either side needs a closure. +function combineHidden(a: Hidden, b: Hidden): Hidden { + if (typeof a !== "function" && typeof b !== "function") return a.concat(b); + return key => + (typeof a === "function" ? a(key) : a.includes(key)) || + (typeof b === "function" ? b(key) : b.includes(key)); +} + +const EMPTY = Object.freeze({}); +// The object a view filters, with a merge memo leaf resolved (tracked, as +// merge's own reads are) and a nullish result read as no keys. +function viewSource(view: OmitView): any { + let s = view.source; + if (typeof s === "function") s = s(); + return s == null ? EMPTY : s; +} + +/** @internal The `OmitView` behind an `omit()` proxy, or undefined. */ +export function omitView(o: any): OmitView | undefined { + return o != null && o[$PROXY] === o ? o[$OMIT] : undefined; +} + +// A props SOURCE ENTRY is a plain object, a proxy (store, merge, omit — the +// last two are normally unwrapped first: `mergeSources` / `omitView`), or an +// `OmitView` record. These three answer for an entry what `Object.keys` / +// `in` / `[]` answer for an object, so every consumer walks entries with one +// code path and an `OmitView` is filtered rather than materialized. + +/** @internal Own string keys of a source entry — every consumer skips symbols + * itself. A proxy answers through ONE `ownKeys` trap (a store's keeps the key + * set tracked); `Object.keys` on a proxy would add a descriptor trap per key. */ +export function sourceKeys(s: any): (string | symbol)[] { + if (s instanceof OmitView) { + const keys = sourceKeys(viewSource(s)); + const out: (string | symbol)[] = []; + for (let i = 0; i < keys.length; i++) if (!isHidden(s, keys[i])) out.push(keys[i]); + return out; + } + return s[$PROXY] === s ? Reflect.ownKeys(s) : Object.keys(s); +} + +/** @internal `key in entry`. */ +export function sourceHas(s: any, key: PropertyKey): boolean { + return s instanceof OmitView ? !isHidden(s, key) && sourceHas(viewSource(s), key) : key in s; +} + +/** @internal `entry[key]` — the source's getter runs once, here. */ +export function sourceGet(s: any, key: PropertyKey): any { + return s instanceof OmitView + ? isHidden(s, key) + ? undefined + : sourceGet(viewSource(s), key) + : s[key]; +} + +// A leaf whose own key set is fixed: a plain object. Not a store (its key +// set is a tracked signal), not a merge memo source (it swaps whole objects), +// and not any proxy that declares itself with `$PROXY in s` — a frames slot +// proxy answers `has` for every key and lists none, so only the `in` walk +// is right for it. +function leafHasStaticKeys(leaf: any): boolean { + if (leaf instanceof OmitView) leaf = leaf.source; + return typeof leaf !== "function" && !($PROXY in leaf); +} + +/** @internal Whether the own key set of a props object cannot change + * reactively: a plain object, or a merge/omit view over plain objects only. + * A consumer may then decide from `Object.getOwnPropertyDescriptor` once — + * "no `children` key" or "a data `children`" holds for the object's lifetime, + * so no tracking scope is needed for it (#3388). For a store, or a view with + * a store or memo leaf, keys can appear later and the reactive path is the + * only correct one. */ +export function hasStaticKeys(o: any): boolean { + if (!($PROXY in o)) return true; + const sources = o[$SOURCES]; + if (sources !== undefined) { + for (let i = 0; i < sources.length; i++) if (!leafHasStaticKeys(sources[i])) return false; + return true; + } + const view = o[$OMIT]; + return view !== undefined && leafHasStaticKeys(view); +} + +function accessorDescriptor(get: () => any, enumerable = true): PropertyDescriptor { + return { configurable: true, enumerable, get, set: trueFn }; +} + +/** The descriptor a consumer should see for `key` on an entry — + * the view proxies answer `getOwnPropertyDescriptor` with it, so it tells the + * truth through any depth of merge/omit layers. + * + * A DATA descriptor means "nothing reactive can hide behind this value": the + * key is a data property of a plain-object leaf — the compiler's own + * encoding of a static attribute. Everything else is an accessor: a getter + * on a leaf, a key on a store proxy (its "data" is a signal), or a key on a + * merge memo source (the whole object is reactive). That is what lets a + * consumer skip a reactive node for a static prop at the bottom of a + * component chain, and it is why the store case must NOT forward the store's + * own descriptor, which reports a value. + * + * `configurable: true` always — the target has no such property, and the + * Proxy invariants forbid reporting a non-configurable one. */ +function sourceDescriptor(s: any, key: PropertyKey): PropertyDescriptor | undefined { + if (s instanceof OmitView) { + if (isHidden(s, key)) return undefined; + const raw = s.source; + if (typeof raw === "function") { + return sourceHas(viewSource(s), key) + ? accessorDescriptor(() => sourceGet(s, key)) + : undefined; + } + return sourceDescriptor(raw, key); + } + if (s[$PROXY] === s) { + const desc = Reflect.getOwnPropertyDescriptor(s, key); + if (desc === undefined) return undefined; + // Another view (an omit's source may be a merge proxy) already answers + // truthfully; a store's reported "data" is a signal. + return s[$SOURCES] !== undefined || s[$OMIT] !== undefined + ? desc + : accessorDescriptor(() => s[key], desc.enumerable); + } + const desc = Reflect.getOwnPropertyDescriptor(s, key); + if (desc === undefined) return undefined; + if (desc.get !== undefined || desc.set !== undefined) + return accessorDescriptor(() => s[key], desc.enumerable); + // The proxy target has no such key, so the descriptor must be configurable; + // Reflect's is a fresh object, so a configurable one is handed out as is. + if (desc.configurable) return desc; + return { configurable: true, enumerable: desc.enumerable, writable: true, value: desc.value }; +} + +// Own ENUMERABLE keys, symbols included, of an entry — the user-facing key +// set (`Object.keys(merged)`), where enumerability matters (#2769). +function sourceEnumerableKeys(s: any): (string | symbol)[] { + if (s instanceof OmitView) { + const keys = sourceEnumerableKeys(viewSource(s)); + const out: (string | symbol)[] = []; + for (let i = 0; i < keys.length; i++) if (!isHidden(s, keys[i])) out.push(keys[i]); + return out; + } + return ownEnumerableKeys(s); +} + +// The target of a merge() proxy: the flattened sources, read by one shared +// handler — like OmitView, no per-instance closures. `sources` is what +// `$SOURCES` answers. +class MergeView { + /** key → the plain leaf that owns it (later sources win), built on first + * read when every leaf has static keys; `null` when one doesn't. */ + table: Map | null | undefined = undefined; + constructor(public sources: any[]) {} +} + +/** @internal The resolved key table of a merge/omit view — every own key of + * the view mapped to the plain object that owns it, in merged order (a key + * at the position of the last source that carries it, see `tableSet`) — or + * undefined when it has none: a leaf is a store or a memo source, whose + * keys can change, or the object is not a view at all. + * + * This is the flat object the eager copy used to build, made lazily and + * without copying: one pass over the leaves' own keys on first read, then + * every `get`/`has`/descriptor is one lookup plus one read of the owning + * leaf, and a consumer (`spread` rerunning its effect, `ssrElement`) walks + * the table instead of re-deriving shadowing from the leaves each time. Own + * keys only, as the copy's were: a plain source's key set is fixed once + * merged (keys added to it later are not seen — the copy didn't see them + * either). */ +export function resolvedTable(o: any): Map | undefined { + if (o == null || !($PROXY in o)) return undefined; + const view = o[$OMIT]; + if (view !== undefined) return omitTable(view); + const sources = o[$SOURCES]; + return sources === undefined ? undefined : mergeTable(mergeViewOf(o)); +} + +// The MergeView behind a merge proxy. `$SOURCES` answers the array; the +// record itself is reached through this symbol so the table can live on it. +const $VIEW = Symbol(__DEV__ ? "MERGE_VIEW" : 0); +function mergeViewOf(proxy: any): MergeView { + return proxy[$VIEW]; +} + +function mergeTable(view: MergeView): Map | undefined { + let table = view.table; + if (table === undefined) { + const f = view.sources; + for (let i = 0; i < f.length; i++) { + if (!leafHasStaticKeys(f[i])) { + view.table = null; + return undefined; + } + } + table = new Map(); + for (let i = 0; i < f.length; i++) { + const leaf = f[i]; + if (leaf instanceof OmitView) { + const src = leaf.source; + const keys = Reflect.ownKeys(src); + for (let j = 0; j < keys.length; j++) { + const key = keys[j]; + if (!isHidden(leaf, key)) tableSet(table, key, src); + // A key this leaf hides that an EARLIER leaf owned must stay: the + // filter applies to this leaf's contribution, not to the merge. + } + } else { + const keys = Reflect.ownKeys(leaf); + for (let j = 0; j < keys.length; j++) tableSet(table, keys[j], leaf); + } + } + view.table = table; + } + return table === null ? undefined : table; +} + +// Key order is the merged one — every key at the position of the LAST source +// that carries it — the order `ssrElement`'s array form serializes in and the +// eager copy enumerated in, so a spread through a view and a spread over the +// sources emit the same attribute order. +function tableSet(table: Map, key: PropertyKey, leaf: any) { + if (table.has(key)) table.delete(key); + table.set(key, leaf); +} + +// An omit view's table: its source's (a merge's table, or a plain object's +// own keys) minus the hidden keys. Cached on the record. +function omitTable(view: OmitView): Map | undefined { + let table = view.table; + if (table === undefined) { + const src = view.source; + let base: Map | undefined; + if (typeof src === "function" || src == null) base = undefined; + else if ($PROXY in src) base = resolvedTable(src); + else { + base = new Map(); + const keys = Reflect.ownKeys(src); + for (let j = 0; j < keys.length; j++) base.set(keys[j], src); + } + if (base === undefined) { + view.table = null; + return undefined; + } + table = new Map(); + for (const [key, leaf] of base) if (!isHidden(view, key)) table.set(key, leaf); + view.table = table; + } + return table === null ? undefined : table; +} + +// The user-facing key set of a resolved table: its keys that are enumerable +// on the leaf that owns them (`Object.keys(merged)`, #2769). +const propertyIsEnumerable = Object.prototype.propertyIsEnumerable; +function tableKeys(table: Map): (string | symbol)[] { + const out: (string | symbol)[] = []; + for (const [key, leaf] of table) + if (propertyIsEnumerable.call(leaf, key)) out.push(key as string | symbol); + return out; +} + +function mergeGet(view: MergeView, property: PropertyKey): any { + const table = mergeTable(view); + if (table !== undefined) { + const leaf = table.get(property); + return leaf === undefined ? undefined : leaf[property]; + } + const f = view.sources; + for (let i = f.length - 1; i >= 0; i--) { + const s = resolveSource(f[i]); + if (sourceHas(s, property)) return sourceGet(s, property); + } +} + +const mergeTraps: ProxyHandler = { + get(view, property, receiver) { + if (property === $PROXY) return receiver; + if (property === $SOURCES) return view.sources; + if (property === $VIEW) return view; + return mergeGet(view, property); + }, + has(view, property) { + if (property === $PROXY) return true; + if (property === $SOURCES || property === $VIEW) return false; + const table = mergeTable(view); + if (table !== undefined) return table.has(property); + const f = view.sources; + for (let i = f.length - 1; i >= 0; i--) { + if (sourceHas(resolveSource(f[i]), property)) return true; + } + return false; + }, + set: trueFn, + deleteProperty: trueFn, + getOwnPropertyDescriptor(view, property) { + if (property === $PROXY || property === $SOURCES || property === $VIEW) return undefined; + const table = mergeTable(view); + if (table !== undefined) { + const leaf = table.get(property); + return leaf === undefined ? undefined : sourceDescriptor(leaf, property); + } + const f = view.sources; + for (let i = f.length - 1; i >= 0; i--) { + const raw = f[i]; + const s = resolveSource(raw); + if (!sourceHas(s, property)) continue; + // A memo source (`merge(() => …)`) is reactive wholesale: whatever + // shape the memo's current object has, the key is an accessor here. + if (typeof raw === "function") return accessorDescriptor(() => mergeGet(view, property)); + // `in` also answers for inherited keys, which have no own descriptor. + return sourceDescriptor(s, property) ?? accessorDescriptor(() => mergeGet(view, property)); + } + return undefined; + }, + ownKeys(view) { + const table = mergeTable(view); + if (table !== undefined) return tableKeys(table); + // Same order as the table's: a key at the position of its last source. + const keys = new Set(); + const f = view.sources; + for (let i = 0; i < f.length; i++) { + const sourceKeys = sourceEnumerableKeys(resolveSource(f[i])); + for (let j = 0; j < sourceKeys.length; j++) { + const key = sourceKeys[j]; + if (keys.has(key)) keys.delete(key); + keys.add(key); + } + } + return [...keys]; + } +}; + +// Over a plain object an omit view reads its source directly — a hidden-key +// check and one property read, nothing to cache. Over a MERGE it answers from +// its resolved table when the merge has one (plain leaves only), so a read +// is one lookup rather than a hop through the merge proxy's traps; the table +// is the merge's, filtered, built once per view. +const omitTraps: ProxyHandler = { + get(view, property, receiver) { + if (property === $PROXY) return receiver; + if (property === $OMIT) return view; + // $SOURCES answers the FILTERED leaf entries (or nothing for a plain + // source) — never the underlying merge's own sources, which would hand a + // re-merge the unfiltered objects and leak the omitted keys (#3014). + if (property === $SOURCES) return view.entries; + if (view.entries !== undefined) { + const table = omitTable(view); + if (table !== undefined) { + const leaf = table.get(property); + return leaf === undefined ? undefined : leaf[property]; + } + } + if (isHidden(view, property)) return undefined; + return view.source[property]; + }, + has(view, property) { + if (property === $PROXY) return true; + if (property === $SOURCES || property === $OMIT) return false; + if (view.entries !== undefined) { + const table = omitTable(view); + if (table !== undefined) return table.has(property); + } + if (isHidden(view, property)) return false; + return property in view.source; + }, + set: trueFn, + deleteProperty: trueFn, + getOwnPropertyDescriptor(view, property) { + if (property === $PROXY || property === $OMIT || property === $SOURCES) return undefined; + if (view.entries !== undefined) { + const table = omitTable(view); + if (table !== undefined) { + const leaf = table.get(property); + return leaf === undefined ? undefined : sourceDescriptor(leaf, property); + } + } + return sourceDescriptor(view, property); + }, + ownKeys(view) { + if (view.entries !== undefined) { + const table = omitTable(view); + if (table !== undefined) return tableKeys(table); + } + const keys = Reflect.ownKeys(view.source); + const out: (string | symbol)[] = []; + for (let i = 0; i < keys.length; i++) if (!isHidden(view, keys[i])) out.push(keys[i]); + return out; + } +}; +/** @internal The flattened sources behind a `merge()` proxy, or undefined. + * A merge's writes are no-ops, so its sources are the whole truth. A COPY of + * a merge (`{...merged}`, a descriptor copy) is a plain object that carries + * no sources — `ownKeys` never answers $SOURCES — so what is on the copy is + * the truth there and every consumer reads it directly (#3384). */ export function mergeSources(o: any): any[] | undefined { return o != null && o[$PROXY] === o ? o[$SOURCES] : undefined; } @@ -98,6 +489,12 @@ export function mergeSources(o: any): any[] | undefined { * Function arguments are treated as memo-backed sources — useful for passing * derived defaults whose computation should track reactively. * + * The result is a live VIEW of its sources, never a copy: creating it costs + * nothing per key, every read goes to the source that owns the key (a getter + * runs there, a data property is read live), and writing to it is a no-op. + * A single non-function source is returned as is. To own a mutable object, + * copy it: `{ ...merged }` snapshots the current values. + * * Use this in component bodies to merge defaults / overrides without losing * Solid's per-property tracking. * @@ -112,59 +509,58 @@ export function mergeSources(o: any): any[] | undefined { */ export function merge(...sources: T): Merge { if (sources.length === 1 && typeof sources[0] !== "function") return sources[0] as any; - let proxy = false; const flattened: T[] = []; + // The one non-falsy source, if there is exactly one: it IS the merge. + let only: unknown = undefined; + let count = 0; for (let i = 0; i < sources.length; i++) { const s = sources[i]; + if (!s) continue; + count++; + only = s; if (typeof s === "function") { - proxy = true; flattened.push(createMemo(s as () => any) as any); continue; } - if (s && $PROXY in (s as object)) { - proxy = true; - // Only a merge() PROXY is flattened through: its writes are no-ops, so - // its sources are exactly what it reads. A plain-object merge result - // is an ordinary source — it may have been copied or mutated since, - // and what is on the object is the truth (#3384). omit() proxies - // answer $SOURCES with undefined on purpose (#3014). + if ($PROXY in (s as object)) { + // A merge() proxy is flattened through: its writes are no-ops, so its + // sources are exactly what it reads. An omit() proxy over a merge + // answers $SOURCES with its FILTERED leaf views, never the merge's own + // sources (#3014); an omit() of a plain object joins as its view + // record. Either way the filter travels with the entry and the hidden + // keys stay hidden. const childSources = (s as object)[$SOURCES]; if (childSources) { for (let j = 0; j < childSources.length; j++) flattened.push(childSources[j]); continue; } + const view = (s as object)[$OMIT]; + if (view) { + flattened.push(view); + continue; + } } flattened.push(s as any); } - if (SUPPORTS_PROXY && proxy) { - return new Proxy( - { - get(property: string | number | symbol) { - if (property === $SOURCES) return flattened; - for (let i = flattened.length - 1; i >= 0; i--) { - const s = resolveSource(flattened[i]); - if (property in s) return s[property]; - } - }, - has(property: string | number | symbol) { - for (let i = flattened.length - 1; i >= 0; i--) { - if (property in resolveSource(flattened[i])) return true; - } - return false; - }, - keys() { - const keys = new Set(); - for (let i = 0; i < flattened.length; i++) { - const sourceKeys = ownEnumerableKeys(resolveSource(flattened[i])); - for (let j = 0; j < sourceKeys.length; j++) keys.add(sourceKeys[j]); - } - return [...keys]; - } - }, - propTraps - ) as unknown as Merge; + if (SUPPORTS_PROXY) { + if (count === 1 && typeof only !== "function") return only as any; + // Always a view, never a copy. Building a plain object here costs a + // descriptor read, a bound getter and a defineProperty per key per + // layer, and component libraries stack several layers per element + // (defaults → omit → call-site statics → …), so the copies dominated + // their render cost while every consumer that matters — `spread`, + // `ssrElement`, a nested merge — reads the flattened sources directly + // anyway (#3448). The view is O(1) to create and reads through to the + // sources, so a data property on a source is read live, like a getter. + // Writes to the result are no-ops (a consumer that needs its own object + // copies: `{...merged}`, which the traps answer truthfully). Copies of + // the result never carry $SOURCES (#3384): `ownKeys` answers only the + // sources' keys. + return new Proxy(new MergeView(flattened), mergeTraps) as unknown as Merge; } + // No Proxy: an eager descriptor copy, semantics as close to the view as a + // plain object allows (getters stay live; data properties are snapshots). const defined: Record = Object.create(null); let nonTargetKey = false; let lastIndex = flattened.length - 1; @@ -214,6 +610,12 @@ export type Omit = { * Use it to forward "rest" props to a child element while pulling out the * keys your component handles itself — the equivalent of `splitProps(p, ["a","b"])[1]`. * + * The result is a live VIEW of `props`, not a copy: nothing is read or + * materialized until a key is used, and a spread (`{...rest}`) or a later + * `merge()` walks the underlying object directly. A predicate hides keys by + * rule instead of by name — `omit(props, k => k[0] === "$")` — without + * enumerating first. + * * @example * ```tsx * function Input(props: { label: string; value: string; onInput: (v: string) => void } & JSX.HTMLAttributes) { @@ -235,38 +637,54 @@ export type Omit = { export function omit, K extends readonly (keyof T)[]>( props: T, ...keys: K -): Omit { - if (SUPPORTS_PROXY && $PROXY in props) { - return new Proxy( - { - get(property) { - // $SOURCES must not tunnel through the filter: merge() flattens - // whatever answers it, so forwarding would hand a re-merge the - // UNFILTERED sources of an underlying merge proxy and the omitted - // keys leak back in (#3014 — the SSR element-spread path re-merges - // static attributes with the rest object). Opaque here: merge - // composes omit proxies through their traps instead. - return property === $SOURCES || keys.includes(property as keyof T) - ? undefined - : props[property as any]; - }, - has(property) { - return property !== $SOURCES && !keys.includes(property as keyof T) && property in props; - }, - keys() { - return ownEnumerableKeys(props).filter(k => !keys.includes(k as keyof T)); - } - }, - propTraps - ) as unknown as Omit; +): Omit; +export function omit>( + props: T, + hidden: (key: keyof T & (string | symbol)) => boolean +): Partial; +export function omit(props: any, ...keys: any[]): any { + let hidden: Hidden = keys.length === 1 && typeof keys[0] === "function" ? keys[0] : keys; + if (SUPPORTS_PROXY) { + // A view over a view flattens: one record, both filters, the original + // source — so a consumer walks the real object however deep the omits go. + let source = props; + const inner: OmitView | undefined = props[$PROXY] === props ? props[$OMIT] : undefined; + if (inner !== undefined) { + source = inner.source; + hidden = combineHidden(inner.hidden, hidden); + } + // Over a merge() proxy: one leaf view per flattened source (see OmitView). + // A flattened source that is itself a view — an earlier omit() this merge + // was built over — folds into one record with both filters, so a + // component chain of omit/merge/omit/merge stays one level deep. + const merged = mergeSources(source); + let entries: OmitView[] | undefined; + if (merged !== undefined) { + entries = new Array(merged.length); + for (let i = 0; i < merged.length; i++) { + const leaf = merged[i]; + entries[i] = + leaf instanceof OmitView + ? new OmitView(leaf.source, combineHidden(leaf.hidden, hidden)) + : new OmitView(leaf, hidden); + } + } + return new Proxy(new OmitView(source, hidden, entries), omitTraps); } const result: Record = {}; const propNames = Object.getOwnPropertyNames(props); - const blocked = - keys.length > 4 && propNames.length > keys.length ? new Set(keys) : undefined; + const isHiddenKey: (key: string) => boolean = + typeof hidden === "function" + ? hidden + : hidden.length > 4 && propNames.length > hidden.length + ? ( + blocked => (key: string) => + blocked.has(key) + )(new Set(hidden)) + : key => hidden.includes(key); for (const propName of propNames) { - if (blocked ? !blocked.has(propName) : !keys.includes(propName)) { + if (!isHiddenKey(propName)) { const desc = Object.getOwnPropertyDescriptor(props, propName)!; !desc.get && !desc.set && desc.enumerable && desc.writable && desc.configurable ? (result[propName] = desc.value) diff --git a/packages/signals/tests/store/utilities-no-proxy.test.ts b/packages/signals/tests/store/utilities-no-proxy.test.ts new file mode 100644 index 000000000..873ba06ca --- /dev/null +++ b/packages/signals/tests/store/utilities-no-proxy.test.ts @@ -0,0 +1,161 @@ +// The copy paths of merge() and omit(): what a platform without `Proxy` +// gets. `SUPPORTS_PROXY` is read once at module load, so it is mocked to +// false for this file and every result here is a plain object. +// +// The contract pinned: the same shadowing, hiding, getter liveness and +// descriptor KINDS as the views — the truths `spread` and a component read — +// with the one documented difference that a data property is a snapshot +// taken at merge()/omit() time rather than a live read. What has always +// needed Proxy still does: a store cannot exist without one, and a function +// source (merge's memo) degrades — pinned as a limitation, not a promise. +import { describe, expect, test, vi } from "vitest"; + +vi.mock("../../src/core/constants.js", async importOriginal => ({ + ...(await importOriginal()), + SUPPORTS_PROXY: false +})); + +import { + $PROXY, + createRoot, + createSignal, + flush, + hasStaticKeys, + merge, + mergeSources, + omit, + omitView, + resolvedTable, + SUPPORTS_PROXY +} from "../../src/index.js"; + +describe("merge/omit without Proxy", () => { + test("the flag is off and nothing wears the proxy mark", () => { + expect(SUPPORTS_PROXY).toBe(false); + const merged = merge({ a: 1 }, { b: 2 }); + const rest = omit({ a: 1, b: 2 }, "a"); + expect($PROXY in merged).toBe(false); + expect($PROXY in rest).toBe(false); + // and every view helper reads them as the plain objects they are + for (const o of [merged, rest]) { + expect(hasStaticKeys(o)).toBe(true); + expect(resolvedTable(o)).toBeUndefined(); + expect(mergeSources(o)).toBeUndefined(); + expect(omitView(o)).toBeUndefined(); + } + }); + + test("merge: later sources win, getters stay live, data properties are snapshots", () => { + const [sig, setSig] = createSignal("x"); + const a = { shadowed: "lower", data: 1 }; + const b = { + shadowed: "upper", + get live() { + return sig(); + } + }; + const props = merge(a, b); + expect(props.shadowed).toBe("upper"); + expect(props.live).toBe("x"); + setSig("y"); + flush(); + expect(props.live).toBe("y"); + expect(Object.keys(props).sort()).toEqual(["data", "live", "shadowed"]); + // the copy-path difference: a source mutated afterwards is not seen + a.data = 2; + expect(props.data).toBe(1); + }); + + test("merge: descriptor kinds are the leaves' — data stays data, getter stays getter", () => { + const props = merge( + { a: 1 }, + { + get b() { + return 2; + } + } + ); + const a = Object.getOwnPropertyDescriptor(props, "a")!; + expect(a.get).toBeUndefined(); + expect(a.value).toBe(1); + expect(typeof Object.getOwnPropertyDescriptor(props, "b")!.get).toBe("function"); + }); + + test("merge: a single non-function source is returned as is; falsy sources are skipped", () => { + const only = { a: 1 }; + expect(merge(only)).toBe(only); + expect(merge(null, only, undefined, false)).toBe(only); + expect(merge({ a: 1 }, null, { b: 2 })).toEqual({ a: 1, b: 2 }); + }); + + test("merge: a copy of a merge is a plain source; writes land on the copy", () => { + const inner = merge({ type: "button" }, { label: "a" }) as Record; + inner.label = "b"; + expect(inner.label).toBe("b"); + const outer = merge({ size: "m" }, inner); + expect(outer.type).toBe("button"); + expect(outer.label).toBe("b"); + expect(outer.size).toBe("m"); + }); + + test("omit: hides by key list or predicate, keeps getters live", () => { + const [sig, setSig] = createSignal(1); + const props = { + $internal: true, + as: "a", + class: "btn", + get count() { + return sig(); + } + }; + const byKeys = omit(props, "as"); + expect("as" in byKeys).toBe(false); + expect(byKeys.class).toBe("btn"); + expect(byKeys.count).toBe(1); + setSig(2); + flush(); + expect(byKeys.count).toBe(2); + expect(typeof Object.getOwnPropertyDescriptor(byKeys, "count")!.get).toBe("function"); + expect(Object.getOwnPropertyDescriptor(byKeys, "class")!.value).toBe("btn"); + + const byRule = omit(props, k => (k as string)[0] === "$"); + expect(Object.keys(byRule).sort()).toEqual(["as", "class", "count"]); + }); + + test("the component chain resolves the same way it does through views", () => { + const [label, setLabel] = createSignal("l"); + const user = { + as: "a", + class: "btn", + get label() { + return label(); + }, + type: "reset" + }; + const l1 = merge({ type: "button", role: "button" }, user); + const l2 = omit(l1, "type"); + const l3 = merge({ as: "button" }, l2); + const l4 = omit(l3, "as"); + expect(Object.keys(l4).sort()).toEqual(["class", "label", "role"]); + expect("type" in l4).toBe(false); + expect("as" in l4).toBe(false); + expect(l4.role).toBe("button"); + expect(l4.label).toBe("l"); + setLabel("m"); + flush(); + expect(l4.label).toBe("m"); + expect(Object.getOwnPropertyDescriptor(l4, "class")!.value).toBe("btn"); + expect(typeof Object.getOwnPropertyDescriptor(l4, "label")!.get).toBe("function"); + }); + + test("LIMITATION: a function source needs Proxy — the copy path does not read through it", () => { + // merge wraps the function in a memo and the copy walks the memo's own + // keys, not the object it returns. Unchanged from before the views; a + // platform without Proxy has never had reactive merge sources. + createRoot(() => { + const props = merge({ a: 1 }, () => ({ b: 2 })) as Record; + expect(props.a).toBe(1); + expect(props.b).toBeUndefined(); + }); + }); +}); diff --git a/packages/signals/tests/store/utilities.test.ts b/packages/signals/tests/store/utilities.test.ts index d091968ce..d872e3926 100644 --- a/packages/signals/tests/store/utilities.test.ts +++ b/packages/signals/tests/store/utilities.test.ts @@ -1,4 +1,5 @@ import { + $PROXY, createEffect, createRoot, createSignal, @@ -8,7 +9,9 @@ import { getOwner, merge, mergeSources, + hasStaticKeys, omit, + OmitView, reconcile, snapshot, type Store @@ -84,16 +87,9 @@ describe("merge", () => { expect("a" in merge(value, getter)).toBeTruthy(); expect("a" in merge(getter, value)).toBeTruthy(); }); - it("doesn't keep references for non-getters", () => { - const a = { value1: 1 }; - const b = { value2: 2 }; - const props = merge(a, b); - a.value1 = b.value2 = 3; - expect(props.value1).toBe(1); - expect(props.value2).toBe(2); - expect(Object.keys(props).join()).toBe("value1,value2"); - }); - it("without getter transfers only value", () => { + it("is a live view: data properties read through to the sources", () => { + // Never a copy (#3448): a plain data property on a source is read at + // access time, the same as a getter, so a source mutated later is seen. const a = { value1: 1 }; const b = { get value2() { @@ -102,10 +98,10 @@ describe("merge", () => { }; const props = merge(a, b); a.value1 = 3; - expect(props.value1).toBe(1); + expect(props.value1).toBe(3); expect(Object.keys(props).join()).toBe("value1,value2"); }); - it("overrides enumerables", () => { + it("mirrors the source's enumerability", () => { const a = Object.defineProperties( {}, { @@ -115,10 +111,11 @@ describe("merge", () => { } } ); - const props = merge(a, {}); + const props = merge(a, { value2: 1 }); expect((props as any).value1).toBe(2); - expect(Object.getOwnPropertyDescriptor(props, "value1")?.enumerable).toBeTruthy(); - expect(Object.keys(props).join()).toBe("value1"); + expect("value1" in props).toBe(true); + expect(Object.getOwnPropertyDescriptor(props, "value1")?.enumerable).toBe(false); + expect(Object.keys(props).join()).toBe("value2"); }); it("does not write the target", () => { const props = { value1: 1 }; @@ -140,15 +137,20 @@ describe("merge", () => { const newProps = merge(props, null, undefined); expect(props === newProps).toBeTruthy(); }); - it("returns same reference when all keys are covered", () => { + it("returns same reference when only one source is non-falsy", () => { const props = { a: 1, b: 2 }; - const newProps = merge({ a: 2 }, { b: 2 }, props); - expect(props === newProps).toBeTruthy(); + expect(merge(null, props, undefined) === props).toBeTruthy(); + const view = omit({ a: 1, b: 2 }, "a"); + expect(merge(false, view) === view).toBeTruthy(); }); - it("returns new reference when all keys are not covered", () => { - const props = { a: 1 }; + it("returns a view when there are several sources, even if the last covers every key", () => { + // No key enumeration at construction: a view is O(1) to make, and the + // shortcut would have cost a key walk on every merge to save nothing. + const props = { a: 1, b: 2 }; const newProps = merge({ a: 2 }, { b: 2 }, props); expect(props === newProps).toBeFalsy(); + expect(newProps.a).toBe(1); + expect(mergeSources(newProps)).toEqual([{ a: 2 }, { b: 2 }, props]); }); it("uses the source instances", () => { const source1 = { @@ -166,11 +168,20 @@ describe("merge", () => { expect(props.b === source2).toBeTruthy(); }); it("flattens nested merge sources in order", () => { + const a = { a: 1 }; + const b = { b: 2 }; const target = { a: 3, b: 4 }; - const props = merge(merge({ a: 1 }, { b: 2 }), target); - expect(props === target).toBeTruthy(); + const props = merge(merge(a, b), target); + expect(mergeSources(props)).toEqual([a, b, target]); + expect(props.a).toBe(3); expect(merge(merge({ value: 1 }, { value: 2 }), { value: 3 }).value).toBe(3); expect(merge({ value: 1 }, merge({ value: 2 }, { value: 3 })).value).toBe(3); + const inner = merge({ value: 2 }, { value: 3 }); + expect(mergeSources(merge({ value: 1 }, inner))).toEqual([ + { value: 1 }, + { value: 2 }, + { value: 3 } + ]); }); it("does not clone nested objects", () => { const b = { value: 1 }; @@ -210,9 +221,11 @@ describe("merge", () => { expect(final.d).toBe(4); expect(Object.getOwnPropertySymbols(final)).toEqual([]); }); - it("re-merging a merged object mutated afterwards reads the mutations (#3384)", () => { - // @solidjs/html assigns props and a children getter onto merge()'s result - // after spreading; a component's own merge(defaults, props) must see them. + it("writes to a merged object are no-ops; a copy is a plain object (#3384)", () => { + // The result is a view over its sources, never a copy: assigning onto it + // changes nothing (a consumer that needs its own object copies it first, + // and the copy carries no $SOURCES). @solidjs/html builds its own objects + // and merges once for exactly this reason. const props = merge({ type: "button", label: "a" }, { disabled: false }) as Record< string, unknown @@ -220,14 +233,18 @@ describe("merge", () => { props.label = "b"; props.extra = 1; Object.defineProperty(props, "children", { get: () => "kids", configurable: true }); - const final = merge({ type: "submit", size: "m" }, props) as Record; + expect(props.label).toBe("a"); + expect("extra" in props).toBe(false); + expect(props.children).toBeUndefined(); + const copy = { ...props, label: "b" }; + expect(Object.getOwnPropertySymbols(copy)).toEqual([]); + expect(mergeSources(copy)).toBeUndefined(); + const final = merge({ type: "submit", size: "m" }, copy) as Record; expect(final.type).toBe("button"); expect(final.label).toBe("b"); - expect(final.extra).toBe(1); - expect(final.children).toBe("kids"); expect(final.size).toBe("m"); }); - it("still flattens merge proxies (writes are no-ops, so the sources are the truth)", () => { + it("flattens merge proxies (writes are no-ops, so the sources are the truth)", () => { const [store] = createStore({ a: 1 }); const b = { b: 2 }; const c = { c: 3 }; @@ -238,8 +255,6 @@ describe("merge", () => { expect(outer.b).toBe(2); expect(outer.c).toBe(3); expect(Object.keys(outer).sort()).toEqual(["a", "b", "c"]); - // and a plain-object result is not a source list - expect(mergeSources(merge(b, c))).toBeUndefined(); }); it("handles undefined values", () => { const props = merge({ a: 1 }, { a: undefined }); @@ -282,7 +297,8 @@ describe("merge", () => { }); it("works with a array source", () => { const props = merge({ value: 1 }, [2]); - expect(Object.keys(props).join()).toBe("0,value,length"); + // `length` is not enumerable on the array and the view mirrors that + expect(Object.keys(props).join()).toBe("value,0"); expect(props.value).toBe(1); expect(props.length).toBe(1); expect(props[0]).toBe(2); @@ -472,12 +488,140 @@ describe("omit Props", () => { expect((spread as any).placeholder).toBe("you@example.com"); }); }); - test("omit result is immutable", () => { - const props = { first: 1, second: 2 }; + test("omit result is a live view, not a copy", () => { + // Nothing is read or materialized at omit() time: the result reads its + // source when a key is used, the same for a plain object as for a store + // or merge proxy, so a later change to the source shows through. + let reads = 0; + const props = { + first: 1, + second: 2, + get third() { + reads++; + return 3; + } + }; const otherProps = omit(props, "first"); + expect(reads).toBe(0); props.first = props.second = 3; - expect(props.first).toBe(3); + expect(otherProps.second).toBe(3); + expect(otherProps.third).toBe(3); + expect(reads).toBe(1); + }); + test("omit result rejects writes", () => { + const props = { first: 1, second: 2 }; + const otherProps = omit(props, "first") as any; + otherProps.second = 9; + delete otherProps.second; expect(otherProps.second).toBe(2); + expect(props.second).toBe(2); + }); + test("omit with a predicate hides keys by rule", () => { + const props = { + $props: 1, + $theme: 2, + id: "x", + get label() { + return "L"; + } + }; + const rest = omit(props, k => typeof k === "string" && k[0] === "$"); + expect(Object.keys(rest)).toEqual(["id", "label"]); + expect("$props" in rest).toBe(false); + expect((rest as any).$props).toBeUndefined(); + expect(rest.label).toBe("L"); + expect({ ...rest }).toEqual({ id: "x", label: "L" }); + }); + test("omit of an omit flattens to one view over the original source", () => { + let reads = 0; + const props = { + a: 1, + b: 2, + get c() { + reads++; + return 3; + }, + d: 4 + }; + const inner = omit(props, "a"); + const outer = omit(inner, "b"); + expect(Object.keys(outer)).toEqual(["c", "d"]); + expect("a" in outer).toBe(false); + expect("b" in outer).toBe(false); + expect(outer.c).toBe(3); + expect(reads).toBe(1); + // and with a predicate on either layer + const outer2 = omit(inner, k => k === "d"); + expect(Object.keys(outer2)).toEqual(["b", "c"]); + }); + test("merge over an omit of a plain object stays lazy and filtered", () => { + let reads = 0; + const props = { + hidden: "h", + get shown() { + reads++; + return "s"; + }, + both: "from-props" + }; + const merged = merge(omit(props, "hidden"), { both: "from-later", extra: 1 }); + expect(reads).toBe(0); + expect(Object.keys(merged).sort()).toEqual(["both", "extra", "shown"]); + expect("hidden" in merged).toBe(false); + expect((merged as any).hidden).toBeUndefined(); + expect(merged.both).toBe("from-later"); + expect(merged.shown).toBe("s"); + expect(reads).toBe(1); + }); + test("a defaults + omit + spread chain flattens to leaf objects with filters", () => { + // A component chain: each layer merges defaults, hides its own keys and + // spreads the rest into the next. The outermost merge must resolve to + // the leaf objects — no merge or omit PROXY left among its sources — and + // every hidden key of every layer must stay hidden. + let reads = 0; + const props = { + a: "a", + b: "b", + get c() { + reads++; + return "c"; + }, + d: "d", + e: "e" + }; + const layer1 = merge({ a: "def-a", x: 1 }, props); // defaults + const rest1 = omit(layer1, "a"); // hides a + const layer2 = merge({ y: 2 }, rest1, () => ({ b: "fn-b" })); // defaults, function source + const rest2 = omit(layer2, "b", "y"); // hides b, y + const out = merge(rest2, { z: 3 }); + + const leaves = mergeSources(out)!; + for (const leaf of leaves) { + expect(typeof leaf === "function" || leaf instanceof OmitView || !($PROXY in leaf)).toBe( + true + ); + } + expect(Object.keys(out).sort()).toEqual(["c", "d", "e", "x", "z"]); + expect("a" in out).toBe(false); + expect("b" in out).toBe(false); + expect("y" in out).toBe(false); + expect((out as any).a).toBeUndefined(); + expect((out as any).b).toBeUndefined(); + expect(out.x).toBe(1); + expect(out.z).toBe(3); + expect(reads).toBe(0); + expect(out.c).toBe("c"); + expect(reads).toBe(1); + // the intermediate views themselves still answer correctly + expect(Object.keys(rest2).sort()).toEqual(["c", "d", "e", "x"]); + expect((rest1 as any).a).toBeUndefined(); + expect(rest1.b).toBe("b"); + }); + test("omit keeps symbol-keyed props unless hidden", () => { + const sym = Symbol("s"); + const props = { a: 1, [sym]: 2 }; + expect(Reflect.ownKeys(omit(props, "a"))).toEqual([sym]); + expect(Reflect.ownKeys(omit(props, sym as any))).toEqual(["a"]); }); test("omit clones the descriptor", () => { let signalValue = 1; @@ -592,6 +736,133 @@ describe("omit Props", () => { }); }); +// The view proxies must tell consumers the truth about what is behind a key, +// through any depth of layers: `getOwnPropertyDescriptor` reports a DATA +// descriptor only when the key is a data property of a plain-object leaf +// (the compiler's encoding of a static prop), and `hasStaticKeys` says +// whether the key set itself is fixed. That is what lets spread() skip a +// reactive node for static children at the bottom of a component chain. +describe("view descriptors", () => { + test("merge reports the owning leaf's kind: data stays data, getter stays getter", () => { + const [sig] = createSignal("x"); + const props = merge( + { a: 1, shadowed: "lower" }, + { + get b() { + return sig(); + }, + shadowed: "upper" + } + ); + const a = Object.getOwnPropertyDescriptor(props, "a")!; + expect(a.get).toBeUndefined(); + expect(a.value).toBe(1); + expect(a.configurable).toBe(true); + const b = Object.getOwnPropertyDescriptor(props, "b")!; + expect(typeof b.get).toBe("function"); + expect(b.get!()).toBe("x"); + expect(Object.getOwnPropertyDescriptor(props, "shadowed")!.value).toBe("upper"); + expect(Object.getOwnPropertyDescriptor(props, "missing")).toBeUndefined(); + }); + test("a store leaf or a memo source is always an accessor, whatever the store reports", () => { + createRoot(() => { + const [store] = createStore({ a: 1 }); + const overStore = merge({ b: 2 }, store); + expect(typeof Object.getOwnPropertyDescriptor(overStore, "a")!.get).toBe("function"); + expect(Object.getOwnPropertyDescriptor(overStore, "b")!.value).toBe(2); + const overMemo = merge({ b: 2 }, () => ({ a: 1 })); + expect(typeof Object.getOwnPropertyDescriptor(overMemo, "a")!.get).toBe("function"); + const omitOverStore = omit(store, "z"); + expect(typeof Object.getOwnPropertyDescriptor(omitOverStore, "a")!.get).toBe("function"); + }); + }); + test("omit forwards its source's kind and hides its keys", () => { + const view = omit( + { + a: 1, + get b() { + return 2; + }, + c: 3 + }, + "c" + ); + expect(Object.getOwnPropertyDescriptor(view, "a")!.value).toBe(1); + expect(typeof Object.getOwnPropertyDescriptor(view, "b")!.get).toBe("function"); + expect(Object.getOwnPropertyDescriptor(view, "c")).toBeUndefined(); + }); + test("kind survives omit → merge → omit → merge, and the layers collapse to leaf views", () => { + const user = { + class: "btn", + get label() { + return "l"; + }, + as: "a", + type: "reset" + }; + const l1 = merge({ type: "button" }, user); + const l2 = omit(l1, "type"); + const l3 = merge({ as: "button", role: "button" }, l2); + const l4 = omit(l3, "as"); + const l5 = merge(l4, { extra: 1 }); + // every layer is one deep: leaf views over the original objects + const leaves = mergeSources(l5)!; + expect(leaves.length).toBe(4); + for (const leaf of leaves.slice(0, 3)) expect(leaf).toBeInstanceOf(OmitView); + // l3's statics, l1's defaults, the user's props — in merge order + expect((leaves[0] as OmitView).hidden).toEqual(["as"]); + expect((leaves[2] as OmitView).source).toBe(user); + expect((leaves[2] as OmitView).hidden).toEqual(["type", "as"]); + // and the truth reaches the top + expect(Object.getOwnPropertyDescriptor(l5, "class")!.value).toBe("btn"); + expect(typeof Object.getOwnPropertyDescriptor(l5, "label")!.get).toBe("function"); + expect(Object.getOwnPropertyDescriptor(l5, "type")).toBeUndefined(); + expect(Object.getOwnPropertyDescriptor(l5, "as")).toBeUndefined(); + expect(Object.getOwnPropertyDescriptor(l5, "role")!.value).toBe("button"); + expect(Object.keys(l5).sort()).toEqual(["class", "extra", "label", "role"]); + }); + test("hasStaticKeys: plain objects and views over them; not stores, memos, or views over them", () => { + createRoot(() => { + const [store] = createStore({ a: 1 }); + const plain = { a: 1 }; + expect(hasStaticKeys(plain)).toBe(true); + expect(hasStaticKeys(merge(plain, { b: 2 }))).toBe(true); + expect(hasStaticKeys(omit(plain, "a"))).toBe(true); + expect(hasStaticKeys(omit(merge(plain, { b: 2 }), "a"))).toBe(true); + expect(hasStaticKeys(merge(omit(merge(plain, { b: 2 }), "a"), { c: 3 }))).toBe(true); + expect(hasStaticKeys(store)).toBe(false); + expect(hasStaticKeys(merge(plain, store))).toBe(false); + expect(hasStaticKeys(omit(store, "a"))).toBe(false); + expect(hasStaticKeys(omit(merge(plain, store), "a"))).toBe(false); + expect(hasStaticKeys(merge(plain, () => ({ b: 2 })))).toBe(false); + }); + }); + test("a spread copy of a view is a plain snapshot with the right kinds", () => { + let n = 0; + const view = merge( + { a: 1 }, + omit( + { + get b() { + return ++n; + }, + c: 3 + }, + "c" + ) + ); + const copy = { ...view }; + expect(copy).toEqual({ a: 1, b: 1 }); + expect(Object.getOwnPropertyDescriptor(copy, "b")!.get).toBeUndefined(); + // a descriptor copy keeps the getter live + const desc: Record = {}; + for (const key of Reflect.ownKeys(view)) + Object.defineProperty(desc, key, Object.getOwnPropertyDescriptor(view, key)!); + expect(desc.b).toBe(2); + expect(desc.b).toBe(3); + }); +}); + // The shape headless-UI libraries (Kobalte) compose per element: a compiled // props object → merge(defaults) → omit(consumed) → merge(call-site statics) // … → omit("as") at the polymorphic renderer. These pin what the chain must diff --git a/packages/solid/src/index.ts b/packages/solid/src/index.ts index 5918d3ce9..06ad6b6b0 100644 --- a/packages/solid/src/index.ts +++ b/packages/solid/src/index.ts @@ -21,6 +21,12 @@ export { mapArray, merge, mergeSources, + omitView, + sourceKeys, + sourceHas, + sourceGet, + hasStaticKeys, + resolvedTable, omit, onCleanup, onSettled, diff --git a/packages/solid/src/server/index.ts b/packages/solid/src/server/index.ts index 8cd13929e..7cc0cbad7 100644 --- a/packages/solid/src/server/index.ts +++ b/packages/solid/src/server/index.ts @@ -37,6 +37,12 @@ export { mapArray, merge, mergeSources, + omitView, + sourceKeys, + sourceHas, + sourceGet, + hasStaticKeys, + resolvedTable, omit, onCleanup, onSettled, diff --git a/packages/solid/src/server/signals.ts b/packages/solid/src/server/signals.ts index 829413d05..bfdf4c72b 100644 --- a/packages/solid/src/server/signals.ts +++ b/packages/solid/src/server/signals.ts @@ -27,7 +27,19 @@ export { } from "@solidjs/signals"; export { flatten } from "@solidjs/signals"; -export { snapshot, omit, storePath, $PROXY, $TRACK } from "@solidjs/signals"; +export { + snapshot, + omit, + storePath, + $PROXY, + $TRACK, + omitView, + sourceKeys, + sourceHas, + sourceGet, + hasStaticKeys, + resolvedTable +} from "@solidjs/signals"; // === Type re-exports === diff --git a/packages/universal/src/universal.ts b/packages/universal/src/universal.ts index 38d9f068f..268d5430d 100644 --- a/packages/universal/src/universal.ts +++ b/packages/universal/src/universal.ts @@ -9,7 +9,13 @@ import { createMemo, createRenderEffect, flush, - $PROXY + mergeSources, + omitView, + sourceKeys, + sourceHas, + sourceGet, + hasStaticKeys, + resolvedTable } from "solid-js"; export interface RendererOptions { @@ -404,8 +410,8 @@ export function createRenderer({ node, () => { for (let i = props.length - 1; i >= 0; i--) { - const s = resolveSource(props[i]); - if (s != null && "children" in s) return s.children; + const s = resolveEntry(props[i]); + if (s != null && sourceHas(s, "children")) return sourceGet(s, "children"); } }, undefined, @@ -416,10 +422,12 @@ export function createRenderer({ return prevProps; } if (!skipChildren) { - if (typeof props !== "function" && props != null && props[$PROXY] !== props) { - // A plain object's key set can't change reactively: no `children` - // key means nothing to insert, a data property inserts its value - // with no effect, only a getter needs the tracking scope. + if (typeof props !== "function" && props != null && hasStaticKeys(props)) { + // A plain object's key set can't change reactively — nor can a + // merge/omit view's over plain objects, and its descriptor trap + // tells the truth about the owning leaf: no `children` key means + // nothing to insert, a data property inserts its value with no + // effect, only a getter needs the tracking scope. const desc = Object.getOwnPropertyDescriptor(props, "children"); if (desc !== undefined) { if (desc.get === undefined) @@ -430,8 +438,8 @@ export function createRenderer({ insert( node, () => { - const s = resolveSource(props); - return s != null ? s.children : undefined; + const s = resolveEntry(props); + return s != null ? sourceGet(s, "children") : undefined; }, undefined, undefined, @@ -442,7 +450,21 @@ export function createRenderer({ () => { const s = resolveSource(props); const newProps = {}; - if (s != null) collectProps(newProps, s); + // A merge() proxy is read through its sources and an omit() proxy + // through its view record, never through their traps; a view over + // plain objects through its resolved table, one read per key on + // every rerun (see @solidjs/web). + const table = resolvedTable(s); + if (table !== undefined) { + for (const [prop, leaf] of table) { + if (typeof prop !== "string" || prop === "children") continue; + newProps[prop] = leaf[prop]; + } + return newProps; + } + const sources = mergeSources(s); + if (sources !== undefined) return collectSources(newProps, sources); + if (s != null) collectProps(newProps, entryOf(s)); return newProps; }, apply, @@ -455,35 +477,56 @@ export function createRenderer({ return typeof s === "function" ? s() : s; } + // A resolved source as the ENTRY a consumer walks: an omit() proxy is + // replaced by its view record, anything else is itself. + function entryOf(s) { + if (s == null) return s; + const view = omitView(s); + return view !== undefined ? view : s; + } + + function resolveEntry(s) { + return entryOf(resolveSource(s)); + } + // Layered sources into `out`. Every function source is resolved once, up - // front; keys are then collected left-to-right (Object.assign order), and - // a key any LATER source has is skipped unread. `in` is mergeProps()'s own - // resolution test, so a proxy source answers through its `has` trap. + // front, a merge() proxy among them contributes its flattened sources in + // place; keys are then collected left-to-right (Object.assign order), and + // a key any LATER source has is skipped unread. `sourceHas` is merge()'s + // own resolution test, so a proxy source answers through its `has` trap + // and an omit view from its filter. function collectSources(out, sources) { - const n = sources.length; - const resolved = new Array(n); - for (let i = 0; i < n; i++) resolved[i] = resolveSource(sources[i]); - for (let i = 0; i < n; i++) { + const resolved = []; + for (let i = 0; i < sources.length; i++) { + const s = resolveSource(sources[i]); + const merged = mergeSources(s); + if (merged !== undefined) { + for (let j = 0; j < merged.length; j++) resolved.push(resolveEntry(merged[j])); + } else resolved.push(entryOf(s)); + } + for (let i = 0; i < resolved.length; i++) { const s = resolved[i]; if (s != null) collectProps(out, s, resolved, i + 1); } return out; } - // One layer of a spread source into `out`: enumerable keys (for...in, so a - // renderer's proxy props answer through their traps), `children` excluded - // (it has its own insert), `ref` carried through for the commit half. With - // `later` (the sources after this one, from index `from`), a key one of - // them defines is shadowed and never read here. + // One layer of a spread source into `out`: own string keys (one `ownKeys` + // trap for a renderer's proxy props, no descriptor trap per key), + // `children` excluded (it has its own insert), `ref` carried through for + // the commit half. With `later` (the sources after this one, from index + // `from`), a key one of them defines is shadowed and never read here. function collectProps(out, s, later?, from?) { - outer: for (const prop in s) { - if (prop === "children") continue; + const keys = sourceKeys(s); + outer: for (let i = 0; i < keys.length; i++) { + const prop = keys[i]; + if (typeof prop !== "string" || prop === "children") continue; if (later !== undefined) for (let j = from; j < later.length; j++) { const t = later[j]; - if (t != null && prop in t) continue outer; + if (t != null && sourceHas(t, prop)) continue outer; } - out[prop] = s[prop]; + out[prop] = sourceGet(s, prop); } return out; } diff --git a/packages/universal/test/spread-sources.spec.js b/packages/universal/test/spread-sources.spec.js index 65f8f0a37..5fb93488b 100644 --- a/packages/universal/test/spread-sources.spec.js +++ b/packages/universal/test/spread-sources.spec.js @@ -1,5 +1,5 @@ import * as r from "./custom.js"; -import { createRoot, createSignal, flush, onCleanup } from "solid-js"; +import { createRoot, createSignal, flush, merge, omit, onCleanup } from "solid-js"; // The renderer's spread() follows @solidjs/web's contract (#3388, #3419): at // most two reactive nodes per element, `ref` folded into the props effect, @@ -189,6 +189,57 @@ describe("universal spread: sources array", () => { }); }); +describe("universal spread: omit() and merge() views", () => { + it("walks an omit() view directly: hidden keys skipped unread, the rest reactive", () => { + const node = document.createElement("div"); + const [title, setTitle] = createSignal("t1"); + const hidden = vi.fn(() => true); + const props = { + id: "a", + get title() { + return title(); + }, + get isActive() { + return hidden(); + }, + children: "kid" + }; + const dispose = mount(() => r.spread(node, omit(props, "isActive"))); + expect(node.getAttribute("id")).toBe("a"); + expect(node.getAttribute("title")).toBe("t1"); + expect(node.textContent).toBe("kid"); + expect(node.hasAttribute("isActive")).toBe(false); + setTitle("t2"); + flush(); + expect(node.getAttribute("title")).toBe("t2"); + expect(hidden).not.toHaveBeenCalled(); + dispose(); + }); + + it("flattens a merge() and reads omit() views inside a sources array, later wins", () => { + const node = document.createElement("button"); + const shadowed = vi.fn(() => "submit"); + const rest = { + id: "r", + get type() { + return shadowed(); + }, + isActive: true + }; + const merged = merge({ "data-a": "1" }, () => ({ "data-b": "2" })); + const dispose = mount(() => + r.spread(node, [omit(rest, "isActive"), merged, { type: "button" }], true) + ); + expect(node.getAttribute("id")).toBe("r"); + expect(node.getAttribute("type")).toBe("button"); + expect(node.getAttribute("data-a")).toBe("1"); + expect(node.getAttribute("data-b")).toBe("2"); + expect(node.hasAttribute("isActive")).toBe(false); + expect(shadowed).not.toHaveBeenCalled(); + dispose(); + }); +}); + describe("universal spread: single reactive and nullish sources", () => { it("resolves a lone accessor inside its own scopes", () => { const node = document.createElement("div"); diff --git a/packages/web/src/client.ts b/packages/web/src/client.ts index b0bb5aaaa..888064282 100644 --- a/packages/web/src/client.ts +++ b/packages/web/src/client.ts @@ -12,6 +12,12 @@ import { merge as mergeProps, $PROXY, mergeSources, + omitView, + sourceKeys, + sourceHas, + sourceGet, + hasStaticKeys, + resolvedTable, flatten, createMemo, flush, @@ -797,7 +803,7 @@ export function readShallow(value) { if (value === null || typeof value !== "object") return value; if (Array.isArray(value)) return value.map(readShallow); if (value[$PROXY] !== value) return value; - const keys = ownKeys(value); + const keys = sourceKeys(value); const out = {}; for (let i = 0; i < keys.length; i++) { const k = keys[i]; @@ -806,16 +812,7 @@ export function readShallow(value) { return out; } -// The own keys of a spread/style source for a one-layer copy. For a proxy -// (merge/omit/`{...props}`, store records) ONE `ownKeys` trap: the trap keeps -// the key set tracked, and the enumerability check `for…in` would run — a -// `getOwnPropertyDescriptor` trap per key, allocating a descriptor plus a -// getter closure, then AGAIN for `hasOwn` — never happens. `Object.keys` for a -// plain object is exactly the own-enumerable set `for…in` + `hasOwn` yielded. -// Callers skip symbol keys. -function ownKeys(o: object): (string | symbol)[] { - return o[$PROXY] === o ? Reflect.ownKeys(o) : Object.keys(o); -} /** Compiler-emitted primitive; not for hand-written code. @internal */ +/** Compiler-emitted primitive; not for hand-written code. @internal */ export function setStyleProperty(node: Element, name: string, value: any): void; export function setStyleProperty(node, name, value) { @@ -883,18 +880,21 @@ export function spread(node, props, skipChildren, skip) { if (!skipChildren && !(skip !== undefined && skip("children"))) insert(node, () => { for (let i = props.length - 1; i >= 0; i--) { - const s = resolveSource(props[i]); - if (s != null && "children" in s) return s.children; + const s = resolveEntry(props[i]); + if (s != null && sourceHas(s, "children")) return sourceGet(s, "children"); } }); effect(() => collectSources({}, props, skip), apply); return prevProps; } if (!skipChildren && !(skip !== undefined && skip("children"))) { - if (typeof props !== "function" && props != null && props[$PROXY] !== props) { - // A plain object's key set can't change reactively: no `children` key - // means nothing to insert, a data property inserts its value with no - // effect, only a getter needs the tracking scope. + if (typeof props !== "function" && props != null && hasStaticKeys(props)) { + // A plain object's key set can't change reactively — nor can a + // merge/omit view's over plain objects, and its descriptor trap tells + // the truth about the owning leaf: no `children` key means nothing to + // insert, a data property inserts its value with no effect, only a + // getter needs the tracking scope. So `` + // with static children costs no children effect either. const desc = Object.getOwnPropertyDescriptor(props, "children"); if (desc !== undefined) { if (desc.get === undefined) insert(node, desc.value); @@ -902,8 +902,10 @@ export function spread(node, props, skipChildren, skip) { } } else insert(node, () => { - const source = resolveSource(props); - return source != null && hasOwn.call(source, "children") ? source.children : undefined; + const source = resolveEntry(props); + return source != null && sourceHas(source, "children") + ? sourceGet(source, "children") + : undefined; }); } effect(() => { @@ -916,31 +918,76 @@ export function spread(node, props, skipChildren, skip) { // 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. - // omit() is not a merge: it stays a proxy and is enumerated through its - // own filtering trap. + // 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); const sources = mergeSources(source); if (sources !== undefined) return collectSources(newProps, sources, skip); - if (source != null) collectProps(newProps, source, skip); + if (source != null) { + const view = omitView(source); + collectProps(newProps, view !== undefined ? view : source, skip); + } return newProps; }, apply); return prevProps; } +// A resolved view table (see `resolvedTable`) into `out`: the owning leaf's +// value per key, children excluded, `skip` honored. +function collectTable(out, table, skip) { + for (const [prop, leaf] of table) { + if (typeof prop !== "string" || prop === "children") continue; + if (skip !== undefined && skip(prop)) continue; + const v = leaf[prop]; + out[prop] = prop === "style" || prop === "class" ? readShallow(v) : v; + } + return out; +} + function resolveSource(s) { return typeof s === "function" ? s() : s; } +// A resolved source as the ENTRY a consumer walks: an omit() proxy is +// replaced by its view record, anything else is itself. (A merge() proxy is +// flattened by the caller, since it contributes several entries.) +function entryOf(s) { + if (s == null) return s; + const view = omitView(s); + return view !== undefined ? view : s; +} + +function resolveEntry(s) { + return entryOf(resolveSource(s)); +} + // Layered sources into `out`. Every function source is resolved once, up -// front; keys are then collected left-to-right (Object.assign order — the +// front, and a merge() proxy among them contributes its flattened sources in +// place; keys are then collected left-to-right (Object.assign order — the // order assign() applies them in, which `type`/`value`/`min`/`max` style -// pairs care about), and a key any LATER source has is skipped unread. `in` -// is merge()'s own resolution test, so a proxy source (store, omit) answers -// through its `has` trap rather than a per-key descriptor trap. +// pairs care about), and a key any LATER source has is skipped unread. +// `sourceHas` is merge()'s own resolution test, so a proxy source (store) +// answers through its `has` trap rather than a per-key descriptor trap, and +// an omit view answers from its filter. function collectSources(out, sources, skip) { - const n = sources.length; - const resolved = new Array(n); - for (let i = 0; i < n; i++) resolved[i] = resolveSource(sources[i]); - for (let i = 0; i < n; i++) { + const resolved = []; + for (let i = 0; i < sources.length; i++) { + const s = resolveSource(sources[i]); + const merged = mergeSources(s); + if (merged !== undefined) { + // A merge() result among the sources: its entries take its place. A + // function among THEM is merge's memo, resolved like any other. + for (let j = 0; j < merged.length; j++) resolved.push(resolveEntry(merged[j])); + } else resolved.push(entryOf(s)); + } + for (let i = 0; i < resolved.length; i++) { const s = resolved[i]; if (s != null) collectProps(out, s, skip, resolved, i + 1); } @@ -953,7 +1000,7 @@ function collectSources(out, sources, skip) { // With `later` (the sources after this one, from index `from`), a key one of // them defines is shadowed and never read here. function collectProps(out, s, skip, later?, from?) { - const keys = ownKeys(s); + const keys = sourceKeys(s); outer: for (let i = 0; i < keys.length; i++) { const prop = keys[i]; if (typeof prop !== "string" || prop === "children") continue; @@ -961,9 +1008,9 @@ function collectProps(out, s, skip, later?, from?) { if (later !== undefined) for (let j = from; j < later.length; j++) { const t = later[j]; - if (t != null && prop in t) continue outer; + if (t != null && sourceHas(t, prop)) continue outer; } - const v = s[prop]; + const v = sourceGet(s, prop); out[prop] = prop === "style" || prop === "class" ? readShallow(v) : v; } } /** Compiler-emitted primitive; not for hand-written code. @internal */ diff --git a/packages/web/src/server.ts b/packages/web/src/server.ts index 4b449c617..a5dd12005 100644 --- a/packages/web/src/server.ts +++ b/packages/web/src/server.ts @@ -11,6 +11,11 @@ import { createComponent, untrack, merge as mergeProps, + mergeSources, + omitView, + sourceKeys, + sourceGet, + resolvedTable, ssrScope as scope } from "solid-js"; import { effect, memo } from "./render.js"; @@ -3822,41 +3827,94 @@ export function ssrElement(tag, props, children, needsId, skip) { // called once and creates no memo, so the array form allocates no // hydration ids of its own — the client `spread` array form follows the // same rule. A nullish source is an empty source. + // + // Sources are walked as ENTRIES (`sourceKeys`/`sourceGet`): a + // merge() proxy contributes its flattened sources and an omit() proxy its + // view record, so neither is enumerated through its traps — a descriptor + // trap per key, allocating, on every element. The common case, one plain + // object, allocates nothing here. let sources = null; + let table = undefined; if (Array.isArray(props)) { sources = props; for (let i = 0; i < sources.length; i++) { - if (typeof sources[i] === "function") { + let s = sources[i]; + if (typeof s === "function") s = s(); + // Sources first: a merge() proxy AND an omit() over a merge both answer + // with flattened entries; only an omit() of a plain object is one view. + const merged = mergeSources(s); + const view = merged === undefined ? omitView(s) : undefined; + if (s !== sources[i] || view !== undefined || merged !== undefined) { // Resolve into a copy: the caller's array stays as passed. if (sources === props) sources = sources.slice(); - sources[i] = sources[i](); + if (merged !== undefined) { + // The flattened sources take this slot and are visited in turn: a + // function source among them is merge's memo, resolved like any + // other function entry. + sources.splice(i, 1, ...merged); + i--; + } else sources[i] = view !== undefined ? view : s; } } } else if (props == null) { // A nullish source (static or resolved) is an empty spread (#3297). props = {}; + } else if ((table = resolvedTable(props)) === undefined) { + const merged = mergeSources(props); + if (merged !== undefined) { + // Flattened entries take the array walk; a function among them is + // merge's memo, resolved here (the keys are allocated, see above). + sources = merged; + for (let i = 0; i < sources.length; i++) { + if (typeof sources[i] === "function") { + if (sources === merged) sources = sources.slice(); + sources[i] = sources[i](); + } + } + } else { + const view = omitView(props); + if (view !== undefined) props = view; + } } + // A merge/omit view over plain objects has a RESOLVED TABLE (key → owning + // leaf, shadowing applied, built once and shared with the component's own + // reads of the same view): one read per key, no per-key walk of the later + // sources. const skipChildren = VOID_ELEMENTS.test(tag); // Each emitted attribute carries its own leading space (the hydration key // already does), so skipped props leave no stray whitespace behind: // `
  • ` rather than `
  • ` (#3382). let result = `<${tag}${hk}`; // One walk over one prop body: the outer loop runs once for a single props - // object and once per source otherwise. + // object and once per source otherwise. With several sources every + // source's key list is taken once up front, and "a later source owns this + // key" is a lookup in that list — one `ownKeys` per source rather than an + // `in` (a trap, or a filtered view's) per key per later source. const last = sources === null ? 0 : sources.length - 1; + let keysOf = null; + if (sources !== null) { + keysOf = new Array(last + 1); + for (let s = 0; s <= last; s++) keysOf[s] = sources[s] == null ? null : sourceKeys(sources[s]); + } for (let s = 0; s <= last; s++) { - if (sources !== null && (props = sources[s]) == null) continue; - const keys = Object.keys(props); + const keys = + keysOf !== null + ? keysOf[s] + : table !== undefined + ? Array.from(table.keys()) + : sourceKeys(props); + if (keys === null) continue; + if (sources !== null) props = sources[s]; nextKey: for (let i = 0; i < keys.length; i++) { const prop = keys[i]; - if (skip !== undefined && skip(prop)) continue; - // A later source that has the key (`in`, so merge/omit proxies answer - // through their traps) owns it; this source's getter stays unread. + if (typeof prop !== "string" || (skip !== undefined && skip(prop))) continue; + // A later source that has the key owns it; this source's getter stays + // unread. for (let j = s + 1; j <= last; j++) { - const later = sources[j]; - if (later != null && prop in later) continue nextKey; + const later = keysOf[j]; + if (later !== null && later.includes(prop)) continue nextKey; } - // Every branch reads `props[prop]` itself, and only when it will use it. + // Every branch reads the prop itself, and only when it will use it. // On a spread these are compiled getters: `children` builds the child // element and consumes hydration ids as it goes. Reading it more than // once — or at all when JSX children already own the slot — burns ids @@ -3869,7 +3927,7 @@ export function ssrElement(tag, props, children, needsId, skip) { // path equivalent: textarea value/defaultValue are its text content, // never HTML attributes (#3286). if (tag === "textarea" && (prop === "value" || prop === "defaultValue")) { - const value = props[prop]; + const value = table !== undefined ? table.get(prop)[prop] : sourceGet(props, prop); if (value !== null) children = escape(value); continue; } @@ -3877,11 +3935,13 @@ export function ssrElement(tag, props, children, needsId, skip) { if (children === undefined && !skipChildren) children = tag === "script" || tag === "style" || prop === "innerHTML" - ? props[prop] - : escape(props[prop]); + ? table !== undefined + ? table.get(prop)[prop] + : sourceGet(props, prop) + : escape(table !== undefined ? table.get(prop)[prop] : sourceGet(props, prop)); continue; } - const value = props[prop]; + const value = table !== undefined ? table.get(prop)[prop] : sourceGet(props, prop); // Nullish is "not set" for every attribute, `style`/`class` included — // the client removes the attribute for `undefined`, and emitting // `style=""` here made the server disagree with it (#3382). diff --git a/packages/web/test/server/ssr-element-sources.spec.tsx b/packages/web/test/server/ssr-element-sources.spec.tsx index 7d3d02ea1..c666dda79 100644 --- a/packages/web/test/server/ssr-element-sources.spec.tsx +++ b/packages/web/test/server/ssr-element-sources.spec.tsx @@ -3,7 +3,7 @@ */ import { describe, expect, test } from "vitest"; import { renderToString, ssrElement } from "@solidjs/web"; -import { merge } from "solid-js"; +import { merge, omit } from "solid-js"; // `ssrElement(tag, [a, b, c], ...)` serializes straight from several prop // sources. Its contract is the merged one — byte-for-byte what @@ -313,3 +313,80 @@ describe("ssrElement with multiple sources", () => { expect(render("br", {}, undefined, true)).toMatch(/^
    $/); }); }); + +// omit() and merge() results are walked as VIEWS — the underlying sources, +// filter attached — not enumerated through their proxies. Output is what the +// proxy would have produced; the getters behind hidden or shadowed keys are +// never read. +describe("ssrElement over omit() and merge() views", () => { + test("a lone omit() of a plain object: hidden keys are skipped unread", () => { + const { source, reads } = counting({ + id: "a", + isActive: true, + disabled: false, + title: "t", + children: "kid" + }); + const html = render("li", omit(source, "isActive", "disabled")); + expect(html).toBe('
  • kid
    • '); + expect(reads).toEqual({ id: 1, isActive: 0, disabled: 0, title: 1, children: 1 }); + }); + + test("a predicate omit hides by rule", () => { + const { source, reads } = counting({ $props: 1, $theme: 2, id: "a", class: "c" }); + const html = render( + "div", + omit(source, k => typeof k === "string" && k[0] === "$") + ); + expect(html).toBe('
    '); + expect(reads.$props).toBe(0); + expect(reads.$theme).toBe(0); + }); + + test("an omit() view inside a sources array follows later-wins", () => { + const { source: rest, reads } = counting({ + id: "from-rest", + type: "submit", + "aria-label": "x", + isActive: true + }); + const html = render("button", [ + omit(rest, "isActive"), + { + type: "button", + get class() { + return "tab"; + } + } + ]); + // merged order: each key at the position of the LAST source carrying it + expect(html).toBe(''); + // `type` is owned by the later source: the view's getter is not read + expect(reads).toEqual({ id: 1, type: 0, "aria-label": 1, isActive: 0 }); + }); + + test("a merge() result among the sources contributes its flattened sources", () => { + const { source: a, reads } = counting({ id: "a", title: "t-a", "data-a": "1" }); + const merged = merge(a, () => ({ title: "t-fn" }), { "data-b": "2" }); + expect(render("div", [merged, { "data-c": "3" }])).toBe( + '
    ' + ); + expect(reads.title).toBe(0); + }); + + test("omit() over a merge() stays filtered (#3014) and reads each getter once", () => { + const { source: dyn, reads } = counting({ value: "v", onChange: () => {}, placeholder: "p" }); + const props = merge({ label: "L", name: "n" }, dyn); + const rest = omit(props, "name", "label", "value", "onChange"); + expect(render("input", [{ name: "n", value: "v" }, rest], undefined, false)).toBe( + '' + ); + expect(reads).toEqual({ value: 0, onChange: 0, placeholder: 1 }); + }); + + test("a nested omit() flattens to one view", () => { + const { source, reads } = counting({ a: "1", b: "2", c: "3" }); + expect(render("i", omit(omit(source, "a"), "b"))).toBe(''); + expect(reads).toEqual({ a: 0, b: 0, c: 1 }); + }); +}); diff --git a/packages/web/test/spread-nodes.spec.tsx b/packages/web/test/spread-nodes.spec.tsx index e26b7e687..11f5ecd7a 100644 --- a/packages/web/test/spread-nodes.spec.tsx +++ b/packages/web/test/spread-nodes.spec.tsx @@ -13,7 +13,7 @@ */ import { describe, expect, test, vi } from "vitest"; import { render, spread } from "@solidjs/web"; -import { createRoot, createSignal, flush, getOwner, merge } from "solid-js"; +import { createRoot, createSignal, createStore, flush, getOwner, merge, omit } from "solid-js"; const mount = (el: () => any) => { const container = document.createElement("div"); @@ -88,6 +88,27 @@ describe("reactive node count per element", () => { expect(spreadNodes(() => ({ title: "a", children: document.createElement("b") }))).toBe(1); }); + test("…and so do data children behind merge/omit views over plain objects", () => { + // The Kobalte shape: `` where the caller + // wrote static children. The view's descriptor trap reports the leaf's + // data property, so no children effect is created (#3388, #3448). + const props = { as: "a", title: "a", children: "static" }; + expect(spreadNodes(() => omit(props, "as"))).toBe(1); + expect(spreadNodes(() => merge({ role: "button" }, omit(props, "as")))).toBe(1); + expect(spreadNodes(() => omit(merge({ role: "button" }, props), "as"))).toBe(1); + // a getter behind the same layers still gets its effect + const reactive = { + as: "a", + get children() { + return "text"; + } + }; + expect(spreadNodes(() => omit(merge({ role: "button" }, reactive), "as"))).toBe(2); + // a store leaf can grow a `children` key later: reactive path + const [store] = createStore<{ title: string; children?: string }>({ title: "a" }); + expect(spreadNodes(() => merge({ role: "button" }, store))).toBe(2); + }); + test("compiled mergeProps source: 1 memo for the reactive part + 1 attribute effect", () => { const [rest] = createSignal({ "data-a": "1" }); // `
    ` → spread(el, merge({class}, () => rest())) diff --git a/packages/web/test/spread-sources.spec.tsx b/packages/web/test/spread-sources.spec.tsx index 614107996..c0624b7a4 100644 --- a/packages/web/test/spread-sources.spec.tsx +++ b/packages/web/test/spread-sources.spec.tsx @@ -10,7 +10,7 @@ * symbol keys ignored, omit() still opaque. */ import { describe, expect, test } from "vitest"; -import { render } from "@solidjs/web"; +import { render, spread } from "@solidjs/web"; import { createSignal, createStore, flush, merge, omit } from "solid-js"; const mount = (el: () => any) => { @@ -77,6 +77,82 @@ describe("spread over merge() sources", () => { expect(m.el().getAttribute("data-drop")).toBeNull(); m.dispose(); }); + + test("an omit() view of a plain object stays reactive and never reads hidden getters", () => { + const [title, setTitle] = createSignal("t1"); + let hiddenReads = 0; + const props = { + id: "keep", + get title() { + return title(); + }, + get isActive() { + hiddenReads++; + return true; + }, + children: "kid" + }; + const m = mount(() =>
    ); + expect(m.el().getAttribute("title")).toBe("t1"); + expect(m.el().textContent).toBe("kid"); + setTitle("t2"); + flush(); + expect(m.el().getAttribute("title")).toBe("t2"); + expect(m.el().hasAttribute("isActive")).toBe(false); + expect(hiddenReads).toBe(0); + m.dispose(); + }); + + test("a predicate omit() hides by rule through the spread", () => { + const m = mount(() => ( +
    String(k)[0] === "$")} /> + )); + expect(m.el().id).toBe("a"); + expect(m.el().hasAttribute("$props")).toBe(false); + m.dispose(); + }); + + test("omit() views and a merge() inside a sources array follow later-wins", () => { + let typeReads = 0; + const rest = { + id: "from-rest", + get type() { + typeReads++; + return "submit"; + }, + isActive: true + }; + const merged = merge({ "data-a": "1" }, () => ({ "data-b": "2" })); + const m = mount(() => { + const el = document.createElement("button"); + spread(el, [omit(rest, "isActive"), merged, { type: "button" }]); + return el; + }); + expect(m.el().id).toBe("from-rest"); + expect(m.el().getAttribute("type")).toBe("button"); + expect(m.el().getAttribute("data-a")).toBe("1"); + expect(m.el().getAttribute("data-b")).toBe("2"); + expect(m.el().hasAttribute("isActive")).toBe(false); + expect(typeReads).toBe(0); + m.dispose(); + }); + + test("children flow from an omit() view", () => { + const [kid, setKid] = createSignal("a"); + const props = { + get children() { + return kid(); + }, + hidden: true + }; + const m = mount(() =>
    ); + expect(m.el().textContent).toBe("a"); + expect(m.el().hasAttribute("hidden")).toBe(false); + setKid("b"); + flush(); + expect(m.el().textContent).toBe("b"); + m.dispose(); + }); }); describe("spread over a store proxy", () => { @@ -141,14 +217,13 @@ describe("style() with an object", () => { m.dispose(); }); - test("merge()'s PLAIN-object form mutated after merging: own writes win over the sources", () => { - // @solidjs/html builds props as `props = merge(props, spread)` and then - // keeps assigning props onto the result. Plain sources produce merge's - // plain-object form, which records $SOURCES too — the spread must read - // the object, not the stale sources. - const props: any = merge({ class: "base", id: "base-id" }, { class: "override" }); - props.id = "final-id"; - props.class = "final"; + test("a copy of a merge() result is a plain object: the spread reads the copy, not the sources", () => { + // merge() is a view; writes to it are no-ops. A caller that needs its own + // object copies it (`{...merged}`), and the copy carries no $SOURCES — the + // spread must read what is on the copy, never tunnel back (#3384). + const merged: any = merge({ class: "base", id: "base-id" }, { class: "override" }); + merged.id = "ignored"; + const props = { ...merged, id: "final-id", class: "final" }; const m = mount(() =>
    ); expect(m.el().className).toBe("final"); expect(m.el().id).toBe("final-id"); From 1ae4058ad7467c332b0359b93ad889d8ee6a6b5e Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Tue, 15 Sep 2026 02:15:41 -0700 Subject: [PATCH 2/3] perf(signals): views identify stores through $TARGET, never a store's generic read path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Constructing a view over a store proxy probed $SOURCES/$OMIT on the store; unknown symbols take the store's generic get path (firewall gate, tracked read), several times per omit()/merge(). Ask $TARGET first — a store's get trap answers it on its symbol fast path and the view traps answer it before anything else — and skip the second `in` on a store when the shadowing walk already established presence. omit(store, ...5): 2.9M/s -> 6.4M/s (next: 9.7M/s) merge(defaults, store): 2.6M/s -> 6.9M/s (next: 4.3M/s) Object.keys(omit(store)): 17k/s -> 28k/s (next: 33k/s) Co-authored-by: Cursor --- packages/signals/src/store/utils.ts | 67 +++++++++++++++++++---------- 1 file changed, 44 insertions(+), 23 deletions(-) diff --git a/packages/signals/src/store/utils.ts b/packages/signals/src/store/utils.ts index c9e93083d..378cf8278 100644 --- a/packages/signals/src/store/utils.ts +++ b/packages/signals/src/store/utils.ts @@ -1,7 +1,7 @@ import { pendingCheckActive } from "../core/core.js"; import { SUPPORTS_PROXY } from "../core/index.js"; import { createMemo } from "../signals.js"; -import { $PROXY, ownEnumerableKeys } from "./store.js"; +import { $PROXY, $TARGET, ownEnumerableKeys } from "./store.js"; function trueFn() { return true; @@ -50,6 +50,9 @@ function resolveSource(s: any) { const $SOURCES = Symbol(__DEV__ ? "MERGE_SOURCE" : 0); const $OMIT = Symbol(__DEV__ ? "OMIT_VIEW" : 0); +// The MergeView behind a merge proxy. `$SOURCES` answers the array; the +// record itself is reached through this symbol so the table can live on it. +const $VIEW = Symbol(__DEV__ ? "MERGE_VIEW" : 0); /** @internal The record behind an `omit()` proxy: `source` with `hidden` * keys removed. It is the proxy's TARGET, so the shared handler reads it as @@ -103,9 +106,19 @@ function viewSource(view: OmitView): any { return s == null ? EMPTY : s; } +// Whether a `$PROXY`-marked object is one of OUR views rather than a store +// (or a foreign proxy). Asked through `$TARGET`, which a store's `get` trap +// answers on its symbol fast path and the view traps answer first thing — +// so the question never reaches a store's generic read path (firewall gate, +// tracked key read), which is what any unknown symbol (`$SOURCES`, `$OMIT`) +// would take. Call only after `$PROXY in o` is known true. +function isView(o: any): boolean { + return o[$TARGET] === undefined && (o[$OMIT] !== undefined || o[$VIEW] !== undefined); +} + /** @internal The `OmitView` behind an `omit()` proxy, or undefined. */ export function omitView(o: any): OmitView | undefined { - return o != null && o[$PROXY] === o ? o[$OMIT] : undefined; + return o != null && $PROXY in o && o[$TARGET] === undefined ? o[$OMIT] : undefined; } // A props SOURCE ENTRY is a plain object, a proxy (store, merge, omit — the @@ -160,6 +173,7 @@ function leafHasStaticKeys(leaf: any): boolean { * only correct one. */ export function hasStaticKeys(o: any): boolean { if (!($PROXY in o)) return true; + if (o[$TARGET] !== undefined) return false; const sources = o[$SOURCES]; if (sources !== undefined) { for (let i = 0; i < sources.length; i++) if (!leafHasStaticKeys(sources[i])) return false; @@ -188,25 +202,31 @@ function accessorDescriptor(get: () => any, enumerable = true): PropertyDescript * * `configurable: true` always — the target has no such property, and the * Proxy invariants forbid reporting a non-configurable one. */ -function sourceDescriptor(s: any, key: PropertyKey): PropertyDescriptor | undefined { +// `present` says the caller has already established `key in s` (a trap's +// shadowing walk did), so a store is not asked a second time. +function sourceDescriptor( + s: any, + key: PropertyKey, + present = false +): PropertyDescriptor | undefined { if (s instanceof OmitView) { if (isHidden(s, key)) return undefined; const raw = s.source; if (typeof raw === "function") { - return sourceHas(viewSource(s), key) + return present || sourceHas(viewSource(s), key) ? accessorDescriptor(() => sourceGet(s, key)) : undefined; } - return sourceDescriptor(raw, key); + return sourceDescriptor(raw, key, present); } - if (s[$PROXY] === s) { - const desc = Reflect.getOwnPropertyDescriptor(s, key); - if (desc === undefined) return undefined; + if ($PROXY in s) { // Another view (an omit's source may be a merge proxy) already answers - // truthfully; a store's reported "data" is a signal. - return s[$SOURCES] !== undefined || s[$OMIT] !== undefined - ? desc - : accessorDescriptor(() => s[key], desc.enumerable); + // truthfully. A store's reported "data" is a signal, and a foreign proxy + // (frames slot props) has no own descriptors: for both, existence is + // `in` and the kind is accessor — one trap, and never the store's + // descriptor path. + if (isView(s)) return Reflect.getOwnPropertyDescriptor(s, key); + return present || key in s ? accessorDescriptor(() => s[key]) : undefined; } const desc = Reflect.getOwnPropertyDescriptor(s, key); if (desc === undefined) return undefined; @@ -255,16 +275,13 @@ class MergeView { * merged (keys added to it later are not seen — the copy didn't see them * either). */ export function resolvedTable(o: any): Map | undefined { - if (o == null || !($PROXY in o)) return undefined; + if (o == null || !($PROXY in o) || o[$TARGET] !== undefined) return undefined; const view = o[$OMIT]; if (view !== undefined) return omitTable(view); const sources = o[$SOURCES]; return sources === undefined ? undefined : mergeTable(mergeViewOf(o)); } -// The MergeView behind a merge proxy. `$SOURCES` answers the array; the -// record itself is reached through this symbol so the table can live on it. -const $VIEW = Symbol(__DEV__ ? "MERGE_VIEW" : 0); function mergeViewOf(proxy: any): MergeView { return proxy[$VIEW]; } @@ -361,13 +378,14 @@ function mergeGet(view: MergeView, property: PropertyKey): any { const mergeTraps: ProxyHandler = { get(view, property, receiver) { if (property === $PROXY) return receiver; + if (property === $TARGET) return undefined; if (property === $SOURCES) return view.sources; if (property === $VIEW) return view; return mergeGet(view, property); }, has(view, property) { if (property === $PROXY) return true; - if (property === $SOURCES || property === $VIEW) return false; + if (property === $TARGET || property === $SOURCES || property === $VIEW) return false; const table = mergeTable(view); if (table !== undefined) return table.has(property); const f = view.sources; @@ -394,7 +412,9 @@ const mergeTraps: ProxyHandler = { // shape the memo's current object has, the key is an accessor here. if (typeof raw === "function") return accessorDescriptor(() => mergeGet(view, property)); // `in` also answers for inherited keys, which have no own descriptor. - return sourceDescriptor(s, property) ?? accessorDescriptor(() => mergeGet(view, property)); + return ( + sourceDescriptor(s, property, true) ?? accessorDescriptor(() => mergeGet(view, property)) + ); } return undefined; }, @@ -424,6 +444,7 @@ const mergeTraps: ProxyHandler = { const omitTraps: ProxyHandler = { get(view, property, receiver) { if (property === $PROXY) return receiver; + if (property === $TARGET) return undefined; if (property === $OMIT) return view; // $SOURCES answers the FILTERED leaf entries (or nothing for a plain // source) — never the underlying merge's own sources, which would hand a @@ -441,7 +462,7 @@ const omitTraps: ProxyHandler = { }, has(view, property) { if (property === $PROXY) return true; - if (property === $SOURCES || property === $OMIT) return false; + if (property === $TARGET || property === $SOURCES || property === $OMIT) return false; if (view.entries !== undefined) { const table = omitTable(view); if (table !== undefined) return table.has(property); @@ -479,7 +500,7 @@ const omitTraps: ProxyHandler = { * no sources — `ownKeys` never answers $SOURCES — so what is on the copy is * the truth there and every consumer reads it directly (#3384). */ export function mergeSources(o: any): any[] | undefined { - return o != null && o[$PROXY] === o ? o[$SOURCES] : undefined; + return o != null && $PROXY in o && o[$TARGET] === undefined ? o[$SOURCES] : undefined; } /** * Merges multiple props-like objects into a single proxy that *preserves @@ -522,13 +543,13 @@ export function merge(...sources: T): Merge { flattened.push(createMemo(s as () => any) as any); continue; } - if ($PROXY in (s as object)) { + if ($PROXY in (s as object) && (s as any)[$TARGET] === undefined) { // A merge() proxy is flattened through: its writes are no-ops, so its // sources are exactly what it reads. An omit() proxy over a merge // answers $SOURCES with its FILTERED leaf views, never the merge's own // sources (#3014); an omit() of a plain object joins as its view // record. Either way the filter travels with the entry and the hidden - // keys stay hidden. + // keys stay hidden. A store (`$TARGET`) is a leaf as it is. const childSources = (s as object)[$SOURCES]; if (childSources) { for (let j = 0; j < childSources.length; j++) flattened.push(childSources[j]); @@ -648,7 +669,7 @@ export function omit(props: any, ...keys: any[]): any { // A view over a view flattens: one record, both filters, the original // source — so a consumer walks the real object however deep the omits go. let source = props; - const inner: OmitView | undefined = props[$PROXY] === props ? props[$OMIT] : undefined; + const inner = omitView(props); if (inner !== undefined) { source = inner.source; hidden = combineHidden(inner.hidden, hidden); From 44da46e98b3c41c42344c40a7a503e43c58c710d Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Tue, 15 Sep 2026 02:43:26 -0700 Subject: [PATCH 3/3] perf(signals,web,universal): source kinds decided once at view build; no brand check per read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A read through merge(defaults, store) was 1.5x slower than next: the walk was the same (in, then get) but went through sourceHas/sourceGet, each starting with `s instanceof OmitView` — on a store proxy that is a getPrototypeOf trap (~20 ns, as much as the read itself), twice per read. spread()'s per-key walk over a store-backed view paid the same. Every source entry now carries its kind (plain / omit record / proxy / memo), decided in merge()'s flattening loop and omit()'s argument check where the information already exists (MergeView.kinds, OmitView.kind). mergeGet/has/descriptor/ownKeys and the consumers (spread in web and universal, ssrElement) switch on the kind; the exported entry helpers take it as a parameter. viewOf(o) hands consumers the record behind a merge or omit proxy in two fast traps. Table-backed views also cache their own-keys array and per-key descriptor shape, so an enumeration through the traps re-reads no leaf descriptor. merge(defaults, store) 84 ns -> 61 ns (next -> branch) merge(defaults, store) + 5 reads 404 ns -> 346 ns (was 635 before this) merge(defaults, store) every key 2081 ns -> 1824 ns (was 3149) omit(store, 5) every key 1470 ns -> 1287 ns Object.keys(omit(store, 5)) 6979 ns -> 5417 ns A trap-logging store-shaped proxy pins it: a read through a merge or omit asks the store exactly what a direct read would. Co-authored-by: Cursor --- .changeset/merge-omit-lazy-views.md | 4 +- packages/signals/src/store/index.ts | 10 +- packages/signals/src/store/utils.ts | 430 ++++++++++++------ .../signals/tests/store/utilities.test.ts | 132 ++++++ packages/solid/src/index.ts | 9 + packages/solid/src/server/index.ts | 9 + packages/solid/src/server/signals.ts | 11 +- packages/universal/src/universal.ts | 111 +++-- packages/web/src/client.ts | 122 +++-- packages/web/src/server.ts | 118 +++-- 10 files changed, 665 insertions(+), 291 deletions(-) diff --git a/.changeset/merge-omit-lazy-views.md b/.changeset/merge-omit-lazy-views.md index dcca1f2ca..d3b520f1f 100644 --- a/.changeset/merge-omit-lazy-views.md +++ b/.changeset/merge-omit-lazy-views.md @@ -14,6 +14,8 @@ The two compose flat. An `omit()` over a `merge()` carries one filtered view per Reads stay cheap: a view over plain objects resolves a key → owning-leaf table once, on first read, and every `get`/`has`/descriptor is one lookup after that. `spread()` (DOM and universal) and `ssrElement()` read the leaves directly — never through the proxies' traps — and walk that table when there is one, so an effect rerun costs one read per key, as it did over the copy. Both proxies use a class target and one shared handler (no per-instance closures). +A view over a store asks the store nothing but the read. Each source's kind (plain object, omit record, proxy, memo) is decided once, when the view is built, and carried beside it — every brand check on a Proxy is a trap (`instanceof` is a `getPrototypeOf` trap, as expensive as a store read), and store detection goes through `$TARGET`, a symbol the store's `get` trap answers on its fast path, never its generic tracked-read path. `merge(defaults, store)` constructs ~30% faster than the copy did and reads ~15% faster; `omit(store)` reads at parity. + The views tell the truth: `Object.getOwnPropertyDescriptor(view, key)` reports a data descriptor only when the key is a data property of a plain leaf (the compiler's encoding of a static prop) and an accessor for a getter, a store key, or a memo source. Together with the new internal `hasStaticKeys()`, `spread()` now skips the children effect for static children behind `omit`/`merge` layers (#3388 through views). Behavior changes: @@ -24,4 +26,4 @@ Behavior changes: - Sources are treated as own-keyed; a key added to a plain source after merging is not seen (the copy did not see it either). - Enumerating a view through its traps (`for…in`, `Object.keys`, `{ ...view }`) costs a trap per key, as any proxy does; the internal consumers avoid it. Environments without `Proxy` keep the copy paths. -Internal helpers for consumers, exported from `solid-js`: `omitView(o)`, `sourceKeys(entry)`, `sourceHas(entry, key)`, `sourceGet(entry, key)`, `hasStaticKeys(o)`, `resolvedTable(o)`. +Internal helpers for consumers, exported from `solid-js`: `viewOf(o)`, `mergeView(o)`, `omitView(o)`, `sourceKeys(entry, kind)`, `sourceHas(entry, kind, key)`, `sourceGet(entry, kind, key)`, `hasStaticKeys(o)`, `resolvedTable(o)`, the `SOURCE_*` kinds. diff --git a/packages/signals/src/store/index.ts b/packages/signals/src/store/index.ts index e48327460..9d665c78d 100644 --- a/packages/signals/src/store/index.ts +++ b/packages/signals/src/store/index.ts @@ -14,14 +14,22 @@ export type { Merge, Omit } from "./utils.js"; export { isWrappable, $TRACK, $PROXY, $TARGET } from "./store.js"; export { mergeSources, + mergeView, + viewOf, omitView, sourceKeys, sourceHas, sourceGet, hasStaticKeys, resolvedTable, - OmitView + OmitView, + MergeView, + SOURCE_PLAIN, + SOURCE_OMIT, + SOURCE_PROXY, + SOURCE_MEMO } from "./utils.js"; +export type { SourceKind } from "./utils.js"; import type { NoFn, ProjectionOptions, Store, StoreOptions, StoreSetter } from "./store.js"; import type { Refreshable } from "../core/index.js"; diff --git a/packages/signals/src/store/utils.ts b/packages/signals/src/store/utils.ts index 378cf8278..67dc17135 100644 --- a/packages/signals/src/store/utils.ts +++ b/packages/signals/src/store/utils.ts @@ -44,8 +44,23 @@ type _Merge = T extends [ export type Merge = Simplify<_Merge>; -function resolveSource(s: any) { - return !(s = typeof s === "function" ? s() : s) ? {} : s; +/** @internal What a source ENTRY is, decided once when the view is built + * (`merge()` learns it while flattening; `omit()` from its argument) and + * carried beside the entry — `MergeView.kinds[i]`, `OmitView.kind` — so no + * read has to ask. Asking is the cost: any brand check on a Proxy is a trap + * (`instanceof` is a `getPrototypeOf` trap, ~20 ns on a store — as much as + * the read itself), and a merge over a store did two per read. */ +export const SOURCE_PLAIN = 0; // a plain object: own keys fixed, data is data +export const SOURCE_OMIT = 1; // an `OmitView` record (only as a merge entry) +export const SOURCE_PROXY = 2; // a store or foreign proxy: everything is a trap +export const SOURCE_MEMO = 3; // a function source (merge's memo): swaps objects +export type SourceKind = 0 | 1 | 2 | 3; + +const EMPTY = Object.freeze({}); +// The object behind a LEAF entry (any kind but OMIT): a memo is read +// (tracked, as merge's own reads are) and a nullish result has no keys. +function leafOf(s: any, kind: SourceKind): any { + return kind === SOURCE_MEMO ? ((s = s()) == null ? EMPTY : s) : s; } const $SOURCES = Symbol(__DEV__ ? "MERGE_SOURCE" : 0); @@ -74,8 +89,14 @@ const $VIEW = Symbol(__DEV__ ? "MERGE_VIEW" : 0); export class OmitView { /** see `resolvedTable` */ table: Map | null | undefined = undefined; + /** see `tableOwnKeys` / `tableDescriptor` */ + keys: (string | symbol)[] | undefined = undefined; + descs: Map | undefined = undefined; constructor( public source: any, + /** of `source` — PLAIN, PROXY (a store, or a merge proxy when `entries` + * is set) or MEMO; never OMIT, a view over a view folds into one. */ + public kind: SourceKind, public hidden: Hidden, public entries?: OmitView[] ) {} @@ -97,13 +118,9 @@ function combineHidden(a: Hidden, b: Hidden): Hidden { (typeof b === "function" ? b(key) : b.includes(key)); } -const EMPTY = Object.freeze({}); -// The object a view filters, with a merge memo leaf resolved (tracked, as -// merge's own reads are) and a nullish result read as no keys. +// The object a view filters (see `leafOf`). function viewSource(view: OmitView): any { - let s = view.source; - if (typeof s === "function") s = s(); - return s == null ? EMPTY : s; + return leafOf(view.source, view.kind); } // Whether a `$PROXY`-marked object is one of OUR views rather than a store @@ -122,46 +139,53 @@ export function omitView(o: any): OmitView | undefined { } // A props SOURCE ENTRY is a plain object, a proxy (store, merge, omit — the -// last two are normally unwrapped first: `mergeSources` / `omitView`), or an -// `OmitView` record. These three answer for an entry what `Object.keys` / -// `in` / `[]` answer for an object, so every consumer walks entries with one -// code path and an `OmitView` is filtered rather than materialized. - -/** @internal Own string keys of a source entry — every consumer skips symbols - * itself. A proxy answers through ONE `ownKeys` trap (a store's keeps the key - * set tracked); `Object.keys` on a proxy would add a descriptor trap per key. */ -export function sourceKeys(s: any): (string | symbol)[] { - if (s instanceof OmitView) { - const keys = sourceKeys(viewSource(s)); +// last two are normally unwrapped first: `mergeView` / `omitView`), a memo, +// or an `OmitView` record, and travels with its kind. These three answer for +// an entry what `Object.keys` / `in` / `[]` answer for an object, so every +// consumer walks entries with one code path, an `OmitView` is filtered +// rather than materialized, and nothing is asked of a proxy but the read. + +// Own keys of a leaf: a proxy answers through ONE `ownKeys` trap (a store's +// keeps the key set tracked; `Object.keys` on a proxy would add a descriptor +// trap per key); a plain object its enumerable string keys. What a memo +// holds is only known once read. +function leafKeys(leaf: any, kind: SourceKind): (string | symbol)[] { + if (kind === SOURCE_PLAIN) return Object.keys(leaf); + if (kind === SOURCE_PROXY || leaf[$PROXY] === leaf) return Reflect.ownKeys(leaf); + return Object.keys(leaf); +} + +/** @internal Own string keys of a source entry — every consumer skips + * symbols itself. */ +export function sourceKeys(s: any, kind: SourceKind): (string | symbol)[] { + if (kind === SOURCE_OMIT) { + const keys = leafKeys(viewSource(s), s.kind); const out: (string | symbol)[] = []; for (let i = 0; i < keys.length; i++) if (!isHidden(s, keys[i])) out.push(keys[i]); return out; } - return s[$PROXY] === s ? Reflect.ownKeys(s) : Object.keys(s); + return leafKeys(leafOf(s, kind), kind); } /** @internal `key in entry`. */ -export function sourceHas(s: any, key: PropertyKey): boolean { - return s instanceof OmitView ? !isHidden(s, key) && sourceHas(viewSource(s), key) : key in s; +export function sourceHas(s: any, kind: SourceKind, key: PropertyKey): boolean { + if (kind === SOURCE_OMIT) return !isHidden(s, key) && key in viewSource(s); + return key in leafOf(s, kind); } /** @internal `entry[key]` — the source's getter runs once, here. */ -export function sourceGet(s: any, key: PropertyKey): any { - return s instanceof OmitView - ? isHidden(s, key) - ? undefined - : sourceGet(viewSource(s), key) - : s[key]; +export function sourceGet(s: any, kind: SourceKind, key: PropertyKey): any { + if (kind === SOURCE_OMIT) return isHidden(s, key) ? undefined : viewSource(s)[key]; + return leafOf(s, kind)[key]; } -// A leaf whose own key set is fixed: a plain object. Not a store (its key -// set is a tracked signal), not a merge memo source (it swaps whole objects), -// and not any proxy that declares itself with `$PROXY in s` — a frames slot -// proxy answers `has` for every key and lists none, so only the `in` walk -// is right for it. -function leafHasStaticKeys(leaf: any): boolean { - if (leaf instanceof OmitView) leaf = leaf.source; - return typeof leaf !== "function" && !($PROXY in leaf); +// An entry whose own key set is fixed: a plain object, or a view over one. +// Not a store (its key set is a tracked signal), not a merge memo source (it +// swaps whole objects), and not any proxy that declares itself with +// `$PROXY in s` — a frames slot proxy answers `has` for every key and lists +// none, so only the `in` walk is right for it. +function entryHasStaticKeys(s: any, kind: SourceKind): boolean { + return kind === SOURCE_PLAIN || (kind === SOURCE_OMIT && s.kind === SOURCE_PLAIN); } /** @internal Whether the own key set of a props object cannot change @@ -174,13 +198,20 @@ function leafHasStaticKeys(leaf: any): boolean { export function hasStaticKeys(o: any): boolean { if (!($PROXY in o)) return true; if (o[$TARGET] !== undefined) return false; - const sources = o[$SOURCES]; - if (sources !== undefined) { - for (let i = 0; i < sources.length; i++) if (!leafHasStaticKeys(sources[i])) return false; + const merged: MergeView | undefined = o[$VIEW]; + if (merged !== undefined) { + const f = merged.sources, + k = merged.kinds; + for (let i = 0; i < f.length; i++) if (!entryHasStaticKeys(f[i], k[i])) return false; return true; } - const view = o[$OMIT]; - return view !== undefined && leafHasStaticKeys(view); + const view: OmitView | undefined = o[$OMIT]; + if (view === undefined) return false; + // An omit over a merge is its filtered leaf entries. + const entries = view.entries; + if (entries === undefined) return view.kind === SOURCE_PLAIN; + for (let i = 0; i < entries.length; i++) if (entries[i].kind !== SOURCE_PLAIN) return false; + return true; } function accessorDescriptor(get: () => any, enumerable = true): PropertyDescriptor { @@ -206,20 +237,22 @@ function accessorDescriptor(get: () => any, enumerable = true): PropertyDescript // shadowing walk did), so a store is not asked a second time. function sourceDescriptor( s: any, + kind: SourceKind, key: PropertyKey, present = false ): PropertyDescriptor | undefined { - if (s instanceof OmitView) { + if (kind === SOURCE_OMIT) { if (isHidden(s, key)) return undefined; - const raw = s.source; - if (typeof raw === "function") { - return present || sourceHas(viewSource(s), key) - ? accessorDescriptor(() => sourceGet(s, key)) - : undefined; - } - return sourceDescriptor(raw, key, present); + return sourceDescriptor(s.source, s.kind, key, present); } - if ($PROXY in s) { + // A memo source (`merge(() => …)`) is reactive wholesale: whatever shape + // the memo's current object has, the key is an accessor here. + if (kind === SOURCE_MEMO) { + return present || key in leafOf(s, kind) + ? accessorDescriptor(() => leafOf(s, kind)[key]) + : undefined; + } + if (kind === SOURCE_PROXY) { // Another view (an omit's source may be a merge proxy) already answers // truthfully. A store's reported "data" is a signal, and a foreign proxy // (frames slot props) has no own descriptors: for both, existence is @@ -240,24 +273,48 @@ function sourceDescriptor( // Own ENUMERABLE keys, symbols included, of an entry — the user-facing key // set (`Object.keys(merged)`), where enumerability matters (#2769). -function sourceEnumerableKeys(s: any): (string | symbol)[] { - if (s instanceof OmitView) { - const keys = sourceEnumerableKeys(viewSource(s)); +function sourceEnumerableKeys(s: any, kind: SourceKind): (string | symbol)[] { + if (kind === SOURCE_OMIT) { + const keys = ownEnumerableKeys(viewSource(s)); const out: (string | symbol)[] = []; for (let i = 0; i < keys.length; i++) if (!isHidden(s, keys[i])) out.push(keys[i]); return out; } - return ownEnumerableKeys(s); + return ownEnumerableKeys(leafOf(s, kind)); } // The target of a merge() proxy: the flattened sources, read by one shared // handler — like OmitView, no per-instance closures. `sources` is what // `$SOURCES` answers. -class MergeView { +/** @internal */ +export class MergeView { /** key → the plain leaf that owns it (later sources win), built on first * read when every leaf has static keys; `null` when one doesn't. */ table: Map | null | undefined = undefined; - constructor(public sources: any[]) {} + /** see `tableOwnKeys` / `tableDescriptor` */ + keys: (string | symbol)[] | undefined = undefined; + descs: Map | undefined = undefined; + constructor( + public sources: any[], + /** `kinds[i]` is what `sources[i]` is (see `SourceKind`). */ + public kinds: SourceKind[] + ) {} +} + +/** @internal The `MergeView` behind a `merge()` proxy — its flattened + * `sources` with their `kinds` — or undefined. */ +export function mergeView(o: any): MergeView | undefined { + return o != null && $PROXY in o && o[$TARGET] === undefined ? o[$VIEW] : undefined; +} + +/** @internal The record behind a merge() or omit() proxy — a `MergeView` + * (flattened `sources` with their `kinds`) or an `OmitView` — or undefined + * for anything else (a plain object, a store, a foreign proxy). Two fast + * traps on a store, none on a plain object. */ +export function viewOf(o: any): MergeView | OmitView | undefined { + if (o == null || !($PROXY in o) || o[$TARGET] !== undefined) return undefined; + const merged = o[$VIEW]; + return merged !== undefined ? merged : o[$OMIT]; } /** @internal The resolved key table of a merge/omit view — every own key of @@ -278,20 +335,17 @@ export function resolvedTable(o: any): Map | undefined { if (o == null || !($PROXY in o) || o[$TARGET] !== undefined) return undefined; const view = o[$OMIT]; if (view !== undefined) return omitTable(view); - const sources = o[$SOURCES]; - return sources === undefined ? undefined : mergeTable(mergeViewOf(o)); -} - -function mergeViewOf(proxy: any): MergeView { - return proxy[$VIEW]; + const merged = o[$VIEW]; + return merged === undefined ? undefined : mergeTable(merged); } function mergeTable(view: MergeView): Map | undefined { let table = view.table; if (table === undefined) { - const f = view.sources; + const f = view.sources, + k = view.kinds; for (let i = 0; i < f.length; i++) { - if (!leafHasStaticKeys(f[i])) { + if (!entryHasStaticKeys(f[i], k[i])) { view.table = null; return undefined; } @@ -299,7 +353,7 @@ function mergeTable(view: MergeView): Map | undefined { table = new Map(); for (let i = 0; i < f.length; i++) { const leaf = f[i]; - if (leaf instanceof OmitView) { + if (k[i] === SOURCE_OMIT) { const src = leaf.source; const keys = Reflect.ownKeys(src); for (let j = 0; j < keys.length; j++) { @@ -334,8 +388,8 @@ function omitTable(view: OmitView): Map | undefined { if (table === undefined) { const src = view.source; let base: Map | undefined; - if (typeof src === "function" || src == null) base = undefined; - else if ($PROXY in src) base = resolvedTable(src); + if (view.kind === SOURCE_MEMO) base = undefined; + else if (view.kind === SOURCE_PROXY) base = resolvedTable(src); else { base = new Map(); const keys = Reflect.ownKeys(src); @@ -353,13 +407,49 @@ function omitTable(view: OmitView): Map | undefined { } // The user-facing key set of a resolved table: its keys that are enumerable -// on the leaf that owns them (`Object.keys(merged)`, #2769). +// on the leaf that owns them (`Object.keys(merged)`, #2769). Fixed, like the +// table, so it is built once per view: an `ownKeys` trap may hand back the +// same array every time (the engine copies it). const propertyIsEnumerable = Object.prototype.propertyIsEnumerable; -function tableKeys(table: Map): (string | symbol)[] { - const out: (string | symbol)[] = []; - for (const [key, leaf] of table) - if (propertyIsEnumerable.call(leaf, key)) out.push(key as string | symbol); - return out; +function tableOwnKeys(view: MergeView | OmitView, table: Map) { + let keys = view.keys; + if (keys === undefined) { + keys = view.keys = []; + for (const [key, leaf] of table) + if (propertyIsEnumerable.call(leaf, key)) keys.push(key as string | symbol); + } + return keys; +} + +// The descriptor for a table key — its owning leaf's, truthful (see +// `sourceDescriptor`) — with the shape cached per key so an enumeration +// (`for…in`, `Object.keys`, `{...props}`: a descriptor trap per key, on +// every pass) does not re-read the leaf's descriptor and re-allocate a +// getter each time. An accessor reads live, so its descriptor is reused as +// is; a data descriptor is rebuilt with the current value. +function tableDescriptor( + view: MergeView | OmitView, + table: Map, + key: PropertyKey +): PropertyDescriptor | undefined { + const leaf = table.get(key); + if (leaf === undefined) return undefined; + let descs = view.descs; + if (descs === undefined) descs = view.descs = new Map(); + let cached = descs.get(key); + if (cached === undefined) { + cached = sourceDescriptor(leaf, SOURCE_PLAIN, key); + if (cached === undefined) return undefined; + descs.set(key, cached); + return cached; + } + if (cached.get !== undefined) return cached; + return { + configurable: true, + enumerable: cached.enumerable, + writable: cached.writable, + value: leaf[key] + }; } function mergeGet(view: MergeView, property: PropertyKey): any { @@ -368,64 +458,75 @@ function mergeGet(view: MergeView, property: PropertyKey): any { const leaf = table.get(property); return leaf === undefined ? undefined : leaf[property]; } - const f = view.sources; + const f = view.sources, + k = view.kinds; for (let i = f.length - 1; i >= 0; i--) { - const s = resolveSource(f[i]); - if (sourceHas(s, property)) return sourceGet(s, property); + const kind = k[i]; + if (kind === SOURCE_OMIT) { + const v: OmitView = f[i]; + if (isHidden(v, property)) continue; + const s = viewSource(v); + if (property in s) return s[property]; + } else { + const s = leafOf(f[i], kind); + if (property in s) return s[property]; + } } } const mergeTraps: ProxyHandler = { get(view, property, receiver) { if (property === $PROXY) return receiver; - if (property === $TARGET) return undefined; + if (property === $TARGET || property === $OMIT) return undefined; if (property === $SOURCES) return view.sources; if (property === $VIEW) return view; return mergeGet(view, property); }, has(view, property) { if (property === $PROXY) return true; - if (property === $TARGET || property === $SOURCES || property === $VIEW) return false; + if (property === $TARGET || property === $OMIT || property === $SOURCES || property === $VIEW) + return false; const table = mergeTable(view); if (table !== undefined) return table.has(property); - const f = view.sources; - for (let i = f.length - 1; i >= 0; i--) { - if (sourceHas(resolveSource(f[i]), property)) return true; - } + const f = view.sources, + k = view.kinds; + for (let i = f.length - 1; i >= 0; i--) if (sourceHas(f[i], k[i], property)) return true; return false; }, set: trueFn, deleteProperty: trueFn, getOwnPropertyDescriptor(view, property) { - if (property === $PROXY || property === $SOURCES || property === $VIEW) return undefined; + if ( + property === $PROXY || + property === $TARGET || + property === $OMIT || + property === $SOURCES || + property === $VIEW + ) + return undefined; const table = mergeTable(view); - if (table !== undefined) { - const leaf = table.get(property); - return leaf === undefined ? undefined : sourceDescriptor(leaf, property); - } - const f = view.sources; + if (table !== undefined) return tableDescriptor(view, table, property); + const f = view.sources, + k = view.kinds; for (let i = f.length - 1; i >= 0; i--) { - const raw = f[i]; - const s = resolveSource(raw); - if (!sourceHas(s, property)) continue; - // A memo source (`merge(() => …)`) is reactive wholesale: whatever - // shape the memo's current object has, the key is an accessor here. - if (typeof raw === "function") return accessorDescriptor(() => mergeGet(view, property)); + if (!sourceHas(f[i], k[i], property)) continue; // `in` also answers for inherited keys, which have no own descriptor. return ( - sourceDescriptor(s, property, true) ?? accessorDescriptor(() => mergeGet(view, property)) + sourceDescriptor(f[i], k[i], property, true) ?? + accessorDescriptor(() => mergeGet(view, property)) ); } return undefined; }, ownKeys(view) { const table = mergeTable(view); - if (table !== undefined) return tableKeys(table); + if (table !== undefined) return tableOwnKeys(view, table); // Same order as the table's: a key at the position of its last source. const keys = new Set(); - const f = view.sources; + const f = view.sources, + k = view.kinds; for (let i = 0; i < f.length; i++) { - const sourceKeys = sourceEnumerableKeys(resolveSource(f[i])); + const sourceKeys = sourceEnumerableKeys(f[i], k[i]); for (let j = 0; j < sourceKeys.length; j++) { const key = sourceKeys[j]; if (keys.has(key)) keys.delete(key); @@ -444,7 +545,8 @@ const mergeTraps: ProxyHandler = { const omitTraps: ProxyHandler = { get(view, property, receiver) { if (property === $PROXY) return receiver; - if (property === $TARGET) return undefined; + // $VIEW is the underlying merge's record, UNFILTERED: never forwarded. + if (property === $TARGET || property === $VIEW) return undefined; if (property === $OMIT) return view; // $SOURCES answers the FILTERED leaf entries (or nothing for a plain // source) — never the underlying merge's own sources, which would hand a @@ -458,37 +560,42 @@ const omitTraps: ProxyHandler = { } } if (isHidden(view, property)) return undefined; - return view.source[property]; + return viewSource(view)[property]; }, has(view, property) { if (property === $PROXY) return true; - if (property === $TARGET || property === $SOURCES || property === $OMIT) return false; + if (property === $TARGET || property === $VIEW || property === $SOURCES || property === $OMIT) + return false; if (view.entries !== undefined) { const table = omitTable(view); if (table !== undefined) return table.has(property); } if (isHidden(view, property)) return false; - return property in view.source; + return property in viewSource(view); }, set: trueFn, deleteProperty: trueFn, getOwnPropertyDescriptor(view, property) { - if (property === $PROXY || property === $OMIT || property === $SOURCES) return undefined; + if ( + property === $PROXY || + property === $TARGET || + property === $VIEW || + property === $OMIT || + property === $SOURCES + ) + return undefined; if (view.entries !== undefined) { const table = omitTable(view); - if (table !== undefined) { - const leaf = table.get(property); - return leaf === undefined ? undefined : sourceDescriptor(leaf, property); - } + if (table !== undefined) return tableDescriptor(view, table, property); } - return sourceDescriptor(view, property); + return sourceDescriptor(view, SOURCE_OMIT, property); }, ownKeys(view) { if (view.entries !== undefined) { const table = omitTable(view); - if (table !== undefined) return tableKeys(table); + if (table !== undefined) return tableOwnKeys(view, table); } - const keys = Reflect.ownKeys(view.source); + const keys = Reflect.ownKeys(viewSource(view)); const out: (string | symbol)[] = []; for (let i = 0; i < keys.length; i++) if (!isHidden(view, keys[i])) out.push(keys[i]); return out; @@ -531,6 +638,7 @@ export function mergeSources(o: any): any[] | undefined { export function merge(...sources: T): Merge { if (sources.length === 1 && typeof sources[0] !== "function") return sources[0] as any; const flattened: T[] = []; + const kinds: SourceKind[] = []; // The one non-falsy source, if there is exactly one: it IS the merge. let only: unknown = undefined; let count = 0; @@ -541,27 +649,46 @@ export function merge(...sources: T): Merge { only = s; if (typeof s === "function") { flattened.push(createMemo(s as () => any) as any); + kinds.push(SOURCE_MEMO); continue; } - if ($PROXY in (s as object) && (s as any)[$TARGET] === undefined) { - // A merge() proxy is flattened through: its writes are no-ops, so its - // sources are exactly what it reads. An omit() proxy over a merge - // answers $SOURCES with its FILTERED leaf views, never the merge's own - // sources (#3014); an omit() of a plain object joins as its view - // record. Either way the filter travels with the entry and the hidden - // keys stay hidden. A store (`$TARGET`) is a leaf as it is. - const childSources = (s as object)[$SOURCES]; - if (childSources) { - for (let j = 0; j < childSources.length; j++) flattened.push(childSources[j]); - continue; - } - const view = (s as object)[$OMIT]; - if (view) { - flattened.push(view); - continue; + if ($PROXY in (s as object)) { + // A store (`$TARGET`) is a leaf as it is. A merge() proxy is flattened + // through: its writes are no-ops, so its sources are exactly what it + // reads. An omit() proxy over a merge answers $SOURCES with its + // FILTERED leaf views, never the merge's own sources (#3014); an + // omit() of a plain object joins as its view record. Either way the + // filter travels with the entry and the hidden keys stay hidden. + if ((s as any)[$TARGET] === undefined) { + const child: MergeView | undefined = (s as any)[$VIEW]; + if (child !== undefined) { + for (let j = 0; j < child.sources.length; j++) { + flattened.push(child.sources[j]); + kinds.push(child.kinds[j]); + } + continue; + } + const view: OmitView | undefined = (s as any)[$OMIT]; + if (view !== undefined) { + const entries = view.entries; + if (entries !== undefined) { + for (let j = 0; j < entries.length; j++) { + flattened.push(entries[j] as any); + kinds.push(SOURCE_OMIT); + } + } else { + flattened.push(view as any); + kinds.push(SOURCE_OMIT); + } + continue; + } } + flattened.push(s as any); + kinds.push(SOURCE_PROXY); + continue; } flattened.push(s as any); + kinds.push(SOURCE_PLAIN); } if (SUPPORTS_PROXY) { if (count === 1 && typeof only !== "function") return only as any; @@ -577,7 +704,7 @@ export function merge(...sources: T): Merge { // copies: `{...merged}`, which the traps answer truthfully). Copies of // the result never carry $SOURCES (#3384): `ownKeys` answers only the // sources' keys. - return new Proxy(new MergeView(flattened), mergeTraps) as unknown as Merge; + return new Proxy(new MergeView(flattened, kinds), mergeTraps) as unknown as Merge; } // No Proxy: an eager descriptor copy, semantics as close to the view as a @@ -669,28 +796,39 @@ export function omit(props: any, ...keys: any[]): any { // A view over a view flattens: one record, both filters, the original // source — so a consumer walks the real object however deep the omits go. let source = props; - const inner = omitView(props); - if (inner !== undefined) { - source = inner.source; - hidden = combineHidden(inner.hidden, hidden); - } - // Over a merge() proxy: one leaf view per flattened source (see OmitView). - // A flattened source that is itself a view — an earlier omit() this merge - // was built over — folds into one record with both filters, so a - // component chain of omit/merge/omit/merge stays one level deep. - const merged = mergeSources(source); + let kind: SourceKind = SOURCE_PLAIN; let entries: OmitView[] | undefined; - if (merged !== undefined) { - entries = new Array(merged.length); - for (let i = 0; i < merged.length; i++) { - const leaf = merged[i]; - entries[i] = - leaf instanceof OmitView - ? new OmitView(leaf.source, combineHidden(leaf.hidden, hidden)) - : new OmitView(leaf, hidden); + if (typeof props === "function") kind = SOURCE_MEMO; + else if ($PROXY in props) { + kind = SOURCE_PROXY; + if (props[$TARGET] === undefined) { + const inner: OmitView | undefined = props[$OMIT]; + if (inner !== undefined) { + source = inner.source; + kind = inner.kind; + hidden = combineHidden(inner.hidden, hidden); + } + // Over a merge() proxy: one leaf view per flattened source (see + // OmitView). A flattened source that is itself a view — an earlier + // omit() this merge was built over — folds into one record with both + // filters, so a component chain of omit/merge/omit/merge stays one + // level deep. + const merged: MergeView | undefined = kind === SOURCE_PROXY ? source[$VIEW] : undefined; + if (merged !== undefined) { + const f = merged.sources, + k = merged.kinds; + entries = new Array(f.length); + for (let i = 0; i < f.length; i++) { + const leaf = f[i]; + entries[i] = + k[i] === SOURCE_OMIT + ? new OmitView(leaf.source, leaf.kind, combineHidden(leaf.hidden, hidden)) + : new OmitView(leaf, k[i], hidden); + } + } } } - return new Proxy(new OmitView(source, hidden, entries), omitTraps); + return new Proxy(new OmitView(source, kind, hidden, entries), omitTraps); } const result: Record = {}; const propNames = Object.getOwnPropertyNames(props); diff --git a/packages/signals/tests/store/utilities.test.ts b/packages/signals/tests/store/utilities.test.ts index d872e3926..4842478af 100644 --- a/packages/signals/tests/store/utilities.test.ts +++ b/packages/signals/tests/store/utilities.test.ts @@ -7,11 +7,22 @@ import { deep, flush, getOwner, + $TARGET, merge, mergeSources, hasStaticKeys, omit, OmitView, + MergeView, + viewOf, + resolvedTable, + sourceKeys, + sourceHas, + sourceGet, + SOURCE_PLAIN, + SOURCE_OMIT, + SOURCE_PROXY, + SOURCE_MEMO, reconcile, snapshot, type Store @@ -821,6 +832,127 @@ describe("view descriptors", () => { expect(Object.getOwnPropertyDescriptor(l5, "role")!.value).toBe("button"); expect(Object.keys(l5).sort()).toEqual(["class", "extra", "label", "role"]); }); + // A store-shaped proxy that logs every trap it is asked. `$PROXY in`, + // `$TARGET` and `$PROXY` are a store's fast paths; anything else — an + // unknown symbol taking its generic read path, `getPrototypeOf` from an + // `instanceof`, a descriptor per key — is a cost per read that the views + // must not add over a direct read of the store. + function storeShaped(data: Record) { + const log: string[] = []; + const target = {}; + const proxy: any = new Proxy(target, { + get(_, key, receiver) { + if (key === $PROXY) return receiver; + if (key === $TARGET) return target; + log.push(`get ${String(key)}`); + return data[key as string]; + }, + has(_, key) { + if (key === $PROXY || key === $TARGET) return true; + log.push(`has ${String(key)}`); + return key in data; + }, + ownKeys() { + log.push("ownKeys"); + return Reflect.ownKeys(data); + }, + getOwnPropertyDescriptor(_, key) { + log.push(`descriptor ${String(key)}`); + const desc = Reflect.getOwnPropertyDescriptor(data, key); + return desc && { ...desc, configurable: true }; + }, + getPrototypeOf() { + log.push("getPrototypeOf"); + return Object.prototype; + } + }); + return { proxy, log }; + } + test("a read through a merge or omit asks a store exactly what a direct read would", () => { + const { proxy: store, log } = storeShaped({ a: 1, b: 2 }); + const merged: any = merge({ a: 0, z: 9 }, store); + expect(log).toEqual([]); + expect(merged.a).toBe(1); + expect(log).toEqual(["has a", "get a"]); + log.length = 0; + expect(merged.z).toBe(9); + expect(log).toEqual(["has z"]); + log.length = 0; + expect("b" in merged).toBe(true); + expect(log).toEqual(["has b"]); + log.length = 0; + + const rest: any = omit(store, "a"); + expect(log).toEqual([]); + expect(rest.b).toBe(2); + expect(rest.a).toBeUndefined(); + expect(log).toEqual(["get b"]); + log.length = 0; + + // Enumeration: one ownKeys, then per key one existence check for the + // descriptor — never the store's own descriptor trap. + expect(Object.keys(rest)).toEqual(["b"]); + expect(log).toEqual(["ownKeys", "has b"]); + log.length = 0; + // A merge with a store leaf has no resolved table: the user-facing key + // set is the enumerable keys of each source (a descriptor per store key, + // #2769), then each key is a shadowing walk (`has` on the store) that + // the descriptor reuses. + expect(Object.keys(merged)).toEqual(["z", "a", "b"]); + expect(log).toEqual(["ownKeys", "descriptor a", "descriptor b", "has z", "has a", "has b"]); + log.length = 0; + + // The consumers' entry helpers over the store's kind: no probe at all. + expect(sourceKeys(store, SOURCE_PROXY)).toEqual(["a", "b"]); + expect(sourceHas(store, SOURCE_PROXY, "a")).toBe(true); + expect(sourceGet(store, SOURCE_PROXY, "a")).toBe(1); + expect(log).toEqual(["ownKeys", "has a", "get a"]); + log.length = 0; + expect(hasStaticKeys(store)).toBe(false); + expect(viewOf(store)).toBeUndefined(); + expect(resolvedTable(store)).toBeUndefined(); + expect(log).toEqual([]); + }); + test("merge and omit record what each source is, once", () => { + const store = createStore({ s: 1 })[0]; + const plain = { p: 1 }; + const rest = omit({ o: 1, hide: 1 }, "hide"); + const memo = () => ({ m: 1 }); + const merged = merge(plain, store, rest, memo); + const view = viewOf(merged) as MergeView; + expect(view).toBeInstanceOf(MergeView); + expect(view.kinds).toEqual([SOURCE_PLAIN, SOURCE_PROXY, SOURCE_OMIT, SOURCE_MEMO]); + expect(view.sources[0]).toBe(plain); + expect(view.sources[1]).toBe(store); + expect(view.sources[2]).toBeInstanceOf(OmitView); + expect(typeof view.sources[3]).toBe("function"); + // A merge among the sources flattens with its kinds. + const outer = viewOf(merge({ x: 1 }, merged)) as MergeView; + expect(outer.kinds).toEqual([ + SOURCE_PLAIN, + SOURCE_PLAIN, + SOURCE_PROXY, + SOURCE_OMIT, + SOURCE_MEMO + ]); + // An omit records its source's kind, and over a merge one entry per leaf. + expect((viewOf(omit(store, "s")) as OmitView).kind).toBe(SOURCE_PROXY); + expect((viewOf(omit(plain, "p")) as OmitView).kind).toBe(SOURCE_PLAIN); + const overProxy: any = omit(merged, "p"); + const over = viewOf(overProxy) as OmitView; + expect(over.kind).toBe(SOURCE_PROXY); + expect(over.entries!.map(e => e.kind)).toEqual([ + SOURCE_PLAIN, + SOURCE_PROXY, + SOURCE_PLAIN, + SOURCE_MEMO + ]); + expect(overProxy.p).toBeUndefined(); + expect(overProxy.s).toBe(1); + expect(overProxy.o).toBe(1); + expect(overProxy.hide).toBeUndefined(); + expect(overProxy.m).toBe(1); + }); test("hasStaticKeys: plain objects and views over them; not stores, memos, or views over them", () => { createRoot(() => { const [store] = createStore({ a: 1 }); diff --git a/packages/solid/src/index.ts b/packages/solid/src/index.ts index 06ad6b6b0..e6366f78f 100644 --- a/packages/solid/src/index.ts +++ b/packages/solid/src/index.ts @@ -21,12 +21,20 @@ export { mapArray, merge, mergeSources, + mergeView, + viewOf, + OmitView, + MergeView, omitView, sourceKeys, sourceHas, sourceGet, hasStaticKeys, resolvedTable, + SOURCE_PLAIN, + SOURCE_OMIT, + SOURCE_PROXY, + SOURCE_MEMO, omit, onCleanup, onSettled, @@ -52,6 +60,7 @@ export { } from "@solidjs/signals"; export type { + SourceKind, Accessor, ComputeFunction, EffectBundle, diff --git a/packages/solid/src/server/index.ts b/packages/solid/src/server/index.ts index 7cc0cbad7..024f75922 100644 --- a/packages/solid/src/server/index.ts +++ b/packages/solid/src/server/index.ts @@ -37,12 +37,20 @@ export { mapArray, merge, mergeSources, + mergeView, + viewOf, + OmitView, + MergeView, omitView, sourceKeys, sourceHas, sourceGet, hasStaticKeys, resolvedTable, + SOURCE_PLAIN, + SOURCE_OMIT, + SOURCE_PROXY, + SOURCE_MEMO, omit, onCleanup, onSettled, @@ -72,6 +80,7 @@ export { // All type re-exports from signals export type { + SourceKind, Accessor, ComputeFunction, EffectFunction, diff --git a/packages/solid/src/server/signals.ts b/packages/solid/src/server/signals.ts index bfdf4c72b..f59211de1 100644 --- a/packages/solid/src/server/signals.ts +++ b/packages/solid/src/server/signals.ts @@ -33,12 +33,20 @@ export { storePath, $PROXY, $TRACK, + mergeView, + viewOf, + OmitView, + MergeView, omitView, sourceKeys, sourceHas, sourceGet, hasStaticKeys, - resolvedTable + resolvedTable, + SOURCE_PLAIN, + SOURCE_OMIT, + SOURCE_PROXY, + SOURCE_MEMO } from "@solidjs/signals"; // === Type re-exports === @@ -53,6 +61,7 @@ import type { export type SourceAccessor = Refreshable>; export type { + SourceKind, Accessor, ComputeFunction, EffectFunction, diff --git a/packages/universal/src/universal.ts b/packages/universal/src/universal.ts index 268d5430d..266a00c9d 100644 --- a/packages/universal/src/universal.ts +++ b/packages/universal/src/universal.ts @@ -9,13 +9,18 @@ import { createMemo, createRenderEffect, flush, - mergeSources, - omitView, + $PROXY, + viewOf, + OmitView, sourceKeys, sourceHas, sourceGet, hasStaticKeys, - resolvedTable + resolvedTable, + SOURCE_PLAIN, + SOURCE_OMIT, + SOURCE_PROXY, + SOURCE_MEMO } from "solid-js"; export interface RendererOptions { @@ -410,15 +415,19 @@ export function createRenderer({ node, () => { for (let i = props.length - 1; i >= 0; i--) { - const s = resolveEntry(props[i]); - if (s != null && sourceHas(s, "children")) return sourceGet(s, "children"); + const s = resolveSource(props[i]); + if (s != null && entryHas(s, "children")) return entryGet(s, "children"); } }, undefined, undefined, childrenOptions() ); - effect(() => collectSources({}, props), apply, named(options, "renderer spread props")); + effect( + () => collectSources({}, props, undefined), + apply, + named(options, "renderer spread props") + ); return prevProps; } if (!skipChildren) { @@ -438,8 +447,8 @@ export function createRenderer({ insert( node, () => { - const s = resolveEntry(props); - return s != null ? sourceGet(s, "children") : undefined; + const s = resolveSource(props); + return s != null ? entryGet(s, "children") : undefined; }, undefined, undefined, @@ -462,9 +471,12 @@ export function createRenderer({ } return newProps; } - const sources = mergeSources(s); - if (sources !== undefined) return collectSources(newProps, sources); - if (s != null) collectProps(newProps, entryOf(s)); + if (s != null) { + const view = viewOf(s); + if (view instanceof OmitView) collectProps(newProps, view, SOURCE_OMIT); + else if (view !== undefined) collectSources(newProps, view.sources, view.kinds); + else collectProps(newProps, s, $PROXY in s ? SOURCE_PROXY : SOURCE_PLAIN); + } return newProps; }, apply, @@ -477,38 +489,55 @@ export function createRenderer({ return typeof s === "function" ? s() : s; } - // A resolved source as the ENTRY a consumer walks: an omit() proxy is - // replaced by its view record, anything else is itself. - function entryOf(s) { - if (s == null) return s; - const view = omitView(s); - return view !== undefined ? view : s; + // `key in s` / `s[key]` for one resolved, non-null spread source: an + // omit() proxy answers from its view record, anything else as itself. + function entryHas(s, key) { + const view = viewOf(s); + return view instanceof OmitView ? sourceHas(view, SOURCE_OMIT, key) : key in s; } - - function resolveEntry(s) { - return entryOf(resolveSource(s)); + function entryGet(s, key) { + const view = viewOf(s); + return view instanceof OmitView ? sourceGet(view, SOURCE_OMIT, key) : s[key]; } // Layered sources into `out`. Every function source is resolved once, up // front, a merge() proxy among them contributes its flattened sources in - // place; keys are then collected left-to-right (Object.assign order), and - // a key any LATER source has is skipped unread. `sourceHas` is merge()'s - // own resolution test, so a proxy source answers through its `has` trap - // and an omit view from its filter. - function collectSources(out, sources) { + // place — each entry with its KIND, so the per-key walk asks nothing of a + // proxy but the read; keys are then collected left-to-right (Object.assign + // order), and a key any LATER source has is skipped unread. `sourceHas` is + // merge()'s own resolution test, so a proxy source answers through its + // `has` trap and an omit view from its filter. + function collectSources(out, sources, kinds) { const resolved = []; - for (let i = 0; i < sources.length; i++) { - const s = resolveSource(sources[i]); - const merged = mergeSources(s); - if (merged !== undefined) { - for (let j = 0; j < merged.length; j++) resolved.push(resolveEntry(merged[j])); - } else resolved.push(entryOf(s)); + const resolvedKinds = []; + for (let i = 0; i < sources.length; i++) + pushEntry(resolved, resolvedKinds, sources[i], kinds !== undefined ? kinds[i] : SOURCE_MEMO); + for (let i = 0; i < resolved.length; i++) + collectProps(out, resolved[i], resolvedKinds[i], resolved, resolvedKinds, i + 1); + return out; + } + + // One source into the resolved entry lists (see @solidjs/web). + function pushEntry(resolved, kinds, s, kind) { + if (kind !== SOURCE_MEMO) { + resolved.push(s); + kinds.push(kind); + return; } - for (let i = 0; i < resolved.length; i++) { - const s = resolved[i]; - if (s != null) collectProps(out, s, resolved, i + 1); + s = resolveSource(s); + if (s == null) return; + const view = viewOf(s); + if (view instanceof OmitView) { + resolved.push(view); + kinds.push(SOURCE_OMIT); + } else if (view !== undefined) { + const f = view.sources, + k = view.kinds; + for (let j = 0; j < f.length; j++) pushEntry(resolved, kinds, f[j], k[j]); + } else { + resolved.push(s); + kinds.push($PROXY in s ? SOURCE_PROXY : SOURCE_PLAIN); } - return out; } // One layer of a spread source into `out`: own string keys (one `ownKeys` @@ -516,17 +545,15 @@ export function createRenderer({ // `children` excluded (it has its own insert), `ref` carried through for // the commit half. With `later` (the sources after this one, from index // `from`), a key one of them defines is shadowed and never read here. - function collectProps(out, s, later?, from?) { - const keys = sourceKeys(s); + function collectProps(out, s, kind, later?, laterKinds?, from?) { + const keys = sourceKeys(s, kind); outer: for (let i = 0; i < keys.length; i++) { const prop = keys[i]; if (typeof prop !== "string" || prop === "children") continue; if (later !== undefined) - for (let j = from; j < later.length; j++) { - const t = later[j]; - if (t != null && sourceHas(t, prop)) continue outer; - } - out[prop] = sourceGet(s, prop); + for (let j = from; j < later.length; j++) + if (sourceHas(later[j], laterKinds[j], prop)) continue outer; + out[prop] = sourceGet(s, kind, prop); } return out; } diff --git a/packages/web/src/client.ts b/packages/web/src/client.ts index 888064282..791699556 100644 --- a/packages/web/src/client.ts +++ b/packages/web/src/client.ts @@ -11,13 +11,17 @@ import { untrack, merge as mergeProps, $PROXY, - mergeSources, - omitView, + viewOf, + OmitView, sourceKeys, sourceHas, sourceGet, hasStaticKeys, resolvedTable, + SOURCE_PLAIN, + SOURCE_OMIT, + SOURCE_PROXY, + SOURCE_MEMO, flatten, createMemo, flush, @@ -803,7 +807,7 @@ export function readShallow(value) { if (value === null || typeof value !== "object") return value; if (Array.isArray(value)) return value.map(readShallow); if (value[$PROXY] !== value) return value; - const keys = sourceKeys(value); + const keys = sourceKeys(value, SOURCE_PROXY); const out = {}; for (let i = 0; i < keys.length; i++) { const k = keys[i]; @@ -880,11 +884,11 @@ export function spread(node, props, skipChildren, skip) { if (!skipChildren && !(skip !== undefined && skip("children"))) insert(node, () => { for (let i = props.length - 1; i >= 0; i--) { - const s = resolveEntry(props[i]); - if (s != null && sourceHas(s, "children")) return sourceGet(s, "children"); + const s = resolveSource(props[i]); + if (s != null && entryHas(s, "children")) return entryGet(s, "children"); } }); - effect(() => collectSources({}, props, skip), apply); + effect(() => collectSources({}, props, undefined, skip), apply); return prevProps; } if (!skipChildren && !(skip !== undefined && skip("children"))) { @@ -902,9 +906,9 @@ export function spread(node, props, skipChildren, skip) { } } else insert(node, () => { - const source = resolveEntry(props); - return source != null && sourceHas(source, "children") - ? sourceGet(source, "children") + const source = resolveSource(props); + return source != null && entryHas(source, "children") + ? entryGet(source, "children") : undefined; }); } @@ -928,11 +932,11 @@ export function spread(node, props, skipChildren, skip) { // 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); - const sources = mergeSources(source); - if (sources !== undefined) return collectSources(newProps, sources, skip); if (source != null) { - const view = omitView(source); - collectProps(newProps, view !== undefined ? view : source, skip); + 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; }, apply); @@ -955,62 +959,80 @@ function resolveSource(s) { return typeof s === "function" ? s() : s; } -// A resolved source as the ENTRY a consumer walks: an omit() proxy is -// replaced by its view record, anything else is itself. (A merge() proxy is -// flattened by the caller, since it contributes several entries.) -function entryOf(s) { - if (s == null) return s; - const view = omitView(s); - return view !== undefined ? view : s; +// `key in s` / `s[key]` for one resolved, non-null spread source: an omit() +// proxy answers from its view record (the filter, then its source), anything +// else — a plain object, a store, a merge() proxy — as itself. +function entryHas(s, key) { + const view = viewOf(s); + return view instanceof OmitView ? sourceHas(view, SOURCE_OMIT, key) : key in s; } - -function resolveEntry(s) { - return entryOf(resolveSource(s)); +function entryGet(s, key) { + const view = viewOf(s); + return view instanceof OmitView ? sourceGet(view, SOURCE_OMIT, key) : s[key]; } // Layered sources into `out`. Every function source is resolved once, up // front, and a merge() proxy among them contributes its flattened sources in -// place; keys are then collected left-to-right (Object.assign order — the -// order assign() applies them in, which `type`/`value`/`min`/`max` style -// pairs care about), and a key any LATER source has is skipped unread. -// `sourceHas` is merge()'s own resolution test, so a proxy source (store) -// answers through its `has` trap rather than a per-key descriptor trap, and -// an omit view answers from its filter. -function collectSources(out, sources, skip) { +// place — each entry with its KIND (see `SourceKind`), so the per-key walk +// below asks nothing of a proxy but the read itself; keys are then collected +// left-to-right (Object.assign order — the order assign() applies them in, +// which `type`/`value`/`min`/`max` style pairs care about), and a key any +// LATER source has is skipped unread. `sourceHas` is merge()'s own +// resolution test, so a proxy source (store) answers through its `has` trap +// rather than a per-key descriptor trap, and an omit view answers from its +// filter. +function collectSources(out, sources, kinds, skip) { const resolved = []; - for (let i = 0; i < sources.length; i++) { - const s = resolveSource(sources[i]); - const merged = mergeSources(s); - if (merged !== undefined) { - // A merge() result among the sources: its entries take its place. A - // function among THEM is merge's memo, resolved like any other. - for (let j = 0; j < merged.length; j++) resolved.push(resolveEntry(merged[j])); - } else resolved.push(entryOf(s)); - } - for (let i = 0; i < resolved.length; i++) { - const s = resolved[i]; - if (s != null) collectProps(out, s, skip, resolved, i + 1); - } + const resolvedKinds = []; + for (let i = 0; i < sources.length; i++) + pushEntry(resolved, resolvedKinds, sources[i], kinds !== undefined ? kinds[i] : SOURCE_MEMO); + for (let i = 0; i < resolved.length; i++) + collectProps(out, resolved[i], resolvedKinds[i], skip, resolved, resolvedKinds, i + 1); return out; } +// One source into the resolved entry lists. A known plain / omit-record / +// proxy entry (a merge's leaf) joins as is. Anything else — a function (the +// compiler's `() => rest`, merge's memo) resolved once — is classified: a +// merge() proxy contributes its leaves, an omit() proxy its record, a proxy +// is walked through its traps, and nothing nullish contributes at all. +function pushEntry(resolved, kinds, s, kind) { + if (kind !== SOURCE_MEMO) { + resolved.push(s); + kinds.push(kind); + return; + } + s = resolveSource(s); + if (s == null) return; + const view = viewOf(s); + if (view instanceof OmitView) { + resolved.push(view); + kinds.push(SOURCE_OMIT); + } else if (view !== undefined) { + const f = view.sources, + k = view.kinds; + for (let j = 0; j < f.length; j++) pushEntry(resolved, kinds, f[j], k[j]); + } else { + resolved.push(s); + kinds.push($PROXY in s ? SOURCE_PROXY : SOURCE_PLAIN); + } +} + // One layer of a spread source into `out`: own string keys, `children` // excluded (it has its own insert), `ref` carried through for the commit // half, object-valued style/class read HERE, tracked (see readShallow()). // With `later` (the sources after this one, from index `from`), a key one of // them defines is shadowed and never read here. -function collectProps(out, s, skip, later?, from?) { - const keys = sourceKeys(s); +function collectProps(out, s, kind, skip, later?, laterKinds?, from?) { + const keys = sourceKeys(s, kind); outer: for (let i = 0; i < keys.length; i++) { const prop = keys[i]; if (typeof prop !== "string" || prop === "children") continue; if (skip !== undefined && skip(prop)) continue; if (later !== undefined) - for (let j = from; j < later.length; j++) { - const t = later[j]; - if (t != null && sourceHas(t, prop)) continue outer; - } - const v = sourceGet(s, prop); + for (let j = from; j < later.length; j++) + if (sourceHas(later[j], laterKinds[j], prop)) continue outer; + const v = sourceGet(s, kind, prop); out[prop] = prop === "style" || prop === "class" ? readShallow(v) : v; } } /** Compiler-emitted primitive; not for hand-written code. @internal */ diff --git a/packages/web/src/server.ts b/packages/web/src/server.ts index a5dd12005..4775cb817 100644 --- a/packages/web/src/server.ts +++ b/packages/web/src/server.ts @@ -11,11 +11,16 @@ import { createComponent, untrack, merge as mergeProps, - mergeSources, - omitView, + $PROXY, + viewOf, + OmitView, sourceKeys, sourceGet, resolvedTable, + SOURCE_PLAIN, + SOURCE_OMIT, + SOURCE_PROXY, + SOURCE_MEMO, ssrScope as scope } from "solid-js"; import { effect, memo } from "./render.js"; @@ -3807,6 +3812,34 @@ export function ssrElement( ): { t: string }; // review with new ssr +// One spread source into the resolved entry lists. A known plain / +// omit-record / proxy entry (a merge's leaf) joins as is. Anything else — a +// function (the array form's thunk, merge's memo), called once — is +// classified: a merge() proxy contributes its leaves, an omit() proxy its +// record, a store is walked through its traps, and nothing nullish +// contributes at all (#3297). +function pushEntry(resolved, kinds, s, kind) { + if (kind !== SOURCE_MEMO) { + resolved.push(s); + kinds.push(kind); + return; + } + if (typeof s === "function") s = s(); + if (s == null) return; + const view = viewOf(s); + if (view instanceof OmitView) { + resolved.push(view); + kinds.push(SOURCE_OMIT); + } else if (view !== undefined) { + const f = view.sources, + k = view.kinds; + for (let j = 0; j < f.length; j++) pushEntry(resolved, kinds, f[j], k[j]); + } else { + resolved.push(s); + kinds.push($PROXY in s ? SOURCE_PROXY : SOURCE_PLAIN); + } +} + export function ssrElement(tag, props, children, needsId, skip) { // The hydration key must be allocated before the props thunk runs: dynamic // props (`mergeProps(() => ...)`) create a memo, which consumes a child id. @@ -3828,53 +3861,37 @@ export function ssrElement(tag, props, children, needsId, skip) { // hydration ids of its own — the client `spread` array form follows the // same rule. A nullish source is an empty source. // - // Sources are walked as ENTRIES (`sourceKeys`/`sourceGet`): a - // merge() proxy contributes its flattened sources and an omit() proxy its - // view record, so neither is enumerated through its traps — a descriptor - // trap per key, allocating, on every element. The common case, one plain - // object, allocates nothing here. + // Sources are walked as ENTRIES (`sourceKeys`/`sourceGet`), each with its + // KIND (see `SourceKind`): a merge() proxy contributes its flattened + // sources and an omit() proxy its view record, so neither is enumerated + // through its traps — a descriptor trap per key, allocating, on every + // element — and nothing is asked of a store proxy per key but the read. + // The common case, one plain object, allocates nothing here. let sources = null; + let kinds = null; + let kind = SOURCE_PLAIN; let table = undefined; if (Array.isArray(props)) { - sources = props; - for (let i = 0; i < sources.length; i++) { - let s = sources[i]; - if (typeof s === "function") s = s(); - // Sources first: a merge() proxy AND an omit() over a merge both answer - // with flattened entries; only an omit() of a plain object is one view. - const merged = mergeSources(s); - const view = merged === undefined ? omitView(s) : undefined; - if (s !== sources[i] || view !== undefined || merged !== undefined) { - // Resolve into a copy: the caller's array stays as passed. - if (sources === props) sources = sources.slice(); - if (merged !== undefined) { - // The flattened sources take this slot and are visited in turn: a - // function source among them is merge's memo, resolved like any - // other function entry. - sources.splice(i, 1, ...merged); - i--; - } else sources[i] = view !== undefined ? view : s; - } - } + sources = []; + kinds = []; + for (let i = 0; i < props.length; i++) pushEntry(sources, kinds, props[i], SOURCE_MEMO); } else if (props == null) { // A nullish source (static or resolved) is an empty spread (#3297). props = {}; } else if ((table = resolvedTable(props)) === undefined) { - const merged = mergeSources(props); - if (merged !== undefined) { + const view = viewOf(props); + if (view instanceof OmitView) { + props = view; + kind = SOURCE_OMIT; + } else if (view !== undefined) { // Flattened entries take the array walk; a function among them is // merge's memo, resolved here (the keys are allocated, see above). - sources = merged; - for (let i = 0; i < sources.length; i++) { - if (typeof sources[i] === "function") { - if (sources === merged) sources = sources.slice(); - sources[i] = sources[i](); - } - } - } else { - const view = omitView(props); - if (view !== undefined) props = view; - } + sources = []; + kinds = []; + const f = view.sources, + k = view.kinds; + for (let i = 0; i < f.length; i++) pushEntry(sources, kinds, f[i], k[i]); + } else if ($PROXY in props) kind = SOURCE_PROXY; } // A merge/omit view over plain objects has a RESOLVED TABLE (key → owning // leaf, shadowing applied, built once and shared with the component's own @@ -3894,7 +3911,7 @@ export function ssrElement(tag, props, children, needsId, skip) { let keysOf = null; if (sources !== null) { keysOf = new Array(last + 1); - for (let s = 0; s <= last; s++) keysOf[s] = sources[s] == null ? null : sourceKeys(sources[s]); + for (let s = 0; s <= last; s++) keysOf[s] = sourceKeys(sources[s], kinds[s]); } for (let s = 0; s <= last; s++) { const keys = @@ -3902,17 +3919,18 @@ export function ssrElement(tag, props, children, needsId, skip) { ? keysOf[s] : table !== undefined ? Array.from(table.keys()) - : sourceKeys(props); - if (keys === null) continue; - if (sources !== null) props = sources[s]; + : sourceKeys(props, kind); + if (sources !== null) { + props = sources[s]; + kind = kinds[s]; + } nextKey: for (let i = 0; i < keys.length; i++) { const prop = keys[i]; if (typeof prop !== "string" || (skip !== undefined && skip(prop))) continue; // A later source that has the key owns it; this source's getter stays // unread. for (let j = s + 1; j <= last; j++) { - const later = keysOf[j]; - if (later !== null && later.includes(prop)) continue nextKey; + if (keysOf[j].includes(prop)) continue nextKey; } // Every branch reads the prop itself, and only when it will use it. // On a spread these are compiled getters: `children` builds the child @@ -3927,7 +3945,7 @@ export function ssrElement(tag, props, children, needsId, skip) { // path equivalent: textarea value/defaultValue are its text content, // never HTML attributes (#3286). if (tag === "textarea" && (prop === "value" || prop === "defaultValue")) { - const value = table !== undefined ? table.get(prop)[prop] : sourceGet(props, prop); + const value = table !== undefined ? table.get(prop)[prop] : sourceGet(props, kind, prop); if (value !== null) children = escape(value); continue; } @@ -3937,11 +3955,11 @@ export function ssrElement(tag, props, children, needsId, skip) { tag === "script" || tag === "style" || prop === "innerHTML" ? table !== undefined ? table.get(prop)[prop] - : sourceGet(props, prop) - : escape(table !== undefined ? table.get(prop)[prop] : sourceGet(props, prop)); + : sourceGet(props, kind, prop) + : escape(table !== undefined ? table.get(prop)[prop] : sourceGet(props, kind, prop)); continue; } - const value = table !== undefined ? table.get(prop)[prop] : sourceGet(props, prop); + const value = table !== undefined ? table.get(prop)[prop] : sourceGet(props, kind, prop); // Nullish is "not set" for every attribute, `style`/`class` included — // the client removes the attribute for `undefined`, and emitting // `style=""` here made the server disagree with it (#3382).