Skip to content

Commit ed2fb43

Browse files
OR flattenArray's needsUnwrap with nested results instead of overwriting (#3133)
Under doNotUnwrap, an accessor child followed at the same level by a function-free fragment reset the flag, so flatten returned a plain array with the raw accessor inside instead of the resolving wrapper. Every renderer crashes on the raw function: universal hosts receive it in insertNode (as reported), and the DOM renderer throws insertBefore 'parameter 1 is not of type Node' — the protective function branch remembered from 1.x dom-expressions does not exist in 2.0, so this was not universal-specific. Fix as proposed in the report; pinned at both the signals and web layers. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent ca16891 commit ed2fb43

4 files changed

Lines changed: 78 additions & 1 deletion

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@solidjs/signals": patch
3+
---
4+
5+
Fix `flattenArray` overwriting its `needsUnwrap` flag with a nested call's result instead of OR-ing it (#3133). Under `doNotUnwrap`, an accessor child (a `<For>`/`<Repeat>`/memo) followed at the same level by a fragment containing no functions reset the flag, so `flatten` returned a plain array with the raw accessor still inside instead of the resolving wrapper. Every renderer crashed on the raw function: universal hosts received it in `insertNode` (as reported), and the DOM renderer threw `insertBefore … parameter 1 is not of type 'Node'` — the protective function branch remembered from 1.x dom-expressions does not exist in 2.0. Reported with the fix by @antoinevanwel; also submitted by @nickshiro.

packages/signals/src/boundaries.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -656,7 +656,10 @@ function flattenArray(
656656
} while (typeof child === "function" && !child.length);
657657
}
658658
if (Array.isArray(child)) {
659-
needsUnwrap = flattenArray(child, results, options);
659+
// OR, don't overwrite: an accessor already pushed under doNotUnwrap
660+
// still needs the resolving wrapper even when a later sibling
661+
// fragment contains no functions (#3133).
662+
needsUnwrap = flattenArray(child, results, options) || needsUnwrap;
660663
} else if (
661664
options?.skipNonRendered &&
662665
(child == null || child === true || child === false || child === "")
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/**
2+
* #3133: `flattenArray` must OR its `needsUnwrap` flag with a nested call's
3+
* result, not overwrite it. Under `doNotUnwrap`, a function child (a
4+
* `<For>`/`<Repeat>`/memo accessor) followed at the same level by an array
5+
* child containing no functions (a fragment) reset the flag, so `flatten`
6+
* returned the plain results array with the raw accessor still inside it
7+
* instead of the resolving wrapper. `@solidjs/web` masked this with its
8+
* insertExpression function branch; universal renderers passed the raw memo
9+
* to the host's insertNode and crashed.
10+
*/
11+
import { describe, expect, it } from "vitest";
12+
import { createMemo, createRoot, flatten } from "../src/index.js";
13+
14+
const OPTS = { skipNonRendered: true, doNotUnwrap: true };
15+
16+
describe("#3133: flatten needsUnwrap under doNotUnwrap", () => {
17+
it("keeps the wrapper when a function-free fragment follows an accessor", () => {
18+
createRoot(() => {
19+
const accessor = createMemo(() => "from memo");
20+
const out = flatten([accessor, ["a", "b"]], OPTS);
21+
expect(typeof out).toBe("function");
22+
expect(out()).toEqual(["from memo", "a", "b"]);
23+
});
24+
});
25+
26+
it("keeps the wrapper when the accessor is inside an earlier fragment", () => {
27+
createRoot(() => {
28+
const accessor = createMemo(() => "nested");
29+
const out = flatten([[accessor], ["plain"]], OPTS);
30+
expect(typeof out).toBe("function");
31+
expect(out()).toEqual(["nested", "plain"]);
32+
});
33+
});
34+
35+
it("still returns a plain array when nothing needs unwrapping", () => {
36+
const out = flatten(["a", ["b", "c"]], OPTS);
37+
expect(Array.isArray(out)).toBe(true);
38+
expect(out).toEqual(["a", "b", "c"]);
39+
});
40+
});
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* @jsxImportSource @solidjs/web
3+
* @vitest-environment jsdom
4+
*
5+
* #3133 at the DOM layer: flattenArray losing `needsUnwrap` when a
6+
* function-free fragment follows an accessor in the same children array made
7+
* `normalize` hand `insertExpression` a plain array with the raw memo still
8+
* inside — `appendNodes` then threw `Failed to execute 'insertBefore' on
9+
* 'Node': parameter 1 is not of type 'Node'`. The issue was reported against
10+
* universal renderers on the belief that web's insertExpression had a
11+
* function branch protecting it; that branch is 1.x dom-expressions — 2.0
12+
* crashes identically.
13+
*/
14+
import { expect, test } from "vitest";
15+
import { createMemo, flush } from "solid-js";
16+
import { render } from "../src/index.js";
17+
18+
test("accessor followed by a fragment inside one children array renders (#3133)", () => {
19+
const container = document.createElement("div");
20+
function App() {
21+
const label = createMemo(() => "from memo");
22+
const children = [label, ["a", "b"]];
23+
return <div>{children}</div>;
24+
}
25+
const dispose = render(() => <App />, container);
26+
flush();
27+
expect(container.textContent).toBe("from memoab");
28+
dispose();
29+
});

0 commit comments

Comments
 (0)