From 461e9368792f3f4fb19c413ee6866cc29ccace17 Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Sun, 13 Sep 2026 23:58:47 -0700 Subject: [PATCH] feat(universal): spread() with fewer reactive nodes and a sources array; compilers emit it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Port of @solidjs/web's spread (#3388, #3419) to the universal renderer: - `ref` folds into the props effect and is re-applied only when its identity changes. `ref()` runs the callback untracked with no owner, so nothing a ref creates is disposed when the effect reruns. - Children keep their own owned `insert` — that effect owns the child subtree, and merging it would rebuild the children on every prop change. A plain object whose `children` is a data property inserts the value with no effect at all. Three nodes become two with children through the spread, one without. - A lone function source resolves inside each tracking scope; an ARRAY of sources is the union of their keys, later sources winning, only the winning source read, function sources called inline with no merge and no memo. Nullish sources are empty. Keys are still enumerated with for...in so a renderer's proxy props answer through their traps. Both compilers' universal generate follows the dom generate: a lone spread passes straight through (reactive included — the `mergeProps(() => expr)` wrap that existed only because the runtime could not resolve a function is gone), and several sources compile to the array instead of a mergeProps() call. Universal has no hydration ids, so the runtime and compiler halves land together. Co-authored-by: Cursor --- .changeset/universal-spread-sources.md | 7 + .../babel-plugin/src/universal/element.ts | 14 +- .../attributeExpressions/output.js | 67 +++--- .../insertChildren/output.js | 15 +- .../jsxAttributeValues/output.js | 16 +- .../attributeExpressions/output.js | 15 +- .../insertChildren/output.js | 8 +- .../jsxAttributeValues/output.js | 5 +- .../universal/attributeExpressions/output.js | 15 +- .../universal/insertChildren/output.js | 8 +- .../universal/jsxAttributeValues/output.js | 5 +- packages/compiler/src/universal/transform.rs | 21 +- packages/universal/src/universal.ts | 164 +++++++++---- .../universal/test/spread-sources.spec.js | 220 ++++++++++++++++++ 14 files changed, 448 insertions(+), 132 deletions(-) create mode 100644 .changeset/universal-spread-sources.md create mode 100644 packages/universal/test/spread-sources.spec.js diff --git a/.changeset/universal-spread-sources.md b/.changeset/universal-spread-sources.md new file mode 100644 index 000000000..be79c0d18 --- /dev/null +++ b/.changeset/universal-spread-sources.md @@ -0,0 +1,7 @@ +--- +"@solidjs/universal": patch +"@solidjs/babel-plugin": patch +"@solidjs/compiler": patch +--- + +The universal renderer's `spread()` follows the `@solidjs/web` contract (#3388): `ref` folds into the props effect and is re-applied only when its identity changes (refs run with no owner, so nothing they create is disposed by the fold); children keep their own owned `insert` — that effect owns the child subtree — but a plain object whose `children` is a data property inserts the value with no effect at all. Three reactive nodes become two when children flow through the spread, one when they don't. `spread` also resolves a lone function source inside its own tracking scopes and accepts an array of sources — `spread(node, [a, b], skipChildren)` — the union of their keys with later sources winning, only the winning source read, function sources called inline with no merge and no memo. Both compilers' universal output uses it: a lone spread passes straight through (reactive included, no more `mergeProps(() => …)`), and several sources compile to the array instead of a `mergeProps()` call. diff --git a/packages/babel-plugin/src/universal/element.ts b/packages/babel-plugin/src/universal/element.ts index a12741efd..121672587 100644 --- a/packages/babel-plugin/src/universal/element.ts +++ b/packages/babel-plugin/src/universal/element.ts @@ -399,7 +399,6 @@ function processSpreads( const filteredAttributes: JSXAttributePath[] = []; const spreadArgs: t.Expression[] = []; let runningObject: Array = []; - let dynamicSpread = false; let firstSpread = false; attributes.forEach(attribute => { const node = attribute.node; @@ -417,7 +416,7 @@ function processSpreads( spreadArgs.push( isDynamic(attribute.get("argument"), { checkMember: true - }) && (dynamicSpread = true) + }) ? t.isCallExpression(node.argument) && !node.argument.arguments.length && !t.isCallExpression(node.argument.callee) && @@ -475,10 +474,13 @@ function processSpreads( spreadArgs.push(t.objectExpression(runningObject)); } - const props = - spreadArgs.length === 1 && !dynamicSpread - ? spreadArgs[0] - : t.callExpression(registerImportMethod(path, "mergeProps"), spreadArgs); + // A lone spread — reactive included — passes straight through: the + // renderer's spread() resolves a function source inside its own tracking + // scopes. Several sources go as an ARRAY, not a mergeProps() call: spread() + // reads the sources directly (later wins per key, only the winner read) + // with no merge proxy to build and walk, and a reactive source is called + // inline with no memo. Same contract as the dom generate. + const props = spreadArgs.length === 1 ? spreadArgs[0] : t.arrayExpression(spreadArgs); return [ filteredAttributes, diff --git a/packages/babel-plugin/test/__universal_fixtures__/attributeExpressions/output.js b/packages/babel-plugin/test/__universal_fixtures__/attributeExpressions/output.js index 1c5281042..d36389a63 100644 --- a/packages/babel-plugin/test/__universal_fixtures__/attributeExpressions/output.js +++ b/packages/babel-plugin/test/__universal_fixtures__/attributeExpressions/output.js @@ -5,7 +5,6 @@ import { ref as _$ref } from "r-custom"; import { createElement as _$createElement } from "r-custom"; import { setProp as _$setProp } from "r-custom"; import { spread as _$spread } from "r-custom"; -import { mergeProps as _$mergeProps } from "r-custom"; import { binding } from "somewhere"; function refFn() {} const refConst = null; @@ -21,39 +20,45 @@ _$insertNode(_el$, _el$2); _$setProp(_el$, "id", "main"); _$spread( _el$, - _$mergeProps(results, { - style: { - color + [ + results, + { + style: { + color + } } - }), + ], true ); _$insertNode(_el$2, _el$3); _$setProp(_el$2, "class", "base"); _$spread( _el$2, - _$mergeProps(results, { - disabled: true, - readonly: "", - get title() { - return welcoming(); - }, - get style() { - return { - "background-color": color(), - "margin-right": "40px" - }; - }, - get ["class"]() { - return [ - "base", - { - dynamic: dynamic(), - selected - } - ]; + [ + results, + { + disabled: true, + readonly: "", + get title() { + return welcoming(); + }, + get style() { + return { + "background-color": color(), + "margin-right": "40px" + }; + }, + get ["class"]() { + return [ + "base", + { + dynamic: dynamic(), + selected + } + ]; + } } - }), + ], true ); _$insertNode(_el$3, _$createTextNode(`Welcome`)); @@ -71,11 +76,7 @@ var _el$5 = _$createElement("div"), _$insertNode(_el$5, _el$6); _$insertNode(_el$5, _el$7); _$insertNode(_el$5, _el$8); -_$spread( - _el$5, - _$mergeProps(() => getProps("test")), - true -); +_$spread(_el$5, () => getProps("test"), true); _$effect( () => row.label, (_v$, _$p) => { @@ -195,11 +196,11 @@ const template17 = _el$22; var _el$24 = _$createElement("div"); _$spread( _el$24, - _$mergeProps(() => ({ + () => ({ get [key()]() { return props.value; } - })), + }), false ); const template18 = _el$24; diff --git a/packages/babel-plugin/test/__universal_fixtures__/insertChildren/output.js b/packages/babel-plugin/test/__universal_fixtures__/insertChildren/output.js index fe2f6366d..fb15a65a9 100644 --- a/packages/babel-plugin/test/__universal_fixtures__/insertChildren/output.js +++ b/packages/babel-plugin/test/__universal_fixtures__/insertChildren/output.js @@ -119,25 +119,28 @@ const foldedChildren = _el$23; var _el$25 = _$createElement("module"); _$spread( _el$25, - _$mergeProps( + [ { get children() { return fallback(); } }, props - ), + ], false ); const childrenBeforeSpread = _el$25; var _el$26 = _$createElement("module"); _$spread( _el$26, - _$mergeProps(props, { - get children() { - return later(); + [ + props, + { + get children() { + return later(); + } } - }), + ], false ); const childrenAfterSpread = _el$26; diff --git a/packages/babel-plugin/test/__universal_fixtures__/jsxAttributeValues/output.js b/packages/babel-plugin/test/__universal_fixtures__/jsxAttributeValues/output.js index 78b0fffca..657cabefa 100644 --- a/packages/babel-plugin/test/__universal_fixtures__/jsxAttributeValues/output.js +++ b/packages/babel-plugin/test/__universal_fixtures__/jsxAttributeValues/output.js @@ -1,7 +1,6 @@ import { insert as _$insert } from "r-custom"; import { createComponent as _$createComponent } from "r-custom"; import { spread as _$spread } from "r-custom"; -import { mergeProps as _$mergeProps } from "r-custom"; import { ref as _$ref } from "r-custom"; import { setProp as _$setProp } from "r-custom"; import { effect as _$effect } from "r-custom"; @@ -87,13 +86,16 @@ const refValue = _el$9; var _el$0 = _$createElement("div"); _$spread( _el$0, - _$mergeProps(props, { - get data() { - var _el$19 = _$createElement("span"); - _$insert(_el$19, () => state.value); - return _el$19; + [ + props, + { + get data() { + var _el$19 = _$createElement("span"); + _$insert(_el$19, () => state.value); + return _el$19; + } } - }), + ], false ); const spreadValue = _el$0; diff --git a/packages/compiler/__tests__/fixtures/dynamic-universal/attributeExpressions/output.js b/packages/compiler/__tests__/fixtures/dynamic-universal/attributeExpressions/output.js index 6343bb63d..f9f13e1e2 100644 --- a/packages/compiler/__tests__/fixtures/dynamic-universal/attributeExpressions/output.js +++ b/packages/compiler/__tests__/fixtures/dynamic-universal/attributeExpressions/output.js @@ -1,6 +1,5 @@ import { createTextNode as _$createTextNode } from "r-custom"; import { effect as _$effect } from "r-custom"; -import { mergeProps as _$mergeProps } from "r-custom"; import { spread as _$spread } from "r-custom"; import { insertNode as _$insertNode } from "r-custom"; import { setProp as _$setProp } from "r-custom"; @@ -19,10 +18,10 @@ var _el$3 = _$createElement("a", { }); _$insertNode(_el$, _el$2); _$setProp(_el$, "id", "main"); -_$spread(_el$, _$mergeProps(results, { style: { color } }), true); +_$spread(_el$, [results, { style: { color } }], true); _$insertNode(_el$2, _el$3); _$setProp(_el$2, "class", "base"); -_$spread(_el$2, _$mergeProps(results, { +_$spread(_el$2, [results, { disabled: true, readonly: "", get title() { @@ -40,7 +39,7 @@ _$spread(_el$2, _$mergeProps(results, { selected }]; } -}), true); +}], true); _$insertNode(_el$3, _$createTextNode("Welcome")); var _ref$ = link; typeof _ref$ === "function" || Array.isArray(_ref$) ? _$ref(() => { @@ -54,9 +53,9 @@ var _el$7 = _$createElement("div", { innerHTML: "
" }); _$insertNode(_el$4, _el$5); _$insertNode(_el$4, _el$6); _$insertNode(_el$4, _el$7); -_$spread(_el$4, _$mergeProps(() => { +_$spread(_el$4, () => { return getProps("test"); -}), true); +}, true); _$effect(() => row.label, (_v$, _$p) => { _$setProp(_el$6, "textContent", _v$, _$p); }); @@ -141,11 +140,11 @@ var _el$21 = _$createElement("button", { _$insertNode(_el$21, _$createTextNode("Hi")); const template17 = _el$21; var _el$22 = _$createElement("div"); -_$spread(_el$22, _$mergeProps(() => { +_$spread(_el$22, () => { return { get [key()]() { return props.value; } }; -}), false); +}, false); const template18 = _el$22; var _el$23 = _$createElement("div"); _$effect(() => ({ diff --git a/packages/compiler/__tests__/fixtures/dynamic-universal/insertChildren/output.js b/packages/compiler/__tests__/fixtures/dynamic-universal/insertChildren/output.js index aa4fdb45b..4442bc7f1 100644 --- a/packages/compiler/__tests__/fixtures/dynamic-universal/insertChildren/output.js +++ b/packages/compiler/__tests__/fixtures/dynamic-universal/insertChildren/output.js @@ -108,12 +108,12 @@ var _el$21 = _$createElement("module"); _$insertNode(_el$21, _$createTextNode("hello")); const foldedChildren = _el$21; var _el$22 = _$createElement("module"); -_$spread(_el$22, _$mergeProps({ get children() { +_$spread(_el$22, [{ get children() { return fallback(); -} }, props), false); +} }, props], false); const childrenBeforeSpread = _el$22; var _el$23 = _$createElement("module"); -_$spread(_el$23, _$mergeProps(props, { get children() { +_$spread(_el$23, [props, { get children() { return later(); -} }), false); +} }], false); const childrenAfterSpread = _el$23; diff --git a/packages/compiler/__tests__/fixtures/dynamic-universal/jsxAttributeValues/output.js b/packages/compiler/__tests__/fixtures/dynamic-universal/jsxAttributeValues/output.js index d80daff85..72befdd7b 100644 --- a/packages/compiler/__tests__/fixtures/dynamic-universal/jsxAttributeValues/output.js +++ b/packages/compiler/__tests__/fixtures/dynamic-universal/jsxAttributeValues/output.js @@ -1,7 +1,6 @@ import { createTextNode as _$createTextNode } from "r-custom"; import { effect as _$effect } from "r-custom"; import { createComponent as _$createComponent } from "r-custom"; -import { mergeProps as _$mergeProps } from "r-custom"; import { spread as _$spread } from "r-custom"; import { insert as _$insert } from "r-custom"; import { insertNode as _$insertNode } from "r-custom"; @@ -72,13 +71,13 @@ _$ref(() => { }, _el$8); const refValue = _el$8; var _el$10 = _$createElement("div"); -_$spread(_el$10, _$mergeProps(props, { get data() { +_$spread(_el$10, [props, { get data() { var _el$15 = _$createElement("span"); _$insert(_el$15, () => { return state.value; }); return _el$15; -} }), false); +} }], false); const spreadValue = _el$10; var _el$11 = _$createElement("div"); _$insert(_el$11, _$createComponent(Comp, { get fallback() { diff --git a/packages/compiler/__tests__/fixtures/universal/attributeExpressions/output.js b/packages/compiler/__tests__/fixtures/universal/attributeExpressions/output.js index 6343bb63d..f9f13e1e2 100644 --- a/packages/compiler/__tests__/fixtures/universal/attributeExpressions/output.js +++ b/packages/compiler/__tests__/fixtures/universal/attributeExpressions/output.js @@ -1,6 +1,5 @@ import { createTextNode as _$createTextNode } from "r-custom"; import { effect as _$effect } from "r-custom"; -import { mergeProps as _$mergeProps } from "r-custom"; import { spread as _$spread } from "r-custom"; import { insertNode as _$insertNode } from "r-custom"; import { setProp as _$setProp } from "r-custom"; @@ -19,10 +18,10 @@ var _el$3 = _$createElement("a", { }); _$insertNode(_el$, _el$2); _$setProp(_el$, "id", "main"); -_$spread(_el$, _$mergeProps(results, { style: { color } }), true); +_$spread(_el$, [results, { style: { color } }], true); _$insertNode(_el$2, _el$3); _$setProp(_el$2, "class", "base"); -_$spread(_el$2, _$mergeProps(results, { +_$spread(_el$2, [results, { disabled: true, readonly: "", get title() { @@ -40,7 +39,7 @@ _$spread(_el$2, _$mergeProps(results, { selected }]; } -}), true); +}], true); _$insertNode(_el$3, _$createTextNode("Welcome")); var _ref$ = link; typeof _ref$ === "function" || Array.isArray(_ref$) ? _$ref(() => { @@ -54,9 +53,9 @@ var _el$7 = _$createElement("div", { innerHTML: "
" }); _$insertNode(_el$4, _el$5); _$insertNode(_el$4, _el$6); _$insertNode(_el$4, _el$7); -_$spread(_el$4, _$mergeProps(() => { +_$spread(_el$4, () => { return getProps("test"); -}), true); +}, true); _$effect(() => row.label, (_v$, _$p) => { _$setProp(_el$6, "textContent", _v$, _$p); }); @@ -141,11 +140,11 @@ var _el$21 = _$createElement("button", { _$insertNode(_el$21, _$createTextNode("Hi")); const template17 = _el$21; var _el$22 = _$createElement("div"); -_$spread(_el$22, _$mergeProps(() => { +_$spread(_el$22, () => { return { get [key()]() { return props.value; } }; -}), false); +}, false); const template18 = _el$22; var _el$23 = _$createElement("div"); _$effect(() => ({ diff --git a/packages/compiler/__tests__/fixtures/universal/insertChildren/output.js b/packages/compiler/__tests__/fixtures/universal/insertChildren/output.js index aa4fdb45b..4442bc7f1 100644 --- a/packages/compiler/__tests__/fixtures/universal/insertChildren/output.js +++ b/packages/compiler/__tests__/fixtures/universal/insertChildren/output.js @@ -108,12 +108,12 @@ var _el$21 = _$createElement("module"); _$insertNode(_el$21, _$createTextNode("hello")); const foldedChildren = _el$21; var _el$22 = _$createElement("module"); -_$spread(_el$22, _$mergeProps({ get children() { +_$spread(_el$22, [{ get children() { return fallback(); -} }, props), false); +} }, props], false); const childrenBeforeSpread = _el$22; var _el$23 = _$createElement("module"); -_$spread(_el$23, _$mergeProps(props, { get children() { +_$spread(_el$23, [props, { get children() { return later(); -} }), false); +} }], false); const childrenAfterSpread = _el$23; diff --git a/packages/compiler/__tests__/fixtures/universal/jsxAttributeValues/output.js b/packages/compiler/__tests__/fixtures/universal/jsxAttributeValues/output.js index d80daff85..72befdd7b 100644 --- a/packages/compiler/__tests__/fixtures/universal/jsxAttributeValues/output.js +++ b/packages/compiler/__tests__/fixtures/universal/jsxAttributeValues/output.js @@ -1,7 +1,6 @@ import { createTextNode as _$createTextNode } from "r-custom"; import { effect as _$effect } from "r-custom"; import { createComponent as _$createComponent } from "r-custom"; -import { mergeProps as _$mergeProps } from "r-custom"; import { spread as _$spread } from "r-custom"; import { insert as _$insert } from "r-custom"; import { insertNode as _$insertNode } from "r-custom"; @@ -72,13 +71,13 @@ _$ref(() => { }, _el$8); const refValue = _el$8; var _el$10 = _$createElement("div"); -_$spread(_el$10, _$mergeProps(props, { get data() { +_$spread(_el$10, [props, { get data() { var _el$15 = _$createElement("span"); _$insert(_el$15, () => { return state.value; }); return _el$15; -} }), false); +} }], false); const spreadValue = _el$10; var _el$11 = _$createElement("div"); _$insert(_el$11, _$createComponent(Comp, { get fallback() { diff --git a/packages/compiler/src/universal/transform.rs b/packages/compiler/src/universal/transform.rs index 9b4e23d50..d5225744f 100644 --- a/packages/compiler/src/universal/transform.rs +++ b/packages/compiler/src/universal/transform.rs @@ -8,6 +8,7 @@ use oxc_ast_visit::VisitMut; use oxc_span::{GetSpan, Span}; use crate::dom::element::{AstDomTransform, DomTransformConfig, jsx_expression_to_expression}; +use crate::shared::array::expression_to_array_element; use crate::shared::ast::{arrow_return_expression, expression_to_argument, object_method_property}; use crate::shared::ast_builder::AstBuilder; use crate::shared::bindings::BindingTable; @@ -687,7 +688,6 @@ impl<'a, 'source> AstUniversalTransform<'a, 'source> { let span = element.span; let mut spread_args: std::vec::Vec> = std::vec::Vec::new(); let mut running: std::vec::Vec> = std::vec::Vec::new(); - let mut dynamic_spread = false; let mut first_spread = false; let mut init_props = std::vec::Vec::new(); @@ -706,7 +706,6 @@ impl<'a, 'source> AstUniversalTransform<'a, 'source> { // raw for the deferred pass (Babel's outer traversal). let argument = spread.argument.clone_in(self.allocator); let arg = if self.classify().is_dynamic(None, &argument, false) { - dynamic_spread = true; match zero_arg_call_thunk(&argument, self.allocator) { Some(callee) => callee, None => arrow_return_expression(self.allocator, spread.span, argument), @@ -810,15 +809,19 @@ impl<'a, 'source> AstUniversalTransform<'a, 'source> { ); } - let props = if spread_args.len() == 1 && !dynamic_spread { + // A lone spread — reactive included — passes straight through: the + // renderer's spread() resolves a function source inside its own + // tracking scopes. Several sources go as an ARRAY, not a mergeProps() + // call: spread() reads the sources directly (later wins per key, + // only the winner read) with no merge proxy to build and walk, and a + // reactive source is called inline with no memo. Same contract as + // the dom generate. + let props = if spread_args.len() == 1 { spread_args.pop().expect("single spread argument exists") } else { - self.uses_merge_props = true; - let args = spread_args - .into_iter() - .map(expression_to_argument) - .collect(); - self.call_identifier(span, &self.helper_local("_$mergeProps"), args) + let elements = spread_args.into_iter().map(expression_to_array_element); + self.ast() + .expression_array(span, self.ast().vec_from_iter(elements)) }; self.uses_spread = true; diff --git a/packages/universal/src/universal.ts b/packages/universal/src/universal.ts index 2a196a4e4..38d9f068f 100644 --- a/packages/universal/src/universal.ts +++ b/packages/universal/src/universal.ts @@ -8,7 +8,8 @@ import { flatten, createMemo, createRenderEffect, - flush + flush, + $PROXY } from "solid-js"; export interface RendererOptions { @@ -60,7 +61,7 @@ export interface Renderer { ): NodeType; spread( node: any, - props: T, + props: T | (() => T) | (T | (() => T) | null | undefined)[] | null | undefined, skipChildren?: boolean, options?: RendererEffectOptions ): void; @@ -355,57 +356,138 @@ export function createRenderer({ } } - // TODO: make this better + // Same contract as @solidjs/web's spread (#3388, #3419), minus the DOM + // specifics: at most TWO reactive nodes per element, one when nothing + // flows through `children`. + // + // - The children `insert` stays separate. It OWNS the child subtree — + // components, memos and effects created while the children getter runs + // are disposed when it reruns — so folding it into the props effect + // would tear the children down and rebuild them on every prop change. + // A plain object whose `children` is a data property inserts the value + // directly, with no effect at all; a getter keeps the tracking scope. + // - `ref` FOLDS into the props effect: collected with the other props and + // applied in the commit half only when its identity changed. `ref()` + // runs the callback untracked with NO owner, so anything a ref creates + // survives the effect rerunning. + // + // Sources. A single source is an object, a mergeProps() proxy or a bare + // accessor; an accessor resolves inside each tracking scope, so a lone + // reactive spread needs no merge and no memo. An ARRAY of sources is the + // union of their keys, later sources winning per key — only the winning + // source's value is read, so a shadowed getter never runs — with function + // sources called inline, once per run. Nullish sources are empty. + // + // A caller-supplied name is shared by the child insertion and the props + // effect (#3063); without one, each gets its own stable dev fallback. function spread(node, props, skipChildren, options) { const prevProps = {}; - props || (props = {}); - // A caller-supplied name is shared by the child insertion and both - // internal effects (#3063); without one, each gets its own stable - // dev fallback. - if (!skipChildren) - insert( - node, - () => props.children, - undefined, - undefined, - named(options, "renderer spread children") - ); - effect( - () => { - const r = props.ref; - (typeof r === "function" || Array.isArray(r)) && ref(() => r, node); - }, - () => {}, - named(options, "renderer spread ref") - ); + const apply = newProps => { + for (const prop in prevProps) { + if (prop in newProps) continue; + if (prop !== "ref") setProperty(node, prop, undefined, prevProps[prop]); + delete prevProps[prop]; + } + for (const prop in newProps) { + const value = newProps[prop]; + if (value === prevProps[prop]) continue; + if (prop === "ref") { + (typeof value === "function" || Array.isArray(value)) && ref(() => value, node); + } else setProperty(node, prop, value, prevProps[prop]); + prevProps[prop] = value; + } + }; + const childrenOptions = () => named(options, "renderer spread children"); + if (Array.isArray(props)) { + if (!skipChildren) + 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; + } + }, + undefined, + undefined, + childrenOptions() + ); + effect(() => collectSources({}, props), apply, named(options, "renderer spread props")); + 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. + const desc = Object.getOwnPropertyDescriptor(props, "children"); + if (desc !== undefined) { + if (desc.get === undefined) + insert(node, desc.value, undefined, undefined, childrenOptions()); + else insert(node, () => props.children, undefined, undefined, childrenOptions()); + } + } else + insert( + node, + () => { + const s = resolveSource(props); + return s != null ? s.children : undefined; + }, + undefined, + undefined, + childrenOptions() + ); + } effect( () => { + const s = resolveSource(props); const newProps = {}; - for (const prop in props) { - if (prop === "children" || prop === "ref") continue; - newProps[prop] = props[prop]; - } + if (s != null) collectProps(newProps, s); return newProps; }, - props => { - for (const prop in prevProps) { - if (!(prop in props)) { - setProperty(node, prop, undefined, prevProps[prop]); - delete prevProps[prop]; - } - } - for (const prop in props) { - const value = props[prop]; - if (value === prevProps[prop]) continue; - setProperty(node, prop, value, prevProps[prop]); - prevProps[prop] = value; - } - }, + apply, named(options, "renderer spread props") ); return prevProps; } + function resolveSource(s) { + return typeof s === "function" ? s() : 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. + 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 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. + function collectProps(out, s, later?, from?) { + outer: for (const prop in s) { + if (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; + } + out[prop] = s[prop]; + } + return out; + } + function applyRef(r, element) { Array.isArray(r) ? r.flat(Infinity).forEach(f => f && f(element)) : r(element); } diff --git a/packages/universal/test/spread-sources.spec.js b/packages/universal/test/spread-sources.spec.js new file mode 100644 index 000000000..65f8f0a37 --- /dev/null +++ b/packages/universal/test/spread-sources.spec.js @@ -0,0 +1,220 @@ +import * as r from "./custom.js"; +import { createRoot, createSignal, flush, 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, +// children kept in their own owned insert, and an array of sources read +// directly — later sources win per key, only the winner read, function +// sources called inline with no merge and no memo. + +function mount(fn) { + let dispose; + createRoot(d => { + dispose = d; + fn(); + }); + flush(); + return dispose; +} + +describe("universal spread: ref folds into the props effect", () => { + it("applies the ref once, then only when its identity changes", () => { + const node = document.createElement("div"); + const [title, setTitle] = createSignal("a"); + const refA = vi.fn(); + const refB = vi.fn(); + // Boxed: createSignal(fn) would make a derived signal of the ref itself. + const [current, setCurrent] = createSignal({ fn: refA }); + const dispose = mount(() => + r.spread(node, { + get title() { + return title(); + }, + get ref() { + return current().fn; + } + }) + ); + expect(refA).toHaveBeenCalledTimes(1); + expect(refA.mock.calls[0][0]).toBe(node); + expect(node.getAttribute("ref")).toBeNull(); + + // An unrelated prop change reruns the effect; the same ref stays applied. + setTitle("b"); + flush(); + expect(node.getAttribute("title")).toBe("b"); + expect(refA).toHaveBeenCalledTimes(1); + + setCurrent({ fn: refB }); + flush(); + expect(refB).toHaveBeenCalledTimes(1); + expect(refA).toHaveBeenCalledTimes(1); + dispose(); + }); + + it("does not dispose what a ref created when other props change", () => { + const node = document.createElement("div"); + const [title, setTitle] = createSignal("a"); + const cleanup = vi.fn(); + const dispose = mount(() => + r.spread(node, { + get title() { + return title(); + }, + ref: () => onCleanup(cleanup) + }) + ); + setTitle("b"); + flush(); + // A ref runs with no owner: onCleanup inside it registers nowhere the + // props effect can dispose. + expect(cleanup).not.toHaveBeenCalled(); + dispose(); + }); + + it("applies an array of refs", () => { + const node = document.createElement("div"); + const seen = []; + const dispose = mount(() => + r.spread(node, { ref: [el => seen.push(["a", el]), null, [el => seen.push(["b", el])]] }) + ); + expect(seen).toEqual([ + ["a", node], + ["b", node] + ]); + dispose(); + }); +}); + +describe("universal spread: children keep their own owned insert", () => { + it("does not rebuild children when a prop changes", () => { + const node = document.createElement("div"); + const [title, setTitle] = createSignal("a"); + const [text, setText] = createSignal("hello"); + const childRuns = vi.fn(); + const dispose = mount(() => + r.spread(node, { + get title() { + return title(); + }, + get children() { + childRuns(); + return text(); + } + }) + ); + expect(childRuns).toHaveBeenCalledTimes(1); + const textNode = node.firstChild; + setTitle("b"); + flush(); + expect(childRuns).toHaveBeenCalledTimes(1); + expect(node.firstChild).toBe(textNode); + setText("bye"); + flush(); + expect(childRuns).toHaveBeenCalledTimes(2); + expect(node.textContent).toBe("bye"); + dispose(); + }); + + it("inserts a plain data-property child with no effect and skips children under skipChildren", () => { + const node = document.createElement("div"); + const dispose = mount(() => r.spread(node, { children: "static", title: "t" })); + expect(node.textContent).toBe("static"); + expect(node.getAttribute("title")).toBe("t"); + + const skipped = document.createElement("div"); + const dispose2 = mount(() => r.spread(skipped, { children: "nope" }, true)); + expect(skipped.textContent).toBe(""); + dispose(); + dispose2(); + }); +}); + +describe("universal spread: sources array", () => { + it("later sources win and shadowed getters are never read", () => { + const node = document.createElement("div"); + const shadowed = vi.fn(() => "old"); + const dispose = mount(() => + r.spread(node, [ + { + get title() { + return shadowed(); + }, + id: "a" + }, + { title: "new", class: "c" } + ]) + ); + expect(node.getAttribute("title")).toBe("new"); + expect(node.getAttribute("id")).toBe("a"); + expect(node.getAttribute("class")).toBe("c"); + expect(shadowed).not.toHaveBeenCalled(); + dispose(); + }); + + it("calls a function source inline and tracks it, with nullish sources skipped", () => { + const node = document.createElement("div"); + const [dyn, setDyn] = createSignal({ title: "one" }); + const calls = vi.fn(() => dyn()); + const dispose = mount(() => r.spread(node, [{ id: "x" }, null, calls, undefined], true)); + expect(node.getAttribute("title")).toBe("one"); + expect(node.getAttribute("id")).toBe("x"); + const before = calls.mock.calls.length; + setDyn({ title: "two", "data-extra": "y" }); + flush(); + expect(node.getAttribute("title")).toBe("two"); + expect(node.getAttribute("data-extra")).toBe("y"); + expect(calls.mock.calls.length).toBe(before + 1); + // A key that disappears from the winning source is removed. + setDyn({ title: "three" }); + flush(); + expect(node.hasAttribute("data-extra")).toBe(false); + dispose(); + }); + + it("takes children from the last source that has them", () => { + const node = document.createElement("div"); + const dispose = mount(() => r.spread(node, [{ children: "first" }, { children: "second" }])); + expect(node.textContent).toBe("second"); + dispose(); + }); + + it("carries ref through the sources", () => { + const node = document.createElement("div"); + const seen = vi.fn(); + const dispose = mount(() => r.spread(node, [{ ref: seen }, { title: "t" }])); + expect(seen.mock.calls[0][0]).toBe(node); + expect(node.getAttribute("ref")).toBeNull(); + dispose(); + }); +}); + +describe("universal spread: single reactive and nullish sources", () => { + it("resolves a lone accessor inside its own scopes", () => { + const node = document.createElement("div"); + const [props, setProps] = createSignal({ title: "a", children: "kid" }); + const dispose = mount(() => r.spread(node, props)); + expect(node.getAttribute("title")).toBe("a"); + expect(node.textContent).toBe("kid"); + setProps({ title: "b", children: "kid2" }); + flush(); + expect(node.getAttribute("title")).toBe("b"); + expect(node.textContent).toBe("kid2"); + dispose(); + }); + + it("treats a nullish source as an empty spread", () => { + const node = document.createElement("div"); + const [props, setProps] = createSignal({ title: "a" }); + const dispose = mount(() => r.spread(node, props)); + expect(node.getAttribute("title")).toBe("a"); + setProps(null); + flush(); + expect(node.hasAttribute("title")).toBe(false); + const bare = document.createElement("div"); + const dispose2 = mount(() => r.spread(bare, undefined)); + expect(bare.attributes.length).toBe(0); + dispose(); + dispose2(); + }); +});