diff --git a/skill/scripts/live-browser.js b/skill/scripts/live-browser.js index db05ec54f..b32a4fac2 100644 --- a/skill/scripts/live-browser.js +++ b/skill/scripts/live-browser.js @@ -5446,6 +5446,16 @@ const texts = collectVisibleTexts(itemEl).filter((t) => !statics.has(t)); const item = {}; slots.forEach((slot, i) => { item[slot.key] = texts[i] != null ? texts[i] : ''; }); + // Attribute-bound values (href={link.href}) hydrate from the + // rendered attribute on the live item element or a descendant. + for (const slot of entry.item.attrSlots || []) { + if (item[slot.key] != null || !slot.tag) continue; + const sel = slot.tag + (slot.classes || []).map((c) => '.' + cssEscapeIdent(c)).join(''); + let el = null; + try { el = itemEl.matches(sel) ? itemEl : itemEl.querySelector(sel); } catch { el = null; } + const value = el ? el.getAttribute(slot.attr) : null; + if (value != null) item[slot.key] = value; + } // Keyed each: the key field is never rendered, so hydrate it with a // unique per-index value or Svelte throws each_key_duplicate. if (entry.item.keyField && item[entry.item.keyField] == null) { diff --git a/skill/scripts/live/svelte-ast.mjs b/skill/scripts/live/svelte-ast.mjs index 3462b9760..72733cbfd 100644 --- a/skill/scripts/live/svelte-ast.mjs +++ b/skill/scripts/live/svelte-ast.mjs @@ -434,6 +434,12 @@ function analyzeAttributes(node, analysis, scopes) { // Unlike ClassDirective, a style directive stores its value in // attribute shape: `true` for the shorthand, else an array of parts. const parts = attr.value === true ? [] : (Array.isArray(attr.value) ? attr.value : [attr.value]); + for (const part of parts) { + if (part?.type === 'ExpressionTag' + && failOnMixedExpression(part.expression, scopes, analysis, analysis.source)) { + return; + } + } const dynamic = parts.some((part) => part?.type === 'ExpressionTag' && isFree(part.expression, scopes)); const shorthandFree = attr.value === true && isFree({ type: 'Identifier', name: attr.name }, scopes); if (dynamic || shorthandFree) { @@ -485,9 +491,6 @@ function analyzeAttributes(node, analysis, scopes) { function describeEachItem(node, source) { const body = node.body; const rootEl = (body?.nodes || []).find((n) => n.type === 'RegularElement'); - const bound = new Set(); - if (node.context) collectPatternNames(node.context, bound); - if (node.index) bound.add(node.index); const textSlots = []; const staticTexts = []; @@ -508,34 +511,162 @@ function describeEachItem(node, source) { } }; collectStatics(body); - const walkForSlots = (fragment, scopes) => { + const attrSlots = []; + // The hydration item is a SHALLOW object whose string fields are the exact + // property names the markup accesses, filled from the rendered page. That + // model supports one item access per slot, optionally wrapped in a global + // transform ({Math.round(r.score)} hydrates `score`). Shapes it cannot + // represent split two ways: CRASHY ones would throw at mount time against a + // shallow item (deep paths like r.meta.label, method calls like r.format()) + // and force the source-preview fallback; LOSSY ones render wrong but safe + // (bare {r}, multi-access expressions that would double their text) and + // also fall back in text position, where the damage is visible. + const boundAs = (name, scopeInfos) => { + for (let i = scopeInfos.length - 1; i >= 0; i--) { + const info = scopeInfos[i]; + if (info.indexName === name) return 'index'; + if (info.itemName === name) return 'item'; + if (info.names.has(name)) return 'field'; + } + return null; + }; + const slotKeysOf = (expression, scopeInfos) => { + const keys = new Set(); + let crashy = false; + let lossy = false; + let touches = false; + const visit = (node, ctx) => { + if (!node || typeof node !== 'object' || crashy) return; + if (Array.isArray(node)) { + for (const item of node) visit(item, {}); + return; + } + switch (node.type) { + case 'Identifier': { + const kind = boundAs(node.name, scopeInfos); + if (!kind) return; + touches = true; + if (kind === 'index') return; // the runtime each provides it + if (kind === 'item') { lossy = true; return; } // bare item reference + if (ctx.callee) { crashy = true; return; } // field() on a hydrated string + keys.add(node.name); // destructured context field + return; + } + case 'MemberExpression': { + if ( + !node.computed + && node.object?.type === 'Identifier' + && boundAs(node.object.name, scopeInfos) === 'item' + && node.property?.type === 'Identifier' + ) { + touches = true; + // item.a.b or item.method(): a shallow string field throws here. + if (ctx.memberObject || ctx.callee) { crashy = true; return; } + keys.add(node.property.name); + return; + } + visit(node.object, { memberObject: true }); + if (node.computed) visit(node.property, {}); + return; + } + case 'CallExpression': + visit(node.callee, { callee: true }); + for (const arg of node.arguments || []) visit(arg, {}); + return; + case 'ArrowFunctionExpression': + case 'FunctionExpression': { + // Closures cannot hydrate; only lossy when they capture the item. + const roots = collectRootIdentifiers(node); + if ([...roots].some((name) => boundAs(name, scopeInfos))) { touches = true; lossy = true; } + return; + } + case 'Property': + if (node.computed) visit(node.key, {}); + visit(node.value, {}); + return; + default: { + for (const key of Object.keys(node)) { + if (key === 'type' || key === 'start' || key === 'end' || key === 'loc' || key === 'range' || key === 'parent') continue; + visit(node[key], {}); + } + } + } + }; + visit(expression, {}); + if (crashy) return { crashy: true }; + if (lossy || keys.size > 1) return { lossy: true }; + if (!touches || keys.size === 0) return { skip: true }; + return { key: [...keys][0] }; + }; + const staticClassesOf = (el) => { + const classes = []; + for (const attr of el?.attributes || []) { + if (attr.type === 'Attribute' && attr.name === 'class' && Array.isArray(attr.value)) { + for (const part of attr.value) { + if (part.type === 'Text') classes.push(...part.data.split(/\s+/).filter(Boolean)); + } + } + } + return classes; + }; + const scopeInfoOf = (eachNode) => { + const names = new Set(); + if (eachNode.context) collectPatternNames(eachNode.context, names); + return { + names, + itemName: eachNode.context?.type === 'Identifier' ? eachNode.context.name : null, + indexName: eachNode.index || null, + }; + }; + const walkForSlots = (fragment, scopeInfos) => { for (const child of fragment?.nodes || []) { if (child.type === 'ExpressionTag') { - const roots = collectRootIdentifiers(child.expression); - const referencesItem = [...roots].some((name) => scopes.some((s) => s.has(name))); - if (referencesItem) { - textSlots.push({ - key: derivePropName(exprText(source, child.expression)), - expr: exprText(source, child.expression), - }); + const slot = slotKeysOf(child.expression, scopeInfos); + if (slot.crashy || slot.lossy) { nestedUnsupported = true; continue; } + if (slot.skip) continue; + textSlots.push({ key: slot.key, expr: exprText(source, child.expression) }); + } else if (child.type === 'RegularElement' || child.type === 'SvelteElement') { + // Bound values in ATTRIBUTES (href={link.href}, src={item.img}) are + // part of the item too: the browser reads the rendered attribute off + // the live element, so the preview does not mount with empty links. + // Only a single-expression attribute hydrates exactly; a mixed value + // ("card {r.status}") stays unhydrated because the rendered attribute + // is not separable into its parts, which was the prior behavior. + for (const attr of child.attributes || []) { + if (attr.type !== 'Attribute' || attr.value === true) continue; + if (HANDLER_ATTR_RE.test(attr.name)) continue; // functions cannot hydrate + const parts = Array.isArray(attr.value) ? attr.value : [attr.value]; + const exprParts = parts.filter((part) => part?.type === 'ExpressionTag'); + for (const part of exprParts) { + const slot = slotKeysOf(part.expression, scopeInfos); + if (slot.crashy) { nestedUnsupported = true; continue; } + if (slot.skip || slot.lossy) continue; + if (parts.length !== 1) continue; // mixed static+dynamic value + attrSlots.push({ + key: slot.key, + expr: exprText(source, part.expression), + attr: attr.name, + tag: child.name || null, + classes: staticClassesOf(child), + }); + } } + walkForSlots(child.fragment, scopeInfos); + continue; } else if (child.type === 'EachBlock') { const roots = collectRootIdentifiers(child.expression); - const boundNested = [...roots].some((name) => scopes.some((s) => s.has(name))); + const boundNested = [...roots].some((name) => boundAs(name, scopeInfos)); if (boundNested) nestedUnsupported = true; // nested per-item arrays: no hydration plan yet - const innerBound = new Set(); - if (child.context) collectPatternNames(child.context, innerBound); - if (child.index) innerBound.add(child.index); - walkForSlots(child.body, [...scopes, innerBound]); + walkForSlots(child.body, [...scopeInfos, scopeInfoOf(child)]); } else if (child.type === 'IfBlock') { - walkForSlots(child.consequent, scopes); - if (child.alternate) walkForSlots(child.alternate, scopes); + walkForSlots(child.consequent, scopeInfos); + if (child.alternate) walkForSlots(child.alternate, scopeInfos); } else if (child.fragment) { - walkForSlots(child.fragment, scopes); + walkForSlots(child.fragment, scopeInfos); } } }; - walkForSlots(body, [bound]); + walkForSlots(body, [scopeInfoOf(node)]); const staticClasses = []; for (const attr of rootEl?.attributes || []) { @@ -550,6 +681,7 @@ function describeEachItem(node, source) { rootTag: rootEl?.name || null, rootClasses: staticClasses, textSlots, + attrSlots, staticTexts, nestedUnsupported, }; @@ -635,7 +767,7 @@ export function analyzeSvelteMarkup(markup, parse) { } for (const entry of analysis.contract) { if (entry.kind === 'collection' && entry.item?.nestedUnsupported) { - return { ok: false, reason: 'nested per-item each blocks require source-preview mode' }; + return { ok: false, reason: 'per-item content (nested blocks or expressions) this preview cannot hydrate requires source-preview mode' }; } } diff --git a/tests/framework-fixtures/vite8-sveltekit-stateful/files/src/routes/+page.svelte b/tests/framework-fixtures/vite8-sveltekit-stateful/files/src/routes/+page.svelte index 6688f43f2..ad4cb43fd 100644 --- a/tests/framework-fixtures/vite8-sveltekit-stateful/files/src/routes/+page.svelte +++ b/tests/framework-fixtures/vite8-sveltekit-stateful/files/src/routes/+page.svelte @@ -8,9 +8,9 @@ }); const CATALOG = [ - { name: 'Design snack', amount: '$12' }, - { name: 'Studio coffee', amount: '$8' }, - { name: 'Type license', amount: '$44' }, + { name: 'Design snack', amount: '$12', doc: '/receipts/snack' }, + { name: 'Studio coffee', amount: '$8', doc: '/receipts/coffee' }, + { name: 'Type license', amount: '$44', doc: '/receipts/type' }, ]; function addExpense() { @@ -40,6 +40,7 @@
  • {expense.name} {expense.amount} + Beleg
    • {/each} diff --git a/tests/framework-fixtures/vite8-sveltekit-stateful/fixture.json b/tests/framework-fixtures/vite8-sveltekit-stateful/fixture.json index b83acc5e9..945c90668 100644 --- a/tests/framework-fixtures/vite8-sveltekit-stateful/fixture.json +++ b/tests/framework-fixtures/vite8-sveltekit-stateful/fixture.json @@ -1,23 +1,38 @@ { "name": "Vite 8 + SvelteKit stateful page", "config": { - "files": ["src/app.html"], + "files": [ + "src/app.html" + ], "insertBefore": "", "commentSyntax": "html" }, - "sourceFiles": ["DESIGN.md", "src/app.html", "src/routes/+page.svelte", "src/routes/+layout.svelte", "svelte.config.js", "vite.config.js"], + "sourceFiles": [ + "DESIGN.md", + "src/app.html", + "src/routes/+page.svelte", + "src/routes/+layout.svelte", + "svelte.config.js", + "vite.config.js" + ], "generatedFiles": [], "wrapCases": [ { "name": "wraps hero title through Svelte component preview", - "args": { "classes": "hero-title", "tag": "h1" }, + "args": { + "classes": "hero-title", + "tag": "h1" + }, "expectedFile": "node_modules/.impeccable-live/wraptest0/manifest.json", "expectedSourceFile": "src/routes/+page.svelte", "expectedPreviewMode": "svelte-component" }, { "name": "wraps the each-block list through Svelte component preview", - "args": { "classes": "expense-list", "tag": "ul" }, + "args": { + "classes": "expense-list", + "tag": "ul" + }, "expectedFile": "node_modules/.impeccable-live/wraptest1/manifest.json", "expectedSourceFile": "src/routes/+page.svelte", "expectedPreviewMode": "svelte-component" @@ -25,27 +40,65 @@ ], "runtime": { "styling": "plain-css", - "install": ["npm", "install", "--no-audit", "--no-fund", "--loglevel=error"], - "devCommand": ["npx", "vite", "dev", "--host", "127.0.0.1"], + "install": [ + "npm", + "install", + "--no-audit", + "--no-fund", + "--loglevel=error" + ], + "devCommand": [ + "npx", + "vite", + "dev", + "--host", + "127.0.0.1" + ], "readyPattern": "Local:\\s+https?://[^:]+:(\\d+)", "readyTimeoutMs": 120000, "steer": false, "pickSelector": "ul.expense-list", - "pickPosition": { "x": 10, "y": 10 }, - "variantSequence": [3, 1, 2], + "pickPosition": { + "x": 10, + "y": 10 + }, + "variantSequence": [ + 3, + 1, + 2 + ], "acceptedSourcePattern": "]*class=\"[^\"]*\\bexpense-list\\b", "assertSourceContains": [ "{#each expenses as expense, i}", "{expense.name}", - "{expense.amount}" + "{expense.amount}", + "href={expense.doc}" ], "preActions": [ - { "type": "click", "selector": "[data-testid='add-expense']" }, - { "type": "wait", "selector": "[data-testid='expense-row'][data-index='0']" }, - { "type": "click", "selector": "[data-testid='add-expense']" }, - { "type": "wait", "selector": "[data-testid='expense-row'][data-index='1']" }, - { "type": "click", "selector": "[data-testid='add-expense']" }, - { "type": "wait", "selector": "[data-testid='expense-row'][data-index='2']" } + { + "type": "click", + "selector": "[data-testid='add-expense']" + }, + { + "type": "wait", + "selector": "[data-testid='expense-row'][data-index='0']" + }, + { + "type": "click", + "selector": "[data-testid='add-expense']" + }, + { + "type": "wait", + "selector": "[data-testid='expense-row'][data-index='1']" + }, + { + "type": "click", + "selector": "[data-testid='add-expense']" + }, + { + "type": "wait", + "selector": "[data-testid='expense-row'][data-index='2']" + } ], "stateProbe": { "textSelector": "[data-testid='open-count']", @@ -60,13 +113,32 @@ "rangeValue": 1.8, "stepsLabel": "Density", "stepsOptionLabel": "Snug", - "expectSourceContains": ["line-height: 1.8", "letter-spacing: 0.01em"], - "expectSourceMissing": ["letter-spacing: 0.14em"] + "expectSourceContains": [ + "line-height: 1.8", + "letter-spacing: 0.01em" + ], + "expectSourceMissing": [ + "letter-spacing: 0.14em" + ] + }, + "componentFailureScenarios": { + "variant": 2, + "storageLoss": false }, - "componentFailureScenarios": { "variant": 2, "storageLoss": false }, "probe": { "expectLiveInit": true, "expectConsoleClean": true - } + }, + "mountedDomProbe": [ + { + "selector": "ul.expense-list a.expense-doc", + "attr": "href", + "expect": "/receipts/snack" + }, + { + "selector": "ul.expense-list strong.expense-name", + "expect": "Design snack" + } + ] } } diff --git a/tests/live-e2e.test.mjs b/tests/live-e2e.test.mjs index 36d08a5b2..24e0f0c12 100644 --- a/tests/live-e2e.test.mjs +++ b/tests/live-e2e.test.mjs @@ -512,6 +512,32 @@ for (const { name, fixture } of fixtures) { ); }; await assertVisibleVariantStyle(visible); + // Optional fixture hook: assert attribute/text values inside the + // MOUNTED variant DOM. Component previews hydrate collection items + // from the rendered page (text slots and attribute slots); a probe + // here catches a preview that mounts but with empty hydrated values, + // which every other assertion (style, counter, accept) misses. + if (Array.isArray(fixture.runtime.mountedDomProbe)) { + for (const probe of fixture.runtime.mountedDomProbe) { + const actual = await evaluatePageWithTimeout( + page, + ({ sel, attr }) => { + const query = window.__impeccableLiveQuery || ((s) => document.querySelector(s)); + const el = query(sel) || document.querySelector(sel); + if (!el) return null; + return attr ? el.getAttribute(attr) : (el.textContent || '').trim(); + }, + { sel: probe.selector, attr: probe.attr || null }, + 5_000, + 'mounted DOM probe', + ); + assert.equal( + actual, + probe.expect, + `mounted variant DOM: ${probe.selector}${probe.attr ? ` [${probe.attr}]` : ' text'}`, + ); + } + } for (const targetVariant of cycleSequence) { t.diagnostic(`Cycling to variant ${targetVariant}`); visible = await cycleToVariant(page, targetVariant, expectedCount, { diff --git a/tests/live-svelte-ast.test.mjs b/tests/live-svelte-ast.test.mjs index a68f27e8a..31f0bd020 100644 --- a/tests/live-svelte-ast.test.mjs +++ b/tests/live-svelte-ast.test.mjs @@ -258,3 +258,39 @@ describe('review regressions: mixed and global identifiers', () => { assert.equal(topRes.contract.length, 0); }); }); + +describe('review regressions: attribute slots and hydration honesty', () => { + it('records attribute-bound values as attr slots', () => { + const src = ``; + const res = analyzeSvelteMarkup(src, parse); + assert.equal(res.ok, true, res.reason); + const item = res.contract.find((c) => c.kind === 'collection').item; + assert.deepEqual(item.textSlots.map((s) => s.key), ['text']); + assert.deepEqual(item.attrSlots, [{ key: 'href', expr: 'link.href', attr: 'href', tag: 'a', classes: ['nav-link'] }]); + }); + + it('falls back for per-item expressions the shallow item cannot represent', () => { + for (const src of [ + ``, + ``, + ``, + ]) { + const res = analyzeSvelteMarkup(src, parse); + assert.equal(res.ok, false, `expected fallback for ${src}`); + assert.match(res.reason, /cannot hydrate/); + } + }); + + it('index-only expressions need no slot', () => { + const res = analyzeSvelteMarkup(``, parse); + assert.equal(res.ok, true, res.reason); + const item = res.contract.find((c) => c.kind === 'collection').item; + assert.deepEqual(item.textSlots.map((s) => s.key), ['name']); + }); + + it('style directives mixing loop and outer names fall back', () => { + const res = analyzeSvelteMarkup(``, parse); + assert.equal(res.ok, false); + assert.match(res.reason, /mixing loop and outer identifiers/); + }); +});