diff --git a/.changeset/fix-sole-child-falsy-primitive.md b/.changeset/fix-sole-child-falsy-primitive.md new file mode 100644 index 000000000..8a3ca3c7b --- /dev/null +++ b/.changeset/fix-sole-child-falsy-primitive.md @@ -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. diff --git a/packages/web/src/client.ts b/packages/web/src/client.ts index 008374501..83b2dec65 100644 --- a/packages/web/src/client.ts +++ b/packages/web/src/client.ts @@ -2621,7 +2621,7 @@ 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; @@ -2629,14 +2629,17 @@ function insertExpression(parent, value, current, marker) { 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 @@ -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_") diff --git a/packages/web/test/hydration/hydrate-falsy-sole-child-issue-3571.spec.tsx b/packages/web/test/hydration/hydrate-falsy-sole-child-issue-3571.spec.tsx new file mode 100644 index 000000000..f5cf8e086 --- /dev/null +++ b/packages/web/test/hydration/hydrate-falsy-sole-child-issue-3571.spec.tsx @@ -0,0 +1,82 @@ +/** + * @jsxImportSource @solidjs/web + * @vitest-environment jsdom + * + * Hydration variant of #3571. The server renders a sole `0` child as bare + * text — `0`, 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 (`0load`). + * + * Server markup captured from renderToString of the identical component + * (ssr generate): + * 0 + */ +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 = "0"; + +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 {loading() ? load : 0}; + }, 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 `0load`. + expect(span.innerHTML).toBe("load"); + 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("load"); + expect(span.childNodes.length).toBe(1); + }); +}); diff --git a/packages/web/test/insert-falsy-sole-child-issue-3571.spec.tsx b/packages/web/test/insert-falsy-sole-child-issue-3571.spec.tsx new file mode 100644 index 000000000..f88a43047 --- /dev/null +++ b/packages/web/test/insert-falsy-sole-child-issue-3571.spec.tsx @@ -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 => { + {loading() ? load : count()}; + 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(NaN); + const [show, setShow] = createSignal(false); + let span!: HTMLSpanElement; + const dispose = createRoot(d => { + {show() ? x : value()}; + 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 => { +
{showList() ? [b, i] : 0}
; + 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([]); + let div!: HTMLDivElement; + const dispose = createRoot(d => { +
+ {items().length && {item => {item}}} +
; + 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 => { +
{showList() ? [b, i] : NaN}
; + 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 => { + {show() ? x : empty}; + 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(); + }); +});