Procházet zdrojové kódy

Fix review findings: multi-binding plumbing, headless terminal directives, doc drift

Three fixes from the adversarial review (task 275): (1) statum-binding.js keyed its -from- plumbing per ELEMENT, so a second stm-local-*-from-* attribute on the same element never updated; plumbing is now per (element, source) with one shared listener per property updating every bound variable, and the compute reads the live source directly (scenario 35 pins both the property and attribute forms). (2) core kept four DOM-touching terminal executors (form-submit/form-reset getElementById, post createElement); they move verbatim to statum-document.js and core dispatches through a setDomTerminal bridge hook whose default warns and drops the directive (correct on a headless state-layer page). (3) TEMPLATES.md intro/expansion block still showed three modules/tags, mkpstm carried a stale THREE-modules comment, the dom run.sh and scenario header were behind, and the __stmAction stash note missed clear-on-error; all corrected. Battery green: dom 35/35, split 8/8, markdown 96.4%, meson test 1/1, example + demo e2e.
saddle-ClankerSaddle před 2 týdny
rodič
revize
f59cf3f8dc

+ 5 - 2
TEMPLATES.md

@@ -4,7 +4,7 @@ This document is the reference for every tag, attribute and expression form
 that is special to Statum: the `pstm-*` preprocessor syntax handled at build
 time by `statum-mkpstm`, and the `stm-*` attributes handled at runtime by the
 browser modules (`statum-core.js`, `statum-document.js`,
-`statum-markdown.js`).
+`statum-markdown.js`, `statum-binding.js`).
 
 Audience: a developer writing Statum pages. For the wire protocol, directives
 and the Vala API see `model.md`; for a tutorial see `getting-started.md`.
@@ -108,6 +108,7 @@ expands at build time into the module tags in canonical load order:
 <script src="/_statum/resource/statum-core.js"></script>
 <script src="/_statum/resource/statum-document.js"></script>
 <script src="/_statum/resource/statum-markdown.js"></script>
+<script src="/_statum/resource/statum-binding.js"></script>
 ```
 
 One build warning per page names the page and the expansion. Pages that
@@ -213,6 +214,7 @@ ones that used to be listed under core:
   `require` evaluates false, the page redirects to `href`. Re-checked every
   render cycle; guards without `pre-entrypoint` only evaluate after the
   entrypoint has loaded. This is UX sugar, NOT security.
+
 **`statum.registerAttribute(name, fn, {after, before})`** — the extension
 seam, provided by this module. `name` is an exact attribute name or, ending
 with `.`, a prefix matcher. `fn(el, scope)` runs once per element per render
@@ -272,7 +274,8 @@ Walk bindings:
     failure / success respectively (mirrored: each clears the other; a new
     attempt clears both).
   The element's `__stmAction` promise (and `__stmActionFailed` flag) are
-  stashed per trigger for `stm-clear-on-success`.
+  stashed per trigger for `stm-clear-on-success` and `stm-clear-on-error`
+  (both await the same stash).
 
 
 ### Section: statum-markdown.js

+ 55 - 20
js/statum-binding.js

@@ -68,10 +68,16 @@
   /** Evaluation stack for cycle detection: {key, name}, innermost last. */
   var evalStack = [];
 
-  /** Elements whose property listener is attached. @type {WeakSet} */
-  var propBound = new WeakSet();
-  /** Elements with a live MutationObserver. @type {WeakSet} */
-  var observed = new WeakSet();
+  /**
+   * Per-(element, source) plumbing registries: which property listeners /
+   * attribute observers are already attached, so several bindings may share
+   * one element without one starving the other (review finding V1).
+   * @type {WeakMap} element -> { source: true }
+   */
+  var propBound = new WeakMap();
+  var observed = new WeakMap();
+  /** Variables bound to each (element, property): element -> { name: prop }. */
+  var propNames = new WeakMap();
   /** Live property values: element -> { varName: value }. @type {WeakMap} */
   var liveProps = new WeakMap();
   /** Stable per-element identity for cache keys. @type {WeakMap} */
@@ -138,33 +144,62 @@
     return tag === 'INPUT' || tag === 'SELECT' || tag === 'TEXTAREA';
   }
 
-  /** Attach the redraw plumbing for one -from- binding (idempotent). */
+  /**
+   * Attach the redraw plumbing for one -from- binding. The listener/observer
+   * is attached ONCE per (element, source); the VARIABLE registration
+   * (propNames + seed) happens for every binding — several variables may
+   * bind the same source, and each must see updates (review finding V1).
+   */
   function bindSource(el, binding) {
-    if (isPropSource(el, binding.source)) {
-      if (propBound.has(el)) return;
-      propBound.add(el);
-      var store = liveProps.get(el) || {};
-      store[binding.name] = el[binding.source];
-      liveProps.set(el, store);
+    var propKey = binding.source;
+    if (isPropSource(el, propKey)) {
+      // Register this variable against the property (always).
+      var names = propNames.get(el) || {};
+      names[binding.name] = propKey;
+      propNames.set(el, names);
+      var seed = liveProps.get(el) || {};
+      if (!Object.prototype.hasOwnProperty.call(seed, binding.name)) {
+        seed[binding.name] = el[propKey];
+        liveProps.set(el, seed);
+      }
+      // Attach the shared listener once per (element, property).
+      var bound = propBound.get(el);
+      if (bound && bound[propKey]) return;
+      if (!bound) bound = {};
+      bound[propKey] = true;
+      propBound.set(el, bound);
       var onChange = function () {
-        var s = liveProps.get(el);
-        if (!s || s[binding.name] === el[binding.source]) return; // no change
-        s[binding.name] = el[binding.source];
-        renderAll();
+        var n = propNames.get(el);
+        if (!n) return;
+        var changed = false;
+        var store = liveProps.get(el) || {};
+        for (var name in n) {
+          var key = n[name];
+          if (store[name] !== el[key]) {
+            store[name] = el[key];
+            changed = true;
+          }
+        }
+        if (changed) liveProps.set(el, store);
+        if (changed) renderAll();
       };
       el.addEventListener('input', onChange);
       el.addEventListener('change', onChange);
       return;
     }
-    if (observed.has(el) || typeof MutationObserver !== 'function') return;
-    observed.add(el);
-    var last = el.getAttribute(binding.source);
+    var seen = observed.get(el);
+    if (seen && seen[propKey]) return;
+    if (typeof MutationObserver !== 'function') return;
+    if (!seen) seen = {};
+    seen[propKey] = true;
+    observed.set(el, seen);
+    var last = el.getAttribute(propKey);
     new MutationObserver(function () {
-      var now = el.getAttribute(binding.source);
+      var now = el.getAttribute(propKey);
       if (now === last) return; // same-value write: never redraw
       last = now;
       renderAll();
-    }).observe(el, { attributes: true, attributeFilter: [binding.source] });
+    }).observe(el, { attributes: true, attributeFilter: [propKey] });
   }
 
   /**

+ 26 - 42
js/statum-core.js

@@ -850,8 +850,8 @@
    * present, running after it. Async because of reenter.
    */
   async function executeTerminal(result) {
-    result.formResets.forEach(doFormReset);
-    result.formSubmits.forEach(doFormSubmit);
+    result.formResets.forEach(function (d) { domTerminal('form-reset', d); });
+    result.formSubmits.forEach(function (d) { domTerminal('form-submit', d); });
     if (result.reenter) {
       try {
         await loadEntrypoint();
@@ -859,48 +859,22 @@
         console.error('[statum] reenter: entrypoint refresh failed', err);
       }
     }
-    if (result.navigate) doNavigate(result.navigate);
-    if (result.post) doPost(result.post);
+    if (result.navigate) domTerminal('navigate', result.navigate);
+    if (result.post) domTerminal('post', result.post);
   }
 
-  /** Execute a `form-submit` directive: optionally override action/method, then submit. */
-  function doFormSubmit(d) {
-    var form = document.getElementById(d.form);
-    if (form) {
-      if (d.action) form.setAttribute('action', d.action);
-      if (d.method) form.setAttribute('method', d.method);
-      form.submit();
-    }
-  }
-
-  /** Execute a `form-reset` directive. */
-  function doFormReset(d) {
-    var form = document.getElementById(d.form);
-    if (form) form.reset();
-  }
-
-  /** Execute a `navigate` directive as a full page load. */
-  function doNavigate(d) {
-    if (d.uri) window.location.href = d.uri;
-  }
-
-  /** Execute a `post` directive as a real (navigating) form POST. */
-  function doPost(d) {
-    if (!d.uri) return;
-    var form = document.createElement('form');
-    form.method = 'POST';
-    form.action = d.uri;
-    form.style.display = 'none';
-    var data = d.data || {};
-    Object.keys(data).forEach(function (name) {
-      var input = document.createElement('input');
-      input.type = 'hidden';
-      input.name = name;
-      input.value = data[name] == null ? '' : String(data[name]);
-      form.appendChild(input);
-    });
-    document.body.appendChild(form);
-    form.submit();
+  /**
+   * Execute one DOM-shaped terminal directive. The executors themselves
+   * (element lookup, form construction, navigation) live in
+   * statum-document.js — core is headless and owns no DOM machinery — and
+   * install themselves through the bridge. A headless page (document not
+   * loaded) drops the directive with a warning: these directives only make
+   * sense with a document.
+   * @param {string} kind 'form-submit' | 'form-reset' | 'navigate' | 'post'
+   * @param {object} d the directive
+   */
+  function domTerminal(kind, d) {
+    console.warn('[statum] ' + kind + ' directive ignored: statum-document.js is not loaded');
   }
 
   /**
@@ -1463,6 +1437,12 @@
      */
     buildScope: function (fn) { if (typeof fn === 'function') scopeBuilder = fn; },
     onPassStart: function (fn) { if (typeof fn === 'function') passStartHook = fn; },
+    /**
+     * DOM-shaped terminal-directive executors (form-submit, form-reset,
+     * navigate, post). Core dispatches and orders them but is headless —
+     * document installs the implementations; the default warns and drops.
+     */
+    setDomTerminal: function (fn) { if (typeof fn === 'function') domTerminal = fn; },
     /** Read back the installed seams (document's walk consumes these). */
     getScopeBuilder: function () { return scopeBuilder; },
     getPassStartHook: function () { return passStartHook; }
@@ -1472,6 +1452,10 @@
   var scopeBuilder = function (el, scope) { return scope; };
   /** @type {Function|null} no-op default; statum-binding.js replaces it. */
   var passStartHook = null;
+  /** @type {Function} warn-and-drop default; statum-document.js replaces it. */
+  var domTerminal = function (kind, d) {
+    console.warn('[statum] ' + kind + ' directive ignored: statum-document.js is not loaded');
+  };
 
   /** @type {Array<[string, Function, object|undefined]>} */
   var pendingRegistrations = [];

+ 50 - 0
js/statum-document.js

@@ -1038,6 +1038,56 @@
   bridge.afterRenderCycle(evaluateGuards);
   bridge.applyErrorClasses(applyErrorClasses);
 
+  // The DOM-shaped terminal directives (moved verbatim from core when it
+  // went headless): core still dispatches and orders them, document owns
+  // the element lookup / form construction / navigation.
+  bridge.setDomTerminal(function (kind, d) {
+    if (kind === 'form-submit') doFormSubmit(d);
+    else if (kind === 'form-reset') doFormReset(d);
+    else if (kind === 'navigate') doNavigate(d);
+    else if (kind === 'post') doPost(d);
+  });
+
+  /** Execute a `form-submit` directive: optionally override action/method, then submit. */
+  function doFormSubmit(d) {
+    var form = document.getElementById(d.form);
+    if (form) {
+      if (d.action) form.setAttribute('action', d.action);
+      if (d.method) form.setAttribute('method', d.method);
+      form.submit();
+    }
+  }
+
+  /** Execute a `form-reset` directive. */
+  function doFormReset(d) {
+    var form = document.getElementById(d.form);
+    if (form) form.reset();
+  }
+
+  /** Execute a `navigate` directive as a full page load. */
+  function doNavigate(d) {
+    if (d.uri) window.location.href = d.uri;
+  }
+
+  /** Execute a `post` directive as a real (navigating) form POST. */
+  function doPost(d) {
+    if (!d.uri) return;
+    var form = document.createElement('form');
+    form.method = 'POST';
+    form.action = d.uri;
+    form.style.display = 'none';
+    var data = d.data || {};
+    Object.keys(data).forEach(function (name) {
+      var input = document.createElement('input');
+      input.type = 'hidden';
+      input.name = name;
+      input.value = data[name] == null ? '' : String(data[name]);
+      form.appendChild(input);
+    });
+    document.body.appendChild(form);
+    form.submit();
+  }
+
   // Boot configuration must be in place before core's init() runs: the same
   // script tags that load this module also trigger core's autoInit on
   // DOMContentLoaded, and a synchronous page load has this module loaded by

+ 2 - 2
tests/dom/run.sh

@@ -5,7 +5,7 @@
 # bootstrap as tests/markdown/run.sh), then runs each scenario in a fresh
 # context. Exits non-zero when any scenario fails.
 #
-# Usage: tests/dom/run.sh [scenario]   (default: all twenty-four)
+# Usage: tests/dom/run.sh [scenario]   (default: all thirty-five)
 set -e
 cd "$(dirname "$0")/../.."
 
@@ -19,7 +19,7 @@ else
   QJS=$(command -v qjs)
 fi
 
-SCENARIOS="${1:-1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34}"
+SCENARIOS="${1:-1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35}"
 
 for n in $SCENARIOS; do
   echo "== dom scenario $n =="

+ 46 - 1
tests/dom/scenarios.js

@@ -35,8 +35,19 @@
  * 22  stm-clear-on-error: a failing action resets the form
  * 23  declined stm-confirm does NOT clear (clear-on-error present)
  * 24  headless core: stubbed fetch applies state, no walk, no crash
+ * 25  stm-local basics: subtree scoping, evaluation through stm-text
+ * 26  stm-local laziness: once per pass across two reads; twice across passes
+ * 27  stm-global: page-wide visibility incl. before its declaring element
+ * 28  stm-global duplicates: last declaration wins, one warn per pass
+ * 29  variable cycles: logged, read undefined, walk survives
+ * 30  from-attribute: observer redraws on real change only (same value: none)
+ * 31  from-value: the declare/username example at the 3-char boundary
+ * 32  from-checked on a checkbox
+ * 33  declare-without-binder reads undefined; slot shadowing both ways
+ * 34  stm-local replaces stm-def (demo's stamp pattern; stm-def inert)
+ * 35  several -from- bindings on ONE element all update (review V1)
  *
- * Usage: qjs tests/dom/scenarios.js <1..34>
+ * Usage: qjs tests/dom/scenarios.js <1..35>
  */
 import * as std from 'std';
 import { installShim } from './shim.js';
@@ -987,6 +998,40 @@ async function main() {
     await s._bridge.renderAll(); await spin();
     check(legacyOut.textContent === 'undefined', 'stm-def must be inert after removal');
 
+  } else if (scenario === '35') {
+    // Several -from- bindings on ONE element (review finding: the plumbing
+    // was keyed per element, so the second binding never updated).
+    var box35 = el('div'); box35.setAttribute('stm-declare-v', '');
+    box35.setAttribute('stm-declare-w', '');
+    var inp35 = input('text', 'dual', '');
+    inp35.setAttribute('stm-local-v-from-value', '');
+    inp35.setAttribute('stm-local-w-from-value', '');
+    var outV = el('span'); outV.setAttribute('stm-text', 'v');
+    var outW = el('span'); outW.setAttribute('stm-text', 'w');
+    box35.appendChild(inp35); box35.appendChild(outV); box35.appendChild(outW);
+    body.appendChild(box35);
+    await s._bridge.renderAll(); await spin();
+    check(outV.textContent === '' && outW.textContent === '',
+      'empty input must read "" for both (got ' + outV.textContent + '/' + outW.textContent + ')');
+    inp35.value = 'hello'; fire(inp35, 'input'); await spin(10);
+    check(outV.textContent === 'hello', 'first from-value binding stuck: ' + outV.textContent);
+    check(outW.textContent === 'hello', 'SECOND from-value binding on the same element stuck: ' + outW.textContent);
+    // Same for two attribute observers on one element.
+    var host35 = el('div'); host35.setAttribute('title', 'T1'); host35.setAttribute('lang', 'L1');
+    host35.setAttribute('stm-global-a', 'a'); host35.removeAttribute('stm-global-a');
+    host35.setAttribute('stm-global-t-from-title', '');
+    host35.setAttribute('stm-global-l-from-lang', '');
+    var outT35 = el('span'); outT35.setAttribute('stm-text', 't');
+    var outL35 = el('span'); outL35.setAttribute('stm-text', 'l');
+    body.appendChild(host35); body.appendChild(outT35); body.appendChild(outL35);
+    await s._bridge.renderAll(); await spin();
+    check(outT35.textContent === 'T1' && outL35.textContent === 'L1',
+      'attribute forms did not read initially: ' + outT35.textContent + '/' + outL35.textContent);
+    host35.setAttribute('lang', 'L2'); await spin(10);
+    check(outL35.textContent === 'L2', 'SECOND attribute observer on the same element stuck: ' + outL35.textContent);
+    host35.setAttribute('title', 'T2'); await spin(10);
+    check(outT35.textContent === 'T2', 'first attribute observer stuck: ' + outT35.textContent);
+
   } else {
     fail('unknown scenario: ' + scenario);
   }

+ 3 - 6
tools/statum-mkpstm/statum-mkpstm.vala

@@ -39,12 +39,9 @@ namespace Statum.Tools {
          * The Statum client ships as modules with a strict load order (core
          * first: every later module registers into the seams core installs). A
          * legacy single-file `pstm-res-src="statum.js"` reference expands at
-         * build time into exactly these tags.
-         *
-         * NOTE (rearchitecture): statum-forms.js was folded into
-         * statum-document.js (core is now headless), so the expansion set is
-         * these THREE modules. A separate upcoming module (the binding layer)
-         * will be added here when it lands — revisit this list then.
+         * build time into exactly these tags. statum-forms.js was folded
+         * into statum-document.js (core is headless); statum-binding.js
+         * adds the client-side variable layer.
          */
         private const string[] CLIENT_MODULE_ORDER = {
             "statum-core.js", "statum-document.js", "statum-markdown.js", "statum-binding.js"