From 1b05b34705916d58fd24148df16b737513a76a90 Mon Sep 17 00:00:00 2001 From: Seva Zaikov Date: Fri, 17 Jul 2026 10:00:34 -0700 Subject: [PATCH] make .renderEach() more robust --- integration-tests/context.test.ts | 14 +- .../create-state/reach-each.test.ts | 163 +++++++++++++++--- src/create-state/index.ts | 36 ++-- src/create-state/types.d.ts | 19 +- src/create-state/update-render-each-value.ts | 75 +++++--- 5 files changed, 227 insertions(+), 80 deletions(-) diff --git a/integration-tests/context.test.ts b/integration-tests/context.test.ts index 22f6a0f..08b8155 100644 --- a/integration-tests/context.test.ts +++ b/integration-tests/context.test.ts @@ -157,15 +157,15 @@ describe("Context", () => { }); const listElement = screen.getByTestId("container"); - expect(listElement.childNodes.length).toBe(2); - expect(listElement.childNodes[0].textContent).toBe("first item 3"); - expect(listElement.childNodes[1].textContent).toBe("second item 6"); + expect(listElement.children.length).toBe(2); + expect(listElement.children[0].textContent).toBe("first item 3"); + expect(listElement.children[1].textContent).toBe("second item 6"); items$.set([item1, item2, item3]); - expect(listElement.childNodes.length).toBe(3); - expect(listElement.childNodes[0].textContent).toBe("first item 3"); - expect(listElement.childNodes[1].textContent).toBe("second item 6"); - expect(listElement.childNodes[2].textContent).toBe("third item 9"); + expect(listElement.children.length).toBe(3); + expect(listElement.children[0].textContent).toBe("first item 3"); + expect(listElement.children[1].textContent).toBe("second item 6"); + expect(listElement.children[2].textContent).toBe("third item 9"); }); it("another Context overrides same value for children correctly", () => { diff --git a/integration-tests/create-state/reach-each.test.ts b/integration-tests/create-state/reach-each.test.ts index 24d090d..972e9a4 100644 --- a/integration-tests/create-state/reach-each.test.ts +++ b/integration-tests/create-state/reach-each.test.ts @@ -1,7 +1,7 @@ import { screen } from "@testing-library/dom"; import userEvent from "@testing-library/user-event"; -import { attachComponent, createElement, createState, onUnmount } from "../../src"; +import { attachComponent, createElement, createState, Fragment, onUnmount } from "../../src"; import type { State } from "../../src"; @@ -83,10 +83,10 @@ describe("state.renderEach", () => { }); const listElement = screen.getByTestId("listComponent"); - expect(listElement.childNodes.length).toBe(3); - const firstListElement = listElement.childNodes[0]; - const secondListElement = listElement.childNodes[1]; - const thirdListElement = listElement.childNodes[2]; + expect(listElement.children.length).toBe(3); + const firstListElement = listElement.children[0]; + const secondListElement = listElement.children[1]; + const thirdListElement = listElement.children[2]; expect(textSpy).toHaveBeenCalledTimes(3); expect(firstListElement.textContent).toBe(item1.text); @@ -95,11 +95,11 @@ describe("state.renderEach", () => { await user.click(screen.getByTestId("updateFirstItem")); expect(unmountSpy).not.toHaveBeenCalled(); - expect(listElement.childNodes[0].textContent).toBe("updated first value"); + expect(listElement.children[0].textContent).toBe("updated first value"); expect(textSpy).toHaveBeenCalledTimes(4); await user.click(screen.getByTestId("updateArrayButton")); - expect(listElement.childNodes.length).toBe(4); + expect(listElement.children.length).toBe(4); expect(unmountSpy).toHaveBeenCalledTimes(1); expect(textSpy).toHaveBeenCalledTimes(7); }); @@ -178,10 +178,10 @@ describe("state.renderEach", () => { }); const listElement = screen.getByTestId("listComponent"); - expect(listElement.childNodes.length).toBe(3); - const firstListElement = listElement.childNodes[0]; - const secondListElement = listElement.childNodes[1]; - const thirdListElement = listElement.childNodes[2]; + expect(listElement.children.length).toBe(3); + const firstListElement = listElement.children[0]; + const secondListElement = listElement.children[1]; + const thirdListElement = listElement.children[2]; expect(firstListElement.textContent).toBe(item1.text); expect(secondListElement.textContent).toBe(item3.text); @@ -191,11 +191,11 @@ describe("state.renderEach", () => { await user.click(screen.getByTestId("updateFirstItem")); expect(unmountSpy).not.toHaveBeenCalled(); - expect(listElement.childNodes[0].textContent).toBe("updated first value"); + expect(listElement.children[0].textContent).toBe("updated first value"); expect(textSpy).toHaveBeenCalledTimes(4); await user.click(screen.getByTestId("updateArrayButton")); - expect(listElement.childNodes.length).toBe(4); + expect(listElement.children.length).toBe(4); expect(unmountSpy).toHaveBeenCalledTimes(1); expect(textSpy).toHaveBeenCalledTimes(7); }); @@ -259,7 +259,7 @@ describe("state.renderEach", () => { expect(textSpy).toHaveBeenCalledTimes(5); expect(indexSpy).toHaveBeenCalledTimes(5); const container = screen.getByTestId("container"); - const children = container.childNodes; + const children = container.children; expect(children.length).toBe(5); expect(children[0].textContent).toBe("0.first item"); expect(children[1].textContent).toBe("1.second item"); @@ -311,12 +311,123 @@ describe("state.renderEach", () => { }); const list = screen.getByTestId("list"); - expect(list.childNodes.length).toBe(0); + expect(list.children.length).toBe(0); await user.click(screen.getByTestId("button")); - expect(list.childNodes.length).toBe(1); - expect(list.childNodes[0].textContent).toBe("first item"); + expect(list.children.length).toBe(1); + expect(list.children[0].textContent).toBe("first item"); + }); + + test("inserts the first item at the empty iterator's original position", () => { + type Item = { id: number; text: string }; + const items$ = createState([]); + + function App() { + return createElement("main", { + "data-testid": "container", + children: [ + createElement("i", { children: "before" }), + items$.renderEach({ key: "id" }, ({ elementState: element$ }) => + createElement("b", { + children: element$.renderSelected((element) => element.text), + }), + ), + createElement("i", { children: "after" }), + ], + }); + } + + cleanup = attachComponent({ + htmlElement: document.body, + component: createElement(App), + }); + + const container = screen.getByTestId("container"); + expect(container.innerHTML).toBe("beforeafter"); + + items$.set([{ id: 1, text: "item" }]); + + expect(container.innerHTML).toBe("beforeitemafter"); + + items$.set([]); + expect(container.innerHTML).toBe("beforeafter"); + + items$.set([{ id: 2, text: "second item" }]); + expect(container.innerHTML).toBe("beforesecond itemafter"); + }); + + test("renders Fragment-rooted iterator items without a phantom element", () => { + type Item = { id: number; text: string }; + const items$ = createState([{ id: 1, text: "item" }]); + + function App() { + return createElement("ul", { + "data-testid": "list", + children: items$.renderEach({ key: "id" }, ({ elementState: element$ }) => + createElement(Fragment, { + children: createElement("li", { + children: element$.renderSelected((element) => element.text), + }), + }), + ), + }); + } + + cleanup = attachComponent({ + htmlElement: document.body, + component: createElement(App), + }); + + expect(screen.getByTestId("list").innerHTML).toBe("
  • item
    • "); + }); + + test("supports numeric zero as a renderEach key", () => { + type Item = { id: number; text: string }; + const items$ = createState([{ id: 0, text: "zero" }]); + + function App() { + return createElement("ul", { + "data-testid": "list", + children: items$.renderEach({ key: "id" }, ({ elementState: element$ }) => + createElement("li", { + children: element$.renderSelected((element) => element.text), + }), + ), + }); + } + + cleanup = attachComponent({ + htmlElement: document.body, + component: createElement(App), + }); + + expect(screen.getByTestId("list").innerHTML).toBe("
  • zero
    • "); + }); + + test.each(["constructor", "__proto__", "toString"])("supports %s as a renderEach key", (key) => { + type Item = { id: string; text: string }; + const items$ = createState([]); + + function App() { + return createElement("ul", { + "data-testid": "list", + children: items$.renderEach({ key: "id" }, ({ elementState: element$ }) => + createElement("li", { + children: element$.renderSelected((element) => element.text), + }), + ), + }); + } + + cleanup = attachComponent({ + htmlElement: document.body, + component: createElement(App), + }); + + items$.set([{ id: key, text: key }]); + + expect(screen.getByTestId("list").innerHTML).toBe(`
  • ${key}
    • `); }); test("renderEach supports removing last element and adding a new one", async () => { @@ -356,17 +467,17 @@ describe("state.renderEach", () => { }); const list = screen.getByTestId("list"); - expect(list.childNodes.length).toBe(1); - expect(list.childNodes[0].textContent).toBe("first item"); + expect(list.children.length).toBe(1); + expect(list.children[0].textContent).toBe("first item"); await user.click(screen.getByTestId("removeButton")); - expect(list.childNodes.length).toBe(0); + expect(list.children.length).toBe(0); await user.click(screen.getByTestId("addButton")); - expect(list.childNodes.length).toBe(1); - expect(list.childNodes[0].textContent).toBe("second item"); + expect(list.children.length).toBe(1); + expect(list.children[0].textContent).toBe("second item"); }); test("renderEach does not update until mounted", async () => { @@ -438,12 +549,12 @@ describe("state.renderEach", () => { expect(indexSpy).toHaveBeenCalledTimes(0); const container = screen.getByTestId("container"); - const children = container.childNodes; + const children = container.children; items$.set([item4, item2, item3, item1]); // empty Text node - expect(children.length).toBe(1); + expect(container.childNodes.length).toBe(1); expect(textSpy).toHaveBeenCalledTimes(0); expect(indexSpy).toHaveBeenCalledTimes(0); @@ -473,7 +584,7 @@ describe("state.renderEach", () => { await user.click(screen.getByTestId("button")); // empty Text node - expect(children.length).toBe(1); + expect(container.childNodes.length).toBe(1); items$.set([item4, item5, item3, { ...item1, text: "1st item" }, item2, item6]); @@ -496,7 +607,7 @@ describe("state.renderEach", () => { await user.click(screen.getByTestId("button")); // empty Text node - expect(children.length).toBe(1); + expect(container.childNodes.length).toBe(1); items$.set([item3, item6]); diff --git a/src/create-state/index.ts b/src/create-state/index.ts index bd45c61..c56fb8b 100644 --- a/src/create-state/index.ts +++ b/src/create-state/index.ts @@ -10,7 +10,13 @@ import { StateCore, createCoreEquality, emptyValue } from "./state-core"; import type { VelesElement, VelesComponentObject, VelesStringElement } from "../types"; -import type { State, TrackingIterator, StateTrackers, TrackingSelectorElement } from "./types"; +import type { + IteratorKey, + State, + TrackingIterator, + StateTrackers, + TrackingSelectorElement, +} from "./types"; const STATE_CORE_PROPERTY = "__velesStateCore"; @@ -219,7 +225,7 @@ function createStateFromCore( */ renderEach( options: { - key: string | ((options: { element: any; index: number }) => string); + key: string | ((options: { element: any; index: number }) => IteratorKey); selector?: (value: T) => Element[]; }, cb: (props: { @@ -232,15 +238,19 @@ function createStateFromCore( trackingParams.savedContext = currentContext; const wrapperComponent = createElement((_props, componentAPI) => { - const children: [VelesElement | VelesComponentObject, string, State][] = []; - const elementsByKey: { - [key: string]: { + const children: [VelesElement | VelesComponentObject, IteratorKey, State][] = []; + const anchor = createTextElement(""); + anchor.needExecutedVersion = true; + trackingParams.anchor = anchor; + const elementsByKey = new Map< + IteratorKey, + { elementState: State; indexState: State; indexValue: number; node: VelesElement | VelesComponentObject; - }; - } = {}; + } + >(); const stateValue = core.get() as T; const elements = options.selector ? options.selector(stateValue) : stateValue; @@ -252,14 +262,14 @@ function createStateFromCore( (elements as Element[]).forEach((element, index) => { // we calculate a key for each element. This key determines whether we render the element from scratch, or do nothing // when the element updates - let calculatedKey: string = ""; + let calculatedKey: IteratorKey | null = null; if ( typeof options.key === "string" && typeof element === "object" && element !== null && options.key in element ) { - calculatedKey = (element as Record)[options.key]; + calculatedKey = (element as Record)[options.key]; } else if (typeof options.key === "function") { calculatedKey = options.key({ element, index }); } else { @@ -269,19 +279,19 @@ function createStateFromCore( const elementState = createState(element); const indexState = createState(index); - if (!calculatedKey) { + if (calculatedKey == null) { return; } let node = cb({ elementState, indexState }); node.needExecutedVersion = true; - elementsByKey[calculatedKey] = { + elementsByKey.set(calculatedKey, { node, indexState, indexValue: index, elementState, - }; + }); children.push([node, calculatedKey, elementState]); }); @@ -298,7 +308,7 @@ function createStateFromCore( }); return createElement("div", { phantom: true, - children: children.map((child) => child[0]), + children: [...children.map((child) => child[0]), anchor], }); }); diff --git a/src/create-state/types.d.ts b/src/create-state/types.d.ts index ea42af1..1b9691a 100644 --- a/src/create-state/types.d.ts +++ b/src/create-state/types.d.ts @@ -10,6 +10,7 @@ type StateEquality = (value1: ValueType, value2: ValueType) => boolea type StateLike = State; type ArrayElement = T extends ReadonlyArray ? Element : never; +export type IteratorKey = string | number; export type State = { track( @@ -51,7 +52,7 @@ export type State = { renderEach = ArrayElement>( options: ValueType extends ReadonlyArray ? { - key: string | ((options: { element: Element; index: number }) => string); + key: string | ((options: { element: Element; index: number }) => IteratorKey); selector?: undefined; } : never, @@ -65,7 +66,7 @@ export type State = { Element extends ArrayElement = ArrayElement, >( options: { - key: string | ((options: { element: Element; index: number }) => string); + key: string | ((options: { element: Element; index: number }) => IteratorKey); selector: (value: ValueType) => SelectorValueType; }, cb: (props: { @@ -140,21 +141,23 @@ export type TrackingAttribute = { }; export type TrackingIterator = { + anchor: VelesStringElement; cb: (props: { elementState: State; indexState: State; }) => VelesElement | VelesComponentObject; selector?: (value: unknown) => any[]; - renderedElements: [VelesElement | VelesComponentObject, string, State][]; - key: string | ((options: { element: unknown; index: number }) => string); - elementsByKey: { - [key: string]: { + renderedElements: [VelesElement | VelesComponentObject, IteratorKey, State][]; + key: string | ((options: { element: unknown; index: number }) => IteratorKey); + elementsByKey: Map< + IteratorKey, + { elementState: State; indexState: State; indexValue: number; node: VelesElement | VelesComponentObject; - }; - }; + } + >; wrapperComponent: VelesElement | VelesComponentObject; savedContext: ComponentContext; }; diff --git a/src/create-state/update-render-each-value.ts b/src/create-state/update-render-each-value.ts index 4a61e65..4d711d0 100644 --- a/src/create-state/update-render-each-value.ts +++ b/src/create-state/update-render-each-value.ts @@ -10,10 +10,12 @@ import { addPublicContext, popPublicContext } from "../context"; import type { ExecutedVelesComponent, ExecutedVelesElement, + ExecutedVelesStringElement, VelesComponentObject, VelesElement, + VelesStringElement, } from "../types"; -import type { TrackingIterator, State, createState as createStateType } from "./types"; +import type { IteratorKey, TrackingIterator, State, createState as createStateType } from "./types"; function updateUseValueIteratorValue({ value, @@ -24,8 +26,16 @@ function updateUseValueIteratorValue({ trackingIterator: TrackingIterator; createState: typeof createStateType; }) { - const { cb, key, renderedElements, elementsByKey, wrapperComponent, selector, savedContext } = - trackingIterator; + const { + anchor, + cb, + key, + renderedElements, + elementsByKey, + wrapperComponent, + selector, + savedContext, + } = trackingIterator; if (!wrapperComponent) { console.error("there is no wrapper component for the iterator"); return; @@ -47,22 +57,25 @@ function updateUseValueIteratorValue({ // but I don't know how to have correct type inferring here // so we check manually if (Array.isArray(elements)) { - const newRenderedElements: [VelesElement | VelesComponentObject, string, State][] = []; - const newElementsByKey: { - [key: string]: { + const newRenderedElements: [ + VelesElement | VelesComponentObject, + IteratorKey, + State, + ][] = []; + const newElementsByKey = new Map< + IteratorKey, + { elementState: State; indexState: State; indexValue: number; node: VelesElement | VelesComponentObject; - }; - } = {}; + } + >(); - const renderedExistingElements: { - [calculatedKey: string]: boolean; - } = {}; + const renderedExistingElements = new Set(); elements.forEach((element, index) => { - let calculatedKey: string = ""; + let calculatedKey: IteratorKey | null = null; if ( typeof key === "string" && typeof element === "object" && @@ -76,7 +89,7 @@ function updateUseValueIteratorValue({ // ignore for now } - if (!calculatedKey) { + if (calculatedKey == null) { return; } @@ -99,10 +112,10 @@ function updateUseValueIteratorValue({ // not rendered anymore, and remove them from DOM and trigger `onUnmount` // for them. - const existingElement = elementsByKey[calculatedKey]; + const existingElement = elementsByKey.get(calculatedKey); if (existingElement) { - renderedExistingElements[calculatedKey] = true; + renderedExistingElements.add(calculatedKey); const currentValue = existingElement.elementState.get(); if (currentValue !== element) { existingElement.elementState.set(element); @@ -117,12 +130,12 @@ function updateUseValueIteratorValue({ calculatedKey, existingElement.elementState, ]); - newElementsByKey[calculatedKey] = { + newElementsByKey.set(calculatedKey, { elementState: existingElement.elementState, indexState: existingElement.indexState, indexValue: index, node: existingElement.node, - }; + }); } else { const elementState = createState(element); const indexState = createState(index); @@ -135,19 +148,23 @@ function updateUseValueIteratorValue({ popPublicContext(); newRenderedElements.push([node, calculatedKey, elementState]); - newElementsByKey[calculatedKey] = { + newElementsByKey.set(calculatedKey, { elementState, indexState, indexValue: index, node, - }; + }); } }); // to replace old wrapper's children to make sure they are removed correctly // on `render` unmount - const newChildRenderedComponents: (ExecutedVelesComponent | ExecutedVelesElement)[] = []; - const newChildComponents: (VelesComponentObject | VelesElement)[] = []; + const newChildRenderedComponents: ( + | ExecutedVelesComponent + | ExecutedVelesElement + | ExecutedVelesStringElement + )[] = []; + const newChildComponents: (VelesComponentObject | VelesElement | VelesStringElement)[] = []; const positioningOffset: { [key: number]: number } = {}; @@ -172,7 +189,7 @@ function updateUseValueIteratorValue({ const [newNode, calculatedKey, _newState] = newRenderedElement; - const existingElement = elementsByKey[calculatedKey]; + const existingElement = elementsByKey.get(calculatedKey); if (existingElement) { const existingElementNode = getExecutedComponentVelesNode( getMountedNodeExecutedVersion( @@ -252,9 +269,7 @@ function updateUseValueIteratorValue({ ); firstRenderedVelesNode.html.before(newNodeVelesElement.html); } else { - // TODO: handle the case when there were 0 rendered elements - // right now this thing assumes there were no - parentVelesElement.html.prepend(newNodeVelesElement.html); + anchor.html.before(newNodeVelesElement.html); } } @@ -273,7 +288,7 @@ function updateUseValueIteratorValue({ // `childComponents` of our `wrapperComponent`, and also from the DOM renderedElements.forEach(([oldNode, calculatedKey]) => { // the element is still in DOM - if (renderedExistingElements[calculatedKey] === true) { + if (renderedExistingElements.has(calculatedKey)) { return; } else { const oldNodeExecutedVersion = getMountedNodeExecutedVersion( @@ -304,6 +319,14 @@ function updateUseValueIteratorValue({ }); } + // Keep the iterator anchor as the last child so an empty iterator retains + // its DOM position and subsequent items can be inserted before it. + if (!anchor.executedVersion) { + throw new Error("Iterator anchor is expected to be mounted"); + } + newChildRenderedComponents.push(anchor.executedVersion); + newChildComponents.push(anchor); + // We need to update `childComponents` of `wrapperVelesElementNode` to have the latest info // otherwise it will not be removed completely if it needs to be unmounted. if ("executedVelesNode" in wrapperVelesElementNode) {