also looking at this
fix(client): preserve client-signal {{ }} + bind SVG classes via setAttribute
#1774
4 files
+354
-293
Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.
| @@ -507,8 +507,15 @@ function expressionUsesOnlyContextVars(expr: string, context: Record<string, any | ||
| 507 | 507 | if (!inTpl && ch === '`') { inTpl = true; tplDepth = 0; continue } |
| 508 | 508 | if (inTpl) { |
| 509 | 509 | if (ch === '`' && tplDepth === 0) { inTpl = false; continue } |
| 510 | if (ch === '$' && exprWithoutStrings[ti + 1] === '{') { tplDepth++; ti++; continue } | |
| 510 | // Separator so adjacent interpolations `${a}${b}` (or ones split only by | |
| 511 | // literal text, which we drop) don't fuse into one bogus identifier `ab`. | |
| 512 | if (ch === '$' && exprWithoutStrings[ti + 1] === '{') { tplDepth++; ti++; tplResult += ' '; continue } | |
| 511 | 513 | if (ch === '}' && tplDepth > 0) { tplDepth--; continue } |
| 514 | // Inside a `${ ... }` interpolation we're looking at an EXPRESSION, not string | |
| 515 | // text — keep it so its identifiers (e.g. a client-only signal `rating` in | |
| 516 | // `` `${rating}/5` ``) are still detected. Only the literal text between | |
| 517 | // interpolations (tplDepth === 0) is dropped. | |
| 518 | if (tplDepth > 0) { tplResult += ch; continue } | |
| 512 | 519 | continue |
| 513 | 520 | } |
| 514 | 521 | tplResult += ch |
| @@ -775,11 +782,15 @@ export function processExpressions(template: string, context: Record<string, any | ||
| 775 | 782 | try { |
| 776 | 783 | const value = evaluateExpression(expr, context) |
| 777 | 784 | |
| 778 | // For signal-based templates: if evaluation returned undefined and the expression | |
| 779 | // references variables NOT in the server-side context, preserve for client-side processing. | |
| 780 | // evaluateExpression() internally catches ReferenceError and returns undefined, | |
| 781 | // so we must check here rather than relying on the catch block below. | |
| 782 | if (value === undefined && hasSignals && !expressionUsesOnlyContextVars(trimmedExpr, context)) { | |
| 785 | // For signal-based templates: if the expression references variables NOT in the | |
| 786 | // server-side context (i.e. client-only signals), preserve it for client-side | |
| 787 | // processing REGARDLESS of the server-evaluated value. Gating on `value === undefined` | |
| 788 | // only preserved BARE `` (which server-evaluates to undefined); a COMPOUND | |
| 789 | // expression like `` server-evaluates to a *defined* | |
| 790 | // string ("undefined people…") and so was wrongly baked into static HTML instead of | |
| 791 | // handed to the client — freezing it at the undefined-substituted text. Mirror the | |
| 792 | // catch-block logic below, which already preserves on the same condition. | |
| 793 | if (hasSignals && !expressionUsesOnlyContextVars(trimmedExpr, context)) { | |
| 783 | 794 | return match // Preserve for runtime evaluation |
| 784 | 795 | } |
| 785 | 796 | |
| @@ -1630,7 +1630,7 @@ catch (e) { | ||
| 1630 | 1630 | if (el.nodeType === Node.TEXT_NODE) { |
| 1631 | 1631 | const text = el.textContent; |
| 1632 | 1632 | if (text && text.includes('{{')) { |
| 1633 | const parts = text.split(/(\\{\\{[^}]+\\}\\})/g); | |
| 1633 | const parts = text.split(/(\\{\\{[\\s\\S]+?\\}\\})/g); | |
| 1634 | 1634 | if (parts.length > 1) { |
| 1635 | 1635 | const fragment = document.createDocumentFragment(); |
| 1636 | 1636 | const parentEl = el.parentNode; |
| @@ -1639,7 +1639,7 @@ catch (e) { | ||
| 1639 | 1639 | // through nested @if/@for processing where componentScope may be restored |
| 1640 | 1640 | const capturedScope = { ...scope, ...(findElementScope(parentEl) || {}), ...globalHelpers }; |
| 1641 | 1641 | parts.forEach(part => { |
| 1642 | const match = part.match(/^\\{\\{\\s*(.+?)\\s*\\}\\}$/); | |
| 1642 | const match = part.match(/^\\{\\{\\s*([\\s\\S]+?)\\s*\\}\\}$/); | |
| 1643 | 1643 | if (match) { |
| 1644 | 1644 | const expr = match[1]; |
| 1645 | 1645 | // Skip placeholder expressions like __TITLE__ (build-time placeholders) |
| @@ -0,0 +1,58 @@ | ||
| 1 | import { describe, expect, it } from 'bun:test' | |
| 2 | import { processExpressions } from '../../src/expressions' | |
| 3 | import { generateSignalsRuntimeDev } from '../../src/signals' | |
| 4 | ||
| 5 | /** | |
| 6 | * Regression tests for: a `` expression that references a client-only | |
| 7 | * signal must be PRESERVED for client hydration, not baked to its | |
| 8 | * server-evaluated value. | |
| 9 | * | |
| 10 | * The server SSG interpolator previously preserved a `` for the client | |
| 11 | * ONLY when the whole expression server-evaluated to exactly `undefined`. | |
| 12 | * A bare `` (signal absent from the server context) evaluates to | |
| 13 | * `undefined` and was preserved. But a COMPOUND expression such as | |
| 14 | * `` evaluates to a *defined* string | |
| 15 | * ("undefined people…") and was therefore baked into static HTML — freezing | |
| 16 | * it at the undefined-substituted text with no way for the client to re-bind. | |
| 17 | * Template literals were doubly affected: the `${…}` body was stripped before | |
| 18 | * the "references a client var?" check, so its signal was never seen. | |
| 19 | */ | |
| 20 | ||
| 21 | // A `:if` directive marks the template client-reactive (usesSignalsInScript), | |
| 22 | // matching a real signal component. `likes`/`liked`/`rating` are client-only | |
| 23 | // signals (absent from the server context); `serverVar` IS server context. | |
| 24 | const reactive = (expr: string): string => | |
| 25 | processExpressions(`<div :if="x">${expr}</div>`, { serverVar: 'S' }, 't.stx') | |
| 26 | ||
| 27 | describe('client-signal preservation', () => { | |
| 28 | it('preserves a COMPOUND expression referencing a client-only signal', () => { | |
| 29 | const out = reactive(`undefined people find this helpful`) | |
| 30 | expect(out).toContain(`undefined people find this helpful`) | |
| 31 | }) | |
| 32 | ||
| 33 | it('preserves a nested ternary over client-only signals', () => { | |
| 34 | const expr = `undefined people` | |
| 35 | expect(reactive(expr)).toContain(expr) | |
| 36 | }) | |
| 37 | ||
| 38 | it('preserves a TEMPLATE LITERAL whose ${} references a client-only signal', () => { | |
| 39 | expect(reactive('undefined/5')).toContain('undefined/5') | |
| 40 | expect(reactive('(undefined)')).toContain('(undefined)') | |
| 41 | }) | |
| 42 | ||
| 43 | it('still BAKES an expression that uses only server-context vars (no over-preserve)', () => { | |
| 44 | expect(reactive('3')).toContain('3') | |
| 45 | expect(reactive('')).toContain('S') | |
| 46 | // template literal over ONLY context vars must bake, not preserve | |
| 47 | const tpl = reactive('Hello undefined') | |
| 48 | expect(tpl).toContain('Hello S') | |
| 49 | expect(tpl).not.toContain('{{') | |
| 50 | }) | |
| 51 | ||
| 52 | it('runtime binds classes on SVG via setAttribute (SVGElement.className is read-only)', () => { | |
| 53 | // Guards the companion client fix: `bindClass` must not do `el.className = …` | |
| 54 | // on an SVG element (throws, aborting hydration for the rest of the page). | |
| 55 | const rt = generateSignalsRuntimeDev() | |
| 56 | expect(rt).toContain(`el.setAttribute('class'`) | |
| 57 | }) | |
| 58 | }) | |