Bug/Refactor: Fix Blocks In Attribute Values (#196)

Author: jlukic

docs/src/components/SpecimenExplorer/SpecimenExplorer.html

@@ -30,7 +30,7 @@
     <ui-panel size="280px" min-size="200px" max-size="400px">
       <ui-panels direction="vertical" class="control-panels">
         {#each group in getControlGroups}
-          <ui-panel size="{#if true}grow{/if}" label="{group.label}">
+          <ui-panel size="{#if group.last}grow{else}natural{/if}" label="{group.label}">
             <div class="section-controls">
               {#each control in group.controls}
                 {#if is control.type 'radio'}

packages/renderer/src/build-html-string.js

@@ -267,10 +267,19 @@ export function buildHTMLString(ast, { snippets = {}, isSVG: initialSVG = false
             rawTextNodes.push(node);
             break;
           }
-          // Block-level directives: if, each, async, rerender, template
+          // Attribute-position blocks emit the same __suiN__ shape as
+          // attribute-position expressions so they share the attribute-
+          // binding path — comment markers in an attribute value become
+          // literal text, not Comment nodes.
           const id = entries.length;
-          htmlString += `<!--${BLOCK_MARKER}${id}-->`;
-          const entry = { id, type: node.type, node };
+          const classification = analyzePosition(htmlBuffer);
+          if (classification.insideTag) {
+            htmlString += `${ATTR_MARKER_PREFIX}${id}${ATTR_MARKER_SUFFIX}`;
+          }
+          else {
+            htmlString += `<!--${BLOCK_MARKER}${id}-->`;
+          }
+          const entry = { id, type: node.type, node, classification };
           if (insideSVG) { entry.isSVG = true; }
           entries.push(entry);
           break;

packages/renderer/src/engines/lit/directives/reactive-conditional.js

@@ -1,9 +1,11 @@
 import { Reaction } from '@semantic-ui/reactivity';
-import { each, isArray, isClient, isObject } from '@semantic-ui/utils';
+import { each, isClient } from '@semantic-ui/utils';
 import { noChange, nothing } from 'lit';
 import { AsyncDirective } from 'lit/async-directive.js';
 import { directive, PartType } from 'lit/directive.js';
 
+import { serializeContent } from './serialize-content.js';
+
 export class ReactiveConditionalDirective extends AsyncDirective {
   constructor(partInfo) {
     super(partInfo);
@@ -20,12 +22,20 @@ export class ReactiveConditionalDirective extends AsyncDirective {
     }
 
     let content = nothing;
+    let initialFormatted = nothing;
     let context = {
       message: `if/else statement: {#if ${conditional.expression}}`,
       conditional: conditional,
     };
 
-    // Create a new reaction that watches for reactive changes on client
+    // Create a new reaction that watches for reactive changes on client.
+    // formatForPart is invoked INSIDE the reaction (even on firstRun) so
+    // its serialization reads — which call .value() on inner directive
+    // markers — register signal deps on this reaction. Without that,
+    // inner expressions in a branch never propagate signal changes
+    // through this directive's setValue.
+    // matchIndex gating is intentionally absent — content can change
+    // within the same branch when inner expressions update.
     if (isClient) {
       this.reaction = Reaction.create((comp) => {
         if (!this.isConnected) {
@@ -34,21 +44,25 @@ export class ReactiveConditionalDirective extends AsyncDirective {
         }
 
         const result = this.getBranch(this.conditional);
-        const matchIndex = result.matchIndex;
+        this.matchIndex = result.matchIndex;
         content = result.content;
-        if (!comp.firstRun && this.matchIndex !== matchIndex) {
-          this.matchIndex = matchIndex;
-          this.setValue(content);
+        const formatted = this.formatForPart(content);
+        if (comp.firstRun) {
+          initialFormatted = formatted;
+        }
+        else {
+          this.setValue(formatted);
         }
         return content;
       }, { context });
     }
     else {
       const result = this.getBranch(this.conditional);
       content = result.content;
+      initialFormatted = this.formatForPart(content);
     }
 
-    return this.formatForPart(content);
+    return initialFormatted;
   }
 
   getBranch(conditional) {
@@ -87,43 +101,12 @@ export class ReactiveConditionalDirective extends AsyncDirective {
     switch (this.partInfo.type) {
       case PartType.ATTRIBUTE:
       case PartType.BOOLEAN_ATTRIBUTE:
-        return this.serializeContent(content);
-
-      case PartType.CHILD:
-      case PartType.PROPERTY:
-      case PartType.EVENT:
-      case PartType.ELEMENT:
+        return serializeContent(content);
       default:
-        // For element content, return as-is (TemplateResult objects are fine here)
         return content;
     }
   }
 
-  serializeContent(content) {
-    // Handle lit's nothing value
-    if (content === nothing) {
-      return '';
-    }
-
-    if (content?.strings) {
-      // For simple conditionals in attributes, just join the static strings
-      // This works for basic cases like {#if condition}text{/if}
-      return content.strings.join('');
-    }
-
-    // Handle arrays and objects like reactive-data does
-    if (isArray(content) || isObject(content)) {
-      try {
-        return JSON.stringify(content);
-      }
-      catch (e) {
-        return String(content);
-      }
-    }
-
-    return String(content);
-  }
-
   disconnected() {
     if (this.reaction) {
       this.reaction.stop();

packages/renderer/src/engines/lit/directives/reactive-rerender.js

@@ -1,66 +1,77 @@
-import { noChange } from 'lit';
+import { noChange, nothing } from 'lit';
 import { AsyncDirective } from 'lit/async-directive.js';
-import { directive } from 'lit/directive.js';
+import { directive, PartType } from 'lit/directive.js';
 
 import { Reaction } from '@semantic-ui/reactivity';
 import { isClient, wrapFunction } from '@semantic-ui/utils';
 
+import { serializeContent } from './serialize-content.js';
+
 export class ReactiveRerenderDirective extends AsyncDirective {
   constructor(partInfo) {
     super(partInfo);
+    this.partInfo = partInfo;
     this.reaction = null;
   }
 
   render(condition) {
     this.condition = condition;
 
-    // Reuse existing reaction — signals handle updates
     if (this.reaction) {
       return noChange;
     }
 
-    // Create new reaction on client
-    if (isClient) {
-      this.watchChanges();
-    }
-
-    return this.condition.content();
-  }
-
-  watchChanges() {
+    let initialFormatted = nothing;
     const context = {
       message: `rerender block: {#${this.condition.key ? 'guard' : 'rerender'} ${
         this.condition.keyString || this.condition.expressionString
       }}`,
       rerender: this.condition,
     };
 
-    this.reaction = Reaction.create((computation) => {
-      if (!this.isConnected) {
-        computation.stop();
-        return;
-      }
+    if (isClient) {
+      this.reaction = Reaction.create((computation) => {
+        if (!this.isConnected) {
+          computation.stop();
+          return;
+        }
+
+        // Outer guard/key reactivity.
+        // {#guard expression} -> key=expression
+        // {#rerender key=expression} -> key=expression
+        if (this.condition.keyString) {
+          Reaction.guard(() => this.getValue(this.condition.key()));
+        }
+        if (this.condition.expressionString) {
+          this.getValue(this.condition.expression());
+        }
 
-      // this guards against the return value of a reactive expression the "key"
-      // {#guard expression} -> key=expression
-      // {#rerender key=expression} -> key=expressin`
-      if (this.condition.keyString) {
-        Reaction.guard(() => this.getValue(this.condition.key()));
-      }
+        const formatted = this.formatForPart(this.condition.content());
+        if (computation.firstRun) {
+          initialFormatted = formatted;
+        }
+        else {
+          this.setValue(formatted);
+        }
+      }, { context });
+    }
+    else {
+      initialFormatted = this.formatForPart(this.condition.content());
+    }
 
-      // {#rerender expression} - naively add a reactive context to this reaction
-      if (this.condition.expressionString) {
-        this.getValue(this.condition.expression());
-      }
+    return initialFormatted;
+  }
 
-      if (!computation.firstRun) {
-        this.setValue(this.condition.content());
-      }
-    }, { context });
+  formatForPart(content) {
+    switch (this.partInfo.type) {
+      case PartType.ATTRIBUTE:
+      case PartType.BOOLEAN_ATTRIBUTE:
+        return serializeContent(content);
+      default:
+        return content;
+    }
   }
 
-  // to make sure signal triggers reactivity
-  // we want to call accessor from our reaction
   getValue(expression) {
     return wrapFunction(expression)();
   }

packages/renderer/src/engines/lit/directives/serialize-content.js

@@ -0,0 +1,49 @@
+import { isArray, isFunction, isObject } from '@semantic-ui/utils';
+import { nothing } from 'lit';
+
+// Serialize lit content (TemplateResult, primitive, array, object) for
+// attribute-position placement. Inner directive markers are resolved via
+// their first arg's `.value()` callback, which LitRenderer attaches in
+// evaluateExpression.
+
+export function serializeContent(content) {
+  if (content == null || content === nothing) { return ''; }
+  if (content?.strings) {
+    const { strings, values } = content;
+    let result = '';
+    for (let i = 0; i < strings.length; i++) {
+      result += strings[i];
+      if (i < values.length) {
+        result += resolveValue(values[i]);
+      }
+    }
+    return result;
+  }
+  if (isArray(content) || isObject(content)) {
+    try {
+      return JSON.stringify(content);
+    }
+    catch (e) {
+      return String(content);
+    }
+  }
+  return String(content);
+}
+
+export function resolveValue(v) {
+  if (isFunction(v?.values?.[0]?.value)) {
+    return String(v.values[0].value() ?? '');
+  }
+  if (v?.strings) {
+    return serializeContent(v);
+  }
+  if (isArray(v) || isObject(v)) {
+    try {
+      return JSON.stringify(v);
+    }
+    catch (e) {
+      return String(v);
+    }
+  }
+  return String(v ?? '');
+}