Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-sole-child-falsy-primitive.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@solidjs/web": patch
---

Remove a sole text child of `0` or `NaN` when that hole is replaced with an element, an array, or other content. Those values stay tracked as the raw primitive, and the old truthiness checks skipped cleanup, so the text node was left in place and zeros accumulated on later toggles.
12 changes: 8 additions & 4 deletions packages/web/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2621,22 +2621,25 @@ function insertExpression(parent, value, current, marker) {
} else if (value.nodeType) {
if (Array.isArray(current)) {
cleanChildren(parent, current, multi ? marker : null, value);
} else if (current && current.nodeType) {
} else if (current != null && current.nodeType) {
// `current` is a node we previously inserted but it may have been
// moved out by user code (e.g. ref-driven migration, JSX wrapping)
// since the last render. If it's still here, replace it in place;
// otherwise append — never `replaceChild` a node that isn't ours.
current.parentNode === parent
? parent.replaceChild(value, current)
: parent.appendChild(value);
} else if (current && parent.firstChild) {
} else if (current != null && parent.firstChild) {
// A sole text child is the raw primitive, and `0` / `NaN` are falsy.
// Truthiness would skip this replace and leave that text node beside
// the new element (#3571).
parent.replaceChild(value, parent.firstChild);
} else {
parent.appendChild(value);
}
if (marker) value[$$SLOT] = marker;
} else if (Array.isArray(value)) {
const currentArray = current && Array.isArray(current);
const currentArray = Array.isArray(current);
// Commit-time text materialization (normalize left primitives raw): a
// primitive slot adopts the positional text node with a `.data` write
// when one is there, and allocates only otherwise. The adopted node's
Expand All @@ -2661,7 +2664,8 @@ function insertExpression(parent, value, current, marker) {
appendNodes(parent, value, marker);
} else reconcileArrays(parent, current, value, marker);
} else {
current && cleanChildren(parent, current);
// Same sole-primitive case: `0` / `NaN` still own a text node (#3571).
if (current != null) cleanChildren(parent, current);
appendNodes(parent, value);
}
} else if ("_SOLID_DEV_")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/**
* @jsxImportSource @solidjs/web
* @vitest-environment jsdom
*
* Hydration variant of #3571. The server renders a sole `0` child as bare
* text — `<span _hk=0>0</span>`, no `<!--$-->` markers — and the claim pass
* returns the raw primitive as `current` while the server's text node stays
* in the DOM. A post-hydration toggle to an element then hits the same
* truthiness guards in `insertExpression` that the client-only fix covers:
* the `0` text node was left beside the new element (`0<i>load</i>`).
*
* Server markup captured from renderToString of the identical component
* (ssr generate):
* <span _hk=0>0</span>
*/
import { describe, expect, test, beforeEach, afterEach } from "vitest";
import { createSignal, flush, enableHydration } from "solid-js";
import { hydrate } from "@solidjs/web";

enableHydration();

function setupHydration() {
(globalThis as any)._$HY = { events: [], completed: new WeakSet(), r: {} };
}

const SERVER_MARKUP = "<span _hk=0>0</span>";

describe("#3571: hydrated sole child of 0 toggles to an element cleanly", () => {
const container = document.createElement("div");
document.body.appendChild(container);
let dispose: (() => void) | undefined;

beforeEach(async () => {
if (dispose) dispose();
await new Promise(r => setTimeout(r, 0));
setupHydration();
container.innerHTML = "";
});

afterEach(() => {
if (dispose) {
dispose();
dispose = undefined;
}
});

test("server 0 text node is dropped when the hole becomes an element", async () => {
container.innerHTML = SERVER_MARKUP;
const span = container.querySelector("span")!;
const serverText = span.firstChild!;
expect(serverText.nodeType).toBe(3);

let setLoading!: (v: boolean) => void;
dispose = hydrate(() => {
const [loading, _setLoading] = createSignal(false);
setLoading = _setLoading;
return <span>{loading() ? <i>load</i> : 0}</span>;
}, container);

await new Promise(r => setTimeout(r, 50));
// The server element and its text node were adopted, not replaced.
expect(container.querySelector("span")).toBe(span);
expect(span.innerHTML).toBe("0");
expect(span.firstChild).toBe(serverText);

setLoading(true);
flush();
// The adopted `0` must go — not sit beside the element as `0<i>load</i>`.
expect(span.innerHTML).toBe("<i>load</i>");
expect(span.childNodes.length).toBe(1);

setLoading(false);
flush();
expect(span.innerHTML).toBe("0");
expect(span.childNodes.length).toBe(1);

setLoading(true);
flush();
expect(span.innerHTML).toBe("<i>load</i>");
expect(span.childNodes.length).toBe(1);
});
});
222 changes: 222 additions & 0 deletions packages/web/test/insert-falsy-sole-child-issue-3571.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,222 @@
/**
* @jsxImportSource @solidjs/web
* @vitest-environment jsdom
*
* A sole dynamic child whose value is `0` or `NaN` is tracked as that raw
* primitive. Replacing it must drop the text node. Truthiness checks used to
* skip that cleanup, so the new content was appended and zeros accumulated
* on later toggles (#3571).
*/
import { describe, expect, test } from "vitest";
import { createSignal, createRoot, flush, For } from "solid-js";

function directText(el: HTMLElement): string[] {
return [...el.childNodes].filter(node => node.nodeType === 3).map(node => node.nodeValue ?? "");
}

describe("sole-child falsy primitives (#3571)", () => {
test("0 swaps with an element and back without leaving text behind", () => {
const [count, setCount] = createSignal(0);
const [loading, setLoading] = createSignal(false);
let span!: HTMLSpanElement;
const dispose = createRoot(d => {
<span ref={span}>{loading() ? <i>load</i> : count()}</span>;
return d;
});
flush();

expect(span.childNodes.length).toBe(1);
expect(span.textContent).toBe("0");

setLoading(true);
flush();
expect(span.childNodes.length).toBe(1);
expect(span.firstChild!.nodeName).toBe("I");
expect(span.textContent).toBe("load");

setLoading(false);
flush();
expect(span.childNodes.length).toBe(1);
expect(span.textContent).toBe("0");

setCount(4);
flush();
expect(span.childNodes.length).toBe(1);
expect(span.textContent).toBe("4");

setCount(0);
flush();
setLoading(true);
flush();
setLoading(false);
flush();
setLoading(true);
flush();
expect(directText(span)).toEqual([]);
expect(span.childNodes.length).toBe(1);
expect(span.textContent).toBe("load");

dispose();
});

test("NaN swaps with an element and back without leaving text behind", () => {
const [value, setValue] = createSignal<number>(NaN);
const [show, setShow] = createSignal(false);
let span!: HTMLSpanElement;
const dispose = createRoot(d => {
<span ref={span}>{show() ? <b>x</b> : value()}</span>;
return d;
});
flush();

expect(span.childNodes.length).toBe(1);
expect(span.textContent).toBe("NaN");

setShow(true);
flush();
expect(span.childNodes.length).toBe(1);
expect(span.firstChild!.nodeName).toBe("B");
expect(span.textContent).toBe("x");

setShow(false);
flush();
expect(span.childNodes.length).toBe(1);
expect(span.textContent).toBe("NaN");

setValue(2);
flush();
expect(span.childNodes.length).toBe(1);
expect(span.textContent).toBe("2");

dispose();
});

test("0 swaps with an array and back without leaving text behind", () => {
const [showList, setShowList] = createSignal(false);
let div!: HTMLDivElement;
const dispose = createRoot(d => {
<div ref={div}>{showList() ? [<b>b</b>, <i>i</i>] : 0}</div>;
return d;
});
flush();

expect(div.childNodes.length).toBe(1);
expect(div.textContent).toBe("0");

setShowList(true);
flush();
expect(directText(div)).toEqual([]);
expect(div.textContent).toBe("bi");
expect(div.childNodes.length).toBe(2);

setShowList(false);
flush();
expect(div.childNodes.length).toBe(1);
expect(div.textContent).toBe("0");

setShowList(true);
flush();
expect(directText(div)).toEqual([]);
expect(div.textContent).toBe("bi");

dispose();
});

test("length && list replaces the 0 text node", () => {
const [items, setItems] = createSignal<string[]>([]);
let div!: HTMLDivElement;
const dispose = createRoot(d => {
<div ref={div}>
{items().length && <For each={items()}>{item => <span>{item}</span>}</For>}
</div>;
return d;
});
flush();

expect(div.childNodes.length).toBe(1);
expect(div.textContent).toBe("0");

setItems(["a", "b"]);
flush();
expect(directText(div)).not.toContain("0");
expect(div.textContent).toBe("ab");
expect(div.querySelectorAll("span").length).toBe(2);

setItems([]);
flush();
expect(div.childNodes.length).toBe(1);
expect(div.textContent).toBe("0");

setItems(["c"]);
flush();
expect(directText(div)).not.toContain("0");
expect(div.textContent).toBe("c");
expect(div.querySelectorAll("span").length).toBe(1);

dispose();
});

test("NaN swaps with an array and back without leaving text behind", () => {
const [showList, setShowList] = createSignal(false);
let div!: HTMLDivElement;
const dispose = createRoot(d => {
<div ref={div}>{showList() ? [<b>b</b>, <i>i</i>] : NaN}</div>;
return d;
});
flush();

expect(div.childNodes.length).toBe(1);
expect(div.textContent).toBe("NaN");

setShowList(true);
flush();
expect(directText(div)).toEqual([]);
expect(div.textContent).toBe("bi");
expect(div.childNodes.length).toBe(2);

setShowList(false);
flush();
expect(div.childNodes.length).toBe(1);
expect(div.textContent).toBe("NaN");

setShowList(true);
flush();
expect(directText(div)).toEqual([]);
expect(div.textContent).toBe("bi");

dispose();
});

test.each([
["empty string", ""],
["null", null],
["false", false]
])("%s swaps with an element and back (nothing to clean up)", (_label, empty) => {
const [show, setShow] = createSignal(false);
let span!: HTMLSpanElement;
const dispose = createRoot(d => {
<span ref={span}>{show() ? <i>x</i> : empty}</span>;
return d;
});
flush();

expect(span.childNodes.length).toBe(0);

setShow(true);
flush();
expect(span.childNodes.length).toBe(1);
expect(span.firstChild!.nodeName).toBe("I");
expect(span.textContent).toBe("x");

setShow(false);
flush();
expect(span.childNodes.length).toBe(0);

setShow(true);
flush();
expect(span.childNodes.length).toBe(1);
expect(span.textContent).toBe("x");

dispose();
});
});
Loading