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(); + }); +});