diff --git a/CHANGELOG.md b/CHANGELOG.md index 7607f8c..1982575 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **Stable list reconciliation key (Epic B, B3 — #22)** — template authors + can mark reorderable list items with `data-key=""` (e.g. `
  • `). `component-client.js` bridges this onto a synthesized + `id="cf-key-"` (on both the live DOM and the incoming server render) + immediately before every `Idiomorph.morph()` call, so idiomorph's + id-based node matching reconciles reordered items by identity — preserving + focus, scroll position, in-flight input, and running CSS animations within + each item — instead of misattributing patches by position. See + `docs/LIST_RECONCILIATION_KEY.md`. + ## [0.5.1b0] - 2026-07-01 ### Added diff --git a/docs/LIST_RECONCILIATION_KEY.md b/docs/LIST_RECONCILIATION_KEY.md new file mode 100644 index 0000000..2dbf224 --- /dev/null +++ b/docs/LIST_RECONCILIATION_KEY.md @@ -0,0 +1,67 @@ +# List Reconciliation Key (`data-key`) + +When a component re-renders a list, `component-client.js` morphs the new +markup onto the live DOM in place via +[Idiomorph](https://github.com/bigskysoftware/idiomorph) instead of a full +`innerHTML` replace (see the [README](../README.md) and Epic B, #22). Morphing +preserves node identity — and with it focus, scroll position, in-flight +input, CSS transitions/animations — for nodes idiomorph can match up between +renders. + +Idiomorph matches nodes strictly by their real `id` attribute. It has no +config option for a custom reconciliation key. That's fine for content that +doesn't move around, but if a list gets **reordered** between renders (e.g. +sorting, or an item added at the front instead of the end) and the items +don't carry an `id`, idiomorph falls back to matching by tag/position — which +can misattribute a patch meant for one item onto a different item that now +happens to sit in the same slot, discarding that item's DOM state (focus, a +mid-typed value, a running CSS animation) in the process. + +## The convention + +Give each reorderable list item a stable `data-key`, matching whatever +identifies that item in your data (a primary key, slug, etc.): + +```html +
      +
    • Buy milk
      • +
    • Walk the dog
      • +
    +``` + +Before every `Idiomorph.morph()` call, `ComponentClient._bridgeListKeys()` +walks both the live (old) subtree and the incoming server-rendered HTML and +assigns `id="cf-key-"` to any `[data-key]` element that doesn't +already have a real `id`. Idiomorph's own id-based matching then reconciles +`data-key="42"` in the old DOM with `data-key="42"` in the new render, +regardless of where each ended up in the list — reordering, insertions, and +removals all resolve by identity instead of by position. + +Elements that already declare a real `id` (e.g. `
  • `) are left untouched — `data-key` only fills the gap when +there's no `id` to match on. + +## Notes and limitations + +- **The synthesized `id` is not stripped after the morph.** Idiomorph copies + matched attributes (including `id`) from the new render onto the surviving + node, so leaving `cf-key-` in place keeps both sides of the next + update carrying the same id, and idiomorph's own focus-restoration logic + depends on `id` staying present across the morph. The tradeoff: + `document.getElementById('cf-key-42')` will resolve to that list item + elsewhere in the page. Keep `data-key` values unique within the page (not + just within the list) if you rely on `getElementById` elsewhere, or give + the element its own real `id` instead of relying on the synthesized one. +- **The incoming HTML is rewritten with a small regex, not a full HTML + parser** (it hasn't been parsed into a DOM tree yet when the bridge runs — + idiomorph does that internally). Keep `data-key` values to plain + identifiers (ids, slugs) and avoid a literal `>` inside another attribute + on the same tag (e.g. an inline JSON `data-payload` containing `>`), which + can confuse the tag boundary. Attribute-name matching is anchored to + whitespace/start-of-attributes (not just a word boundary), so an unrelated + attribute like `data-id` or `aria-id` won't be mistaken for a real `id`; + and a literal `"` inside a single-quoted `data-key='...'` value is escaped + before being re-embedded in the synthesized `id`. +- This only addresses **identity for matching**, not sort order — idiomorph + still needs to move/reuse the nodes into their new positions, which it + already does once it can tell old and new nodes apart by id. diff --git a/src/component_framework/static/component_framework/js/component-client.js b/src/component_framework/static/component_framework/js/component-client.js index e369344..0efc905 100644 --- a/src/component_framework/static/component_framework/js/component-client.js +++ b/src/component_framework/static/component_framework/js/component-client.js @@ -19,6 +19,12 @@ * instantly. A `[data-loading]` attribute is toggled for the request duration. * Ship `component-framework.css` (or your own rules) to style these hooks. * + * List items that get reordered between renders should carry a stable + * `data-key` (e.g. `
  • `) so Idiomorph — which matches nodes + * by their real `id`, not a custom key — can still tell that item apart from + * its siblings and reconcile it (rather than the DOM node at its old + * position) after a reorder. See `_bridgeListKeys()` below. + * * Usage: * import { componentClient } from './component-client.js'; * @@ -76,6 +82,41 @@ function findComponentElement(target) { return target.closest('[data-component]') || null; } +/** + * Attribute template authors use to mark a stable per-item identity within a + * list that gets re-rendered (e.g. `
  • ...
    • `). See + * `ComponentClient._bridgeListKeys()`. + * + * @type {string} + */ +const LIST_KEY_ATTR = 'data-key'; + +/** + * Prefix used when synthesizing an `id` from a `data-key` value, so the + * synthesized id can't collide with an unrelated real `id` elsewhere in the + * document. Consistent with this codebase's other `cf-` prefixed hooks + * (`_cfHandler`, the `cf-optimistic-pulse` CSS animation). + * + * @type {string} + */ +const LIST_KEY_ID_PREFIX = 'cf-key'; + +// Matches one opening HTML tag, e.g. `
  • `. +const TAG_PATTERN = /<([a-zA-Z][\w:-]*)\b([^>]*)>/g; + +// Matches a `data-key="..."` / `data-key='...'` attribute *as a whole +// attribute* — anchored on whitespace or the start of the attribute list +// immediately before it, not just a word boundary, so an unrelated +// attribute that merely *ends* with "data-key" (e.g. `sub-data-key="x"`) +// can't false-positive match. +const LIST_KEY_ATTR_PATTERN = new RegExp(`(?:^|\\s)${LIST_KEY_ATTR}\\s*=\\s*(?:"([^"]*)"|'([^']*)')`); + +// Same anchoring rationale for detecting a pre-existing real `id`: a plain +// `\bid\s*=` would false-positive on `data-id="..."` or `aria-id="..."` +// (the hyphen before "id" is a non-word character, so `\b` matches there +// too) and wrongly skip synthesizing an id for that element. +const ID_ATTR_PATTERN = /(?:^|\s)id\s*=/; + // --------------------------------------------------------------------------- // ComponentClient // --------------------------------------------------------------------------- @@ -293,8 +334,9 @@ class ComponentClient { const element = document.getElementById(componentId); if (!element) return; + const html = this._bridgeListKeys(element, snapshot.html); const scrollPositions = this._captureScrollPositions(element); - Idiomorph.morph(element, snapshot.html, this._morphConfig()); + Idiomorph.morph(element, html, this._morphConfig()); this._restoreScrollPositions(element, scrollPositions); this.bind(element); @@ -329,8 +371,9 @@ class ComponentClient { return; } + const morphHtml = this._bridgeListKeys(element, html); const scrollPositions = this._captureScrollPositions(element); - Idiomorph.morph(element, html, this._morphConfig()); + Idiomorph.morph(element, morphHtml, this._morphConfig()); this._restoreScrollPositions(element, scrollPositions); // Persist state so subsequent dispatches send the correct value back. @@ -518,6 +561,72 @@ class ComponentClient { } } + /** + * Bridge `data-key`-carrying elements to a stable `id` so Idiomorph's + * node-matching engine — which matches strictly by the real `id` + * attribute (see `createIdMaps`/`populateIdMapWithTree` in + * `vendor/idiomorph.js`; there is no config option for a custom key) — + * reconciles reordered list items by identity instead of misattributing + * patches across them by position. + * + * Run immediately before every `Idiomorph.morph()` call, this: + * 1. Walks the *live* `oldElement` subtree with real DOM APIs and sets + * `id="cf-key-"` on any `[data-key]` descendant that doesn't + * already have a real `id`. + * 2. Rewrites the *incoming* HTML the same way. It can't be walked with + * DOM APIs yet — Idiomorph parses the string itself — so this uses a + * small tag-attribute regex instead. This is a best-effort text + * rewrite: a `data-key` value or a sibling attribute value containing + * a literal `>` (e.g. inside a JSON `data-payload`) can confuse the + * regex; keep `data-key` values simple (plain ids/slugs). + * + * Elements that already declare a real `id` are always left untouched — + * this only fills the gap for items that don't have one. + * + * The synthesized ids are intentionally *not* stripped after the morph. + * Idiomorph copies matched attributes (including `id`) from the incoming + * content onto the surviving node, so leaving them keeps both sides + * carrying the same id into the next update pass, and Idiomorph's own + * focus-restoration logic (`saveAndRestoreFocus`) depends on `id` + * remaining a stable, present attribute across the morph. The tradeoff: + * `document.getElementById()` elsewhere in the app can now resolve a + * `cf-key-` id. The prefix keeps that from colliding with + * unrelated real ids; pick `data-key` values that are unique to the list. + * + * @param {Element} oldElement - Live component root about to be morphed. + * @param {string} html - Incoming HTML that will be handed to Idiomorph. + * @returns {string} `html`, with reconciliation ids bridged in. + */ + _bridgeListKeys(oldElement, html) { + if (oldElement && typeof oldElement.querySelectorAll === 'function') { + for (const el of oldElement.querySelectorAll(`[${LIST_KEY_ATTR}]`)) { + if (!el.getAttribute('id')) { + const key = el.getAttribute(LIST_KEY_ATTR); + if (key) el.setAttribute('id', `${LIST_KEY_ID_PREFIX}-${key}`); + } + } + } + + if (!html) return html; + + return html.replace(TAG_PATTERN, (tag, tagName, attrs) => { + const keyMatch = attrs.match(LIST_KEY_ATTR_PATTERN); + if (!keyMatch) return tag; + if (ID_ATTR_PATTERN.test(attrs)) return tag; + + const key = keyMatch[1] ?? keyMatch[2]; + if (!key) return tag; + + // The key may have come from a single-quoted `data-key='...'` value + // that legally contains an unescaped `"`; escape it before embedding + // it in the double-quoted `id` we're injecting, so it can't prematurely + // close that attribute and corrupt the tag. + const safeKey = key.replace(/"/g, '"'); + + return `<${tagName} id="${LIST_KEY_ID_PREFIX}-${safeKey}"${attrs}>`; + }); + } + /** * Idiomorph config shared by `update()` and `rollback()` (B2, #22). * diff --git a/tests/js/list-key.test.mjs b/tests/js/list-key.test.mjs new file mode 100644 index 0000000..3af1dc8 --- /dev/null +++ b/tests/js/list-key.test.mjs @@ -0,0 +1,271 @@ +/** + * Node-native tests for the `data-key` list-reconciliation bridge in + * component-client.js (Epic B, B3 — #22). + * + * Run with: node --test "tests/js/**\/*.test.mjs" (see morph.test.mjs for + * why not `just test-js` on Windows) + * + * Idiomorph matches nodes strictly by their real `id` attribute + * (`createIdMaps`/`populateIdMapWithTree` in vendor/idiomorph.js do + * `root.querySelectorAll("[id]")` — there is no config option for a custom + * key). So a `data-key` convention can't just be a morph config flag; it + * needs a small preprocessing pass that bridges `data-key` values onto a + * real `id` (synthesized as `cf-key-`, consistent with this file's + * existing `cf-` prefix convention — see `_cfHandler` — and CSS's + * `cf-optimistic-pulse`) on *both* sides before `Idiomorph.morph()` runs: + * - the live old subtree, walked with real DOM APIs, and + * - the incoming HTML string, rewritten with a small tag-attribute regex + * (it can't be walked with DOM APIs yet — Idiomorph itself parses it). + * + * These tests only verify that integration seam (consistent with this + * repo's no-jsdom, hand-stub philosophy — see morph.test.mjs), not + * idiomorph's own matching/reordering behaviour, which has its own upstream + * test suite. + */ + +import assert from 'node:assert/strict'; +import { test } from 'node:test'; + +// --- Minimal DOM stubs (installed before importing the module) ------------ +// +// Extends the `makeEl()` pattern from morph.test.mjs/optimistic.test.mjs +// with a tiny `[attr]`-selector-only `querySelectorAll` that actually walks +// a `_children` tree, since this feature needs real subtree traversal (the +// existing stub's `querySelectorAll()` always returns `[]`). + +function matchesAttrSelector(el, selector) { + const m = /^\[([\w-]+)\]$/.exec(selector); + if (!m) return false; + return el.getAttribute(m[1]) !== null; +} + +function makeEl(dataset = {}, extra = {}) { + const el = { + dataset, + id: extra.id ?? 'c1', + tagName: extra.tagName ?? 'DIV', + outerHTML: extra.outerHTML ?? '
    ', + _attrs: { ...(extra.attrs || {}) }, + _children: extra.children || [], + _replacedWith: null, + setAttribute(k, v) { + this._attrs[k] = String(v); + if (k === 'id') this.id = String(v); + }, + getAttribute(k) { + return k in this._attrs ? this._attrs[k] : null; + }, + getAttributeNames() { + return Object.keys(this._attrs); + }, + removeAttribute(k) { + delete this._attrs[k]; + }, + replaceWith(n) { + this._replacedWith = n; + }, + closest() { + return null; + }, + matches(selector) { + return matchesAttrSelector(this, selector); + }, + querySelectorAll(selector) { + const out = []; + const walk = (node) => { + for (const child of node._children) { + if (matchesAttrSelector(child, selector)) out.push(child); + walk(child); + } + }; + walk(this); + return out; + }, + }; + if (extra.id !== undefined || el._attrs.id === undefined) { + if (el.id) el._attrs.id = el.id; + } + return el; +} + +/** + * A `[data-key]` list-item stub — no `_children` traversal needed for it. + * Unlike `makeEl()`'s component-root default, this must NOT default to a + * fake `id` of `'c1'`, since these tests are specifically about elements + * that start out *without* a real id. + */ +function makeItem(key, { id } = {}) { + const attrs = { 'data-key': key }; + if (id) attrs.id = id; + return makeEl({}, { attrs, children: [], id: id ?? '' }); +} + +const registry = {}; +globalThis.document = { + querySelector: () => null, + cookie: '', + getElementById: (id) => registry[id] ?? null, + addEventListener: () => {}, + createElement: () => makeEl({}), +}; + +const { ComponentClient } = await import( + '../../src/component_framework/static/component_framework/js/component-client.js' +); +const { Idiomorph } = await import( + '../../src/component_framework/static/component_framework/js/vendor/idiomorph.js' +); + +// --- Tests ------------------------------------------------------------------ + +test('_bridgeListKeys assigns a synthetic id to [data-key] elements in the incoming HTML that lack a real id', () => { + const client = new ComponentClient(); + const el = makeEl({}, { children: [] }); // no live data-key descendants yet + + const html = '
    • A
    • B
    '; + const out = client._bridgeListKeys(el, html); + + assert.match(out, /
  • A<\/li>/); + assert.match(out, /
  • B<\/li>/); + // The root element already had a real id and must be left untouched. + assert.match(out, /^
    /); +}); + +test('_bridgeListKeys leaves [data-key] elements alone when they already carry a real id', () => { + const client = new ComponentClient(); + const el = makeEl({}, { children: [] }); + + const html = '
  • A
    • '; + const out = client._bridgeListKeys(el, html); + + assert.equal(out, html, 'element with an existing real id must not be rewritten'); +}); + +test('_bridgeListKeys is not fooled by an unrelated attribute ending in "id=" (e.g. data-id, aria-id)', () => { + const client = new ComponentClient(); + const el = makeEl({}, { children: [] }); + + // Regression test: a naive `/\bid\s*=/` check false-positives here because + // `\b` matches the hyphen/letter boundary inside "data-id", wrongly + // treating the element as if it already had a real `id` and skipping it. + const html = '
  • A
    • '; + const out = client._bridgeListKeys(el, html); + + assert.match(out, /
  • A<\/li>/); +}); + +test('_bridgeListKeys escapes a literal `"` from a single-quoted data-key value before re-embedding it', () => { + const client = new ComponentClient(); + const el = makeEl({}, { children: [] }); + + // Regression test: a single-quoted `data-key='...'` may legally contain + // an unescaped `"`. Without escaping, re-embedding it verbatim into the + // new double-quoted `id="cf-key-"` would prematurely close that + // attribute and corrupt the tag. + const html = "
  • A
    • "; + const out = client._bridgeListKeys(el, html); + + assert.match(out, /^
  • A<\/li>$/); +}); + +test('_bridgeListKeys tags matching elements in the live old subtree in place', () => { + const client = new ComponentClient(); + const itemA = makeItem('a'); + const itemB = makeItem('b', { id: 'already-real' }); + const root = makeEl({}, { children: [itemA, itemB] }); + + client._bridgeListKeys(root, ''); + + assert.equal(itemA.getAttribute('id'), 'cf-key-a', 'missing id should be synthesized from data-key'); + assert.equal(itemB.getAttribute('id'), 'already-real', 'existing real id must not be overridden'); +}); + +test('_bridgeListKeys returns falsy/empty html unchanged', () => { + const client = new ComponentClient(); + const el = makeEl({}, { children: [] }); + assert.equal(client._bridgeListKeys(el, ''), ''); +}); + +test('update() bridges reordered [data-key] list items onto matching ids on both sides before morphing', () => { + const client = new ComponentClient(); + const itemA = makeItem('a'); + const itemB = makeItem('b'); + const el = makeEl({ state: '{}' }, { id: 'c1', children: [itemA, itemB] }); + registry.c1 = el; + + // Incoming render reorders the list (b before a) and has no ids yet. + const html = '
    • B
    • A
    '; + + let morphArgs = null; + const originalMorph = Idiomorph.morph; + Idiomorph.morph = (...args) => { + morphArgs = args; + }; + try { + client.update(el, html, '{}'); + } finally { + Idiomorph.morph = originalMorph; + } + + assert.ok(morphArgs, 'Idiomorph.morph should have been called'); + // Incoming HTML handed to morph must carry synthesized ids for both items. + assert.match(morphArgs[1], /
  • B<\/li>/); + assert.match(morphArgs[1], /
  • A<\/li>/); + // The live (old) subtree must carry the *same* synthesized ids, so + // Idiomorph's id-based matching links each old node to its new + // counterpart regardless of position. + assert.equal(itemA.getAttribute('id'), 'cf-key-a'); + assert.equal(itemB.getAttribute('id'), 'cf-key-b'); + delete registry.c1; +}); + +test('update() with no [data-key] content passes the HTML through unchanged (no regression)', () => { + const client = new ComponentClient(); + const el = makeEl({ state: '{"count":1}' }, { children: [] }); + registry.c1 = el; + + let morphArgs = null; + const originalMorph = Idiomorph.morph; + Idiomorph.morph = (...args) => { + morphArgs = args; + }; + try { + client.update(el, '
    2
    ', '{"count":2}'); + } finally { + Idiomorph.morph = originalMorph; + } + + assert.equal(morphArgs[1], '
    2
    '); + delete registry.c1; +}); + +test('rollback() bridges [data-key] items in the snapshot HTML before morphing back', () => { + const client = new ComponentClient(); + const itemA = makeItem('a'); + const el = makeEl( + { state: '{"count":2}' }, + { + id: 'c1', + children: [itemA], + outerHTML: '
  • A
    • ', + }, + ); + registry.c1 = el; + + client._snapshot(el, 'c1'); + + let morphArgs = null; + const originalMorph = Idiomorph.morph; + Idiomorph.morph = (...args) => { + morphArgs = args; + }; + try { + client.rollback('c1'); + } finally { + Idiomorph.morph = originalMorph; + } + + assert.ok(morphArgs, 'Idiomorph.morph should have been called'); + assert.match(morphArgs[1], /
  • A<\/li>/); + delete registry.c1; +});