fix: hydrate attribute-bound each values and guard style directives

Addresses two cursor review findings:

- {#each} bodies whose bound values appear in attributes (href={link.href},
  src={item.img}) now record attr slots; the browser hydrates them from the
  rendered attribute so component previews no longer mount with empty links.
  Single-expression attributes hydrate exactly; mixed values stay unhydrated
  as before. A new slot classifier also refuses shapes that would crash a
  shallow hydration item (deep paths, method calls, bare item renders) and
  routes them to source-preview mode instead.
- Style directives now run the mixed loop/outer identifier check before the
  free-identifier param check, so style:width={base + r.pct} falls back
  instead of minting a broken param.

Tests: attr-slot analysis units, crashy/lossy fallback units, an attribute-
bound anchor in the stateful SvelteKit fixture asserted through accept, and
a mountedDomProbe e2e hook that reads the hydrated href off the mounted
variant DOM (verified to fail when hydration is disabled).

AI-assisted (Claude Code).

Co-Authored-By: Claude Code <noreply@anthropic.com>
This commit is contained in:
Paul Bakaus
2026-07-28 15:33:16 -07:00
co-authored by Claude Code
parent 6997e4bdb5
commit 39f233ac24
6 changed files with 320 additions and 43 deletions
+10
View File
@@ -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) {
+153 -21
View File
@@ -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' };
}
}
@@ -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 @@
<li class="expense-row" data-testid="expense-row" data-index={i}>
<strong class="expense-name">{expense.name}</strong>
<span class="expense-amount">{expense.amount}</span>
<a class="expense-doc" href={expense.doc}>Beleg</a>
</li>
{/each}
</ul>
@@ -1,23 +1,38 @@
{
"name": "Vite 8 + SvelteKit stateful page",
"config": {
"files": ["src/app.html"],
"files": [
"src/app.html"
],
"insertBefore": "</body>",
"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": "<ul[^>]*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"
}
]
}
}
+26
View File
@@ -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, {
+36
View File
@@ -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 = `<nav>{#each links as link}<a class="nav-link" href={link.href}>{link.text}</a>{/each}</nav>`;
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 [
`<ul>{#each rows as r}<li>{r.meta.label}</li>{/each}</ul>`,
`<ul>{#each rows as r}<li>{r.format()}</li>{/each}</ul>`,
`<ul>{#each rows as r}<li>{r}</li>{/each}</ul>`,
]) {
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(`<ul>{#each rows as r, i}<li>{i}: {r.name}</li>{/each}</ul>`, 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(`<ul>{#each rows as r}<li style:width={base + r.pct}>x</li>{/each}</ul>`, parse);
assert.equal(res.ok, false);
assert.match(res.reason, /mixing loop and outer identifiers/);
});
});