fix(compiler): align lone spread hydration ids

.changeset/fix-lone-spread-hydration-ids.md

@@ -0,0 +1,6 @@
+---
+"@solidjs/babel-plugin": patch
+"@solidjs/compiler": patch
+---
+
+Fix hydration IDs for reactive lone spreads. Hydratable SSR output now defers `mergeProps` until after the element key, which matches DOM output. Static and non-hydratable lone spreads keep the direct path.

packages/babel-plugin/src/ssr/element.ts

@@ -1003,7 +1003,14 @@ function createElement(
     }, []);
 
   let props: babelTypes.Expression[];
-  if (attributes.length === 1 && t.isJSXSpreadAttribute(attributes[0].node)) {
+  if (
+    attributes.length === 1 &&
+    t.isJSXSpreadAttribute(attributes[0].node) &&
+    (!hydratable ||
+      !isDynamic(attributes[0].get("argument"), {
+        checkMember: true
+      }))
+  ) {
     props = [attributes[0].node.argument];
   } else {
     props = [];

packages/babel-plugin/test/__ssr_hydratable_fixtures__/attributeExpressions/output.js

@@ -135,7 +135,7 @@ const template = _$ssrElement(
 );
 const template2 = _$ssrElement(
   "div",
-  getProps("test"),
+  () => _$mergeProps(() => getProps("test")),
   () => [
     ((_v$ = _$escape(rowId || " ")), _$ssr(_tmpl$2, _v$)),
     ((_v$2 = () => _$escape(row.label || " ")), _$ssr(_tmpl$2, _v$2)),
@@ -210,11 +210,12 @@ var _v$20 = _$ssrHydrationKey();
 const template17 = _$ssr(_tmpl$13, _v$20, "a  b  c");
 const template18 = _$ssrElement(
   "div",
-  {
-    get [key()]() {
-      return props.value;
-    }
-  },
+  () =>
+    _$mergeProps(() => ({
+      get [key()]() {
+        return props.value;
+      }
+    })),
   undefined,
   true
 );
@@ -322,16 +323,16 @@ const template27 = _$ssrElement(
 );
 const template28 = _$ssrElement(
   "label",
-  api(),
+  () => _$mergeProps(api),
   () => [
     _$ssrElement(
       "span",
-      api(),
+      () => _$mergeProps(api),
       () => ["Input is ", "<!--$-->", () => (api() ? "checked" : "unchecked"), "<!--/-->"],
       false
     ),
-    _$ssrElement("input", api(), undefined, false),
-    _$ssrElement("div", api(), undefined, false)
+    _$ssrElement("input", () => _$mergeProps(api), undefined, false),
+    _$ssrElement("div", () => _$mergeProps(api), undefined, false)
   ],
   true
 );

packages/compiler/__tests__/fixtures/ssr-hydratable/attributeExpressions/output.js

@@ -209,7 +209,11 @@ const template = _$ssrElement("div", () => {
 		return _ref$ = link, _$ssr(_tmpl$, "ccc ddd");
 	}, false);
 }, true);
-const template2 = _$ssrElement("div", getProps("test"), () => {
+const template2 = _$ssrElement("div", () => {
+	return _$mergeProps(() => {
+		return getProps("test");
+	});
+}, () => {
 	return [
 		(_v$ = _$escape(rowId || " "), _$ssr(_tmpl$2, _v$)),
 		(_v$2 = () => {
@@ -266,9 +270,13 @@ var _v$21 = _$ssrHydrationKey();
 const template16 = _$ssr(_tmpl$14, _v$21, _$ssrClassName(["static", { hi: "k" }]));
 var _v$22 = _$ssrHydrationKey();
 const template17 = _$ssr(_tmpl$15, _v$22, "a  b  c");
-const template18 = _$ssrElement("div", { get [key()]() {
-	return props.value;
-} }, undefined, true);
+const template18 = _$ssrElement("div", () => {
+	return _$mergeProps(() => {
+		return { get [key()]() {
+			return props.value;
+		} };
+	});
+}, undefined, true);
 var _v$23 = _$ssrHydrationKey();
 const template19 = _$ssr(_tmpl$16, _v$23, _$ssrClassName([{ "bg-red-500": true }, "flex flex-col"]));
 var _v$24 = _$ssrHydrationKey(), _g$3 = _$ssrGroup(() => {
@@ -318,9 +326,13 @@ const template27 = _$ssrElement("div", () => {
 }, () => {
 	return "Hi";
 }, true);
-const template28 = _$ssrElement("label", api(), () => {
+const template28 = _$ssrElement("label", () => {
+	return _$mergeProps(api);
+}, () => {
 	return [
-		_$ssrElement("span", api(), () => {
+		_$ssrElement("span", () => {
+			return _$mergeProps(api);
+		}, () => {
 			return [
 				"Input is ",
 				"<!--$-->",
@@ -330,8 +342,12 @@ const template28 = _$ssrElement("label", api(), () => {
 				"<!--/-->"
 			];
 		}, false),
-		_$ssrElement("input", api(), undefined, false),
-		_$ssrElement("div", api(), undefined, false)
+		_$ssrElement("input", () => {
+			return _$mergeProps(api);
+		}, undefined, false),
+		_$ssrElement("div", () => {
+			return _$mergeProps(api);
+		}, undefined, false)
 	];
 }, true);
 var _v$40 = _$ssrHydrationKey(), _v$41 = !!someValue;

packages/compiler/__tests__/parity/expected-cross/ssr-hydratable/dom_fixtures--attributeExpressions.diff

@@ -12,7 +12,7 @@
  _$ssrSelectValues__r_server();
  import * as styles from "./styles.module.css";
  import { binding } from "somewhere";
-@@ -122,118 +121,118 @@
+@@ -126,62 +125,62 @@
      return _$escape__r_server(row.label || " ");
    }, _$ssr__r_server(_g11$, _g3$)), _$ssr__r_server("<div><div/></div>")];
  }, true);
@@ -104,10 +104,12 @@
 -const template17 = _$ssr__r_server(_g23$, _g82$, "a  b  c");
 +var _g81$ = _$ssrHydrationKey__r_server();
 +const template17 = _$ssr__r_server(_g23$, _g81$, "a  b  c");
- const template18 = _$ssrElement__r_server("div", {
-   get [key()]() {
-     return props.value;
-   }
+ const template18 = _$ssrElement__r_server("div", () => {
+   return _$mergeProps__r_server(() => {
+     return {
+@@ -191,57 +190,57 @@
+     };
+   });
  }, undefined, true);
 -var _g83$ = _$ssrHydrationKey__r_server();
 -const template19 = _$ssr__r_server(_g24$, _g83$, _$ssrClassName__r_server([{
@@ -183,9 +185,9 @@
  const template26 = _$ssrElement__r_server("div", () => {
    return _$mergeProps__r_server({
      start: "Hi",
-@@ -258,146 +257,146 @@
-     }, "<!--/-->"];
-   }, false), _$ssrElement__r_server("input", api(), undefined, false), _$ssrElement__r_server("div", api(), undefined, false)];
+@@ -274,146 +273,146 @@
+     return _$mergeProps__r_server(api);
+   }, undefined, false)];
  }, true);
 -var _g98$ = _$ssrHydrationKey__r_server();
 -var _g99$ = !!someValue;
@@ -416,7 +418,7 @@
  const propsSpread = {
    something: color(),
    style: {
-@@ -438,39 +437,39 @@
+@@ -454,39 +453,39 @@
      height: props.height
    }
  };
@@ -472,7 +474,7 @@
  const o = {
    ref: null
  };
-@@ -479,76 +478,76 @@
+@@ -495,76 +494,76 @@
  };
  const valid = Div({
    ref(r$) {

packages/compiler/__tests__/spread-hydration.test.js

@@ -0,0 +1,147 @@
+const path = require("path");
+const { createRequire } = require("module");
+const { compileBabel, compileOxc } = require("./parity/harness");
+
+const requireBabel = createRequire(path.resolve(__dirname, "../../babel-plugin/package.json"));
+const babel = requireBabel("@babel/core");
+const t = babel.types;
+
+const reactiveSource = `
+const attrs = () => ({ class: "example" });
+const view = <div {...attrs()}><span /></div>;
+`;
+const staticSource = `
+const attrs = { class: "example" };
+const view = <div {...attrs}><span /></div>;
+`;
+const markedStaticSource = `
+const attrs = () => ({ class: "example" });
+const view = <div {.../* @static */ attrs()}><span /></div>;
+`;
+
+const compilers = {
+  Babel: (source, options) => compileBabel(source, options),
+  native: (source, options) => compileOxc(source, "spread-hydration", options)
+};
+
+function inspect(code) {
+  const ast = babel.parseSync(code, {
+    babelrc: false,
+    configFile: false,
+    sourceType: "module"
+  });
+  const helpers = new Map();
+
+  for (const statement of ast.program.body) {
+    if (!t.isImportDeclaration(statement)) continue;
+    for (const specifier of statement.specifiers) {
+      if (t.isImportSpecifier(specifier))
+        helpers.set(specifier.imported.name, specifier.local.name);
+    }
+  }
+
+  return {
+    helpers,
+    call(name) {
+      const local = helpers.get(name);
+      const matches = [];
+      babel.traverse(ast, {
+        CallExpression(callPath) {
+          if (t.isIdentifier(callPath.node.callee, { name: local })) matches.push(callPath.node);
+        }
+      });
+      expect(matches, `${name} calls`).toHaveLength(1);
+      return matches[0];
+    }
+  };
+}
+
+function returnedExpression(fn) {
+  if (!t.isBlockStatement(fn.body)) return fn.body;
+  const statement = fn.body.body.find(t.isReturnStatement);
+  expect(statement?.argument, "returned expression").toBeDefined();
+  return statement.argument;
+}
+
+function expectIdentifier(node, name) {
+  expect(t.isIdentifier(node, { name })).toBe(true);
+}
+
+describe.each(Object.entries(compilers))("%s lone spread hydration", (_name, compile) => {
+  test("defers a hydratable reactive SSR merge until after the element key", () => {
+    const output = inspect(
+      compile(reactiveSource, {
+        generate: "ssr",
+        hydratable: true,
+        moduleName: "r-server"
+      })
+    );
+    const props = output.call("ssrElement").arguments[1];
+
+    expect(t.isArrowFunctionExpression(props)).toBe(true);
+    const merged = returnedExpression(props);
+    expect(t.isCallExpression(merged)).toBe(true);
+    expectIdentifier(merged.callee, output.helpers.get("mergeProps"));
+    expect(merged.arguments).toHaveLength(1);
+    expectIdentifier(merged.arguments[0], "attrs");
+  });
+
+  test("keeps a hydratable static lone spread on the direct path", () => {
+    const output = inspect(
+      compile(staticSource, {
+        generate: "ssr",
+        hydratable: true,
+        moduleName: "r-server"
+      })
+    );
+
+    expectIdentifier(output.call("ssrElement").arguments[1], "attrs");
+    expect(output.helpers.has("mergeProps")).toBe(false);
+  });
+
+  test("keeps a marked-static hydratable lone spread on the direct path", () => {
+    const output = inspect(
+      compile(markedStaticSource, {
+        generate: "ssr",
+        hydratable: true,
+        moduleName: "r-server"
+      })
+    );
+    const props = output.call("ssrElement").arguments[1];
+
+    expect(t.isCallExpression(props)).toBe(true);
+    expectIdentifier(props.callee, "attrs");
+    expect(output.helpers.has("mergeProps")).toBe(false);
+  });
+
+  test("keeps a non-hydratable reactive lone spread on the direct path", () => {
+    const output = inspect(
+      compile(reactiveSource, {
+        generate: "ssr",
+        hydratable: false,
+        moduleName: "r-server"
+      })
+    );
+    const props = output.call("ssrElement").arguments[1];
+
+    expect(t.isCallExpression(props)).toBe(true);
+    expectIdentifier(props.callee, "attrs");
+    expect(output.helpers.has("mergeProps")).toBe(false);
+  });
+
+  test("uses the matching merge path in hydratable DOM output", () => {
+    const output = inspect(
+      compile(reactiveSource, {
+        generate: "dom",
+        hydratable: true,
+        moduleName: "r-dom"
+      })
+    );
+    const props = output.call("spread").arguments[1];
+
+    expect(t.isCallExpression(props)).toBe(true);
+    expectIdentifier(props.callee, output.helpers.get("mergeProps"));
+    expect(props.arguments).toHaveLength(1);
+    expectIdentifier(props.arguments[0], "attrs");
+  });
+});

packages/compiler/src/ssr/transform.rs

@@ -1294,9 +1294,14 @@ impl<'a, 'source> AstSsrTransform<'a, 'source> {
         attributes: &[JSXAttributeItem<'a>],
         has_children: bool,
     ) -> Result<Expression<'a>> {
-        // A lone spread attribute passes its argument straight through.
+        // A lone spread attribute passes its argument straight through unless
+        // its reactive merge must consume the same hydration id as the client.
         if attributes.len() == 1
             && let JSXAttributeItem::SpreadAttribute(spread) = &attributes[0]
+            && (!self.hydratable
+                || !self
+                    .classify()
+                    .is_dynamic(Some(spread.span.start), &spread.argument, false))
         {
             return Ok(spread.argument.clone_in(self.allocator));
         }

packages/web/test/harness/__artifacts__/reactive-lone-spread-id-parity.json

@@ -0,0 +1,5 @@
+{
+  "name": "reactive-lone-spread-id-parity",
+  "shell": "<div _hk=0 class=\"example\"><span>spread</span></div><button _hk=2>before</button>",
+  "rest": ""
+}