fix: make the SSR attribute reader see the browser's attribute set

[vivek7405]

Closes #1341

Summary

The SSR attribute reader and the browser's attribute reader did not see the same set of attributes, so for certain hand-written markup one reader consumed an attribute the other never saw. The SSR'd first paint held one value, the upgraded element held another, and nothing errored.

Both readers now resolve a name through one resolveAttributeProperty in packages/core/src/attribute-reader.js, matching exactly the d.attribute || hyphenate(k) expression observedAttributes maps over and nothing else, so the two agree by construction rather than by two copies staying in step. The SSR caller supplies the name the platform would deliver (its lowercased source name). A new decodeAttrEntities replaces the three-entity unescapeAttr and runs once per attribute, ahead of the type coercion, so every branch gets a decoded value rather than only the Object / Array one.

What changed for an existing app

Four behaviour changes. The first three are SSR reading LESS:

• a state: true prop is no longer populated from a source attribute at SSR;
• a camelCase attribute name in markup no longer resolves at SSR;
• a prop that renames its attribute ({ attribute: 'is-open' }) no longer also answers to its property name, at SSR;
• and SSR now decodes every HTML character reference, where before it reversed three.

All four make SSR agree with what the browser already did, so an app relying on any of the first three was already broken after hydration. There is no back-compat flag, per the issue.

Merge order

#1340 lands first and owns the VALUE half of the shared reader (readAttributeValue plus its converter arm). This PR does the NAME half plus the decoder, and deliberately leaves applyAttrsToInstance's own type-coercion chain in place rather than building #1340's extraction a second time. On the rebase onto main after #1340 merges, that chain becomes a readAttributeValue(def, value) call and #1340's decode parameter is deleted, which is this issue's Step 6. attribute-reader.js is where that function belongs when it arrives, for the reason its header gives.

Deviations from the plan, all deliberate

The entity table has 2125 entries, not the 2124 the issue states. https://html.spec.whatwg.org/entities.json carries 2231 keys: 2125 semicolon-terminated and 106 semicolon-less. The issue's own 106 figure implies 2125, so the 2124 was off by one. 93 entries map to two code points, as stated.

The 106 legacy semicolon-less names ARE decoded, where the issue scoped them out as a non-goal. Both reasons the issue gave for excluding them are wrong, checked against Chromium, Firefox, and WebKit: &nbsp is a recognised name that a browser really does decode (s="&nbsp" reaches a reader holding U+00A0, not five literal characters), and the rule that governs it is a one-character lookahead rather than tokenizer state, so it is reproducible here. Leaving them literal would have been a live value divergence of exactly the kind this PR exists to remove, documented as intentional. The decoder decodes a legacy name only when the next character is not =, so &nbsp=x stays literal; &nbspx and &notin stay literal for a different reason, because the whole alphanumeric run is captured and neither of those is a legacy name. All three verified against the three engines.

The props[name] || props[camelCase(name)] fallback is REMOVED, where Decision 2 said to keep it. Its stated justification was that the client carries the same fallback so both sides agree on it. They do not: the browser only calls the client reader with a name from observedAttributes, and that list holds the declared attribute alone, so the fallback is unreachable on the client and live on the server. Measured, open: prop(Boolean, { attribute: 'is-open' }) with <my-el open> SSR'd true and upgraded to false. Keeping it would have left the headline divergence in the function written to remove it.

resolveAttributeProperty lives in src/attribute-reader.js, not in component.js (the issue's Step 5 and Step 7). Two guards bracket every mapped exports entry and ./component is one: #388 requires every runtime named export of component.js to be declared in component.d.ts, and #1031 requires every declaration reachable from index.d.ts (which re-exports it with export *) to exist on index.js at runtime. So a named export added to component.js is either published as root API or carries the _ test-only prefix packages/core/AGENTS.md documents. A module with no exports entry says what the seam is without overloading a convention that means something else. Nothing is re-exported from index.js or index-browser.js, per the issue.

Test plan

• Unit, SSR (packages/core/test/rendering/ssr-prop-options.test.js): 39 added covering the state skip, the .prop channel that must keep working, the camelCase and kebab names, the renamed-attribute rule in both directions, the unmapped-attribute case, the full entity matrix on String (named, decimal, hex, missing-semicolon, C1, null, surrogate, over-range, all four legacy shapes, and the three carve-out shapes) plus the Object and Number branches, the two double-decoding counterfactuals as named tests, the seven Object.prototype names plus &__proto__; as individually named tests, the legacy-versus-table invariant with both entry counts, getAttribute() through seedServerAttrs, and a byte-identical assertion on framework-emitted markup captured from origin/main at 207f216.
• Unit, client (packages/core/test/lifecycle/component-lifecycle.test.js): 2 added for the two client-visible consequences. The existing coercion tests are untouched and green, which is the proof that routing the client through the shared resolver changed nothing.
• Unit, updated: the two tests in reflect-function-guard.test.js that pinned the old divergence. They were written with a note saying The SSR and client attribute readers see different attribute sets #1341 would be the change that made them notice, and it is; they now pin the agreement.
• Browser (packages/core/test/rendering/browser/ssr-client-parity.test.js): 7 added, each through a REAL element upgrade (customElements.whenDefined plus updateComplete), then the same markup through renderToString, comparing the two. A throwaway differential harness ran 58 entity shapes the same way, comparing getAttribute() against the SSR decoder, and reported zero mismatches on all three engines. npm run test:browser green: Chromium 846, Firefox 836, Webkit 846, 0 failed.
• Bun (test/bun/attribute-reader-parity.mjs plus its .test.mjs wrapper): green on node and on bun 1.3.14. node scripts/run-bun-tests.js: 303 pass, 27 documented node-only skips, 0 genuine failures.
• Node (npm test): 4190 pass, 0 fail.
• Crash regression caught in review: reading the 2125-key table by indexing the object literal resolved through Object.prototype, so &constructor; and six sibling names returned a function and threw out of the decoder, painting the SSR error box for any attribute of any custom element, while a browser leaves them literal. It is read through a Map now. Covered at three layers (seven named SSR tests, eight cross-runtime rows, and a browser test reading getAttribute() off a real upgrade) and counterfactualled.
• Counterfactuals: eight, each reverted and re-run (committing the fix first, so the revert restores it rather than discarding it). Dropping the state skip reds SSR unit 1, client unit 1, and the Bun script; dropping .toLowerCase() reds the camelCase tests; restoring instance[propName] = raw reds the unmapped-attribute test; restoring the three-entity decoder reds 22 entity rows and the Bun script; wrapping the decode in a rescan loop reds both double-decoding tests and the Bun script; disabling the legacy arm reds the four legacy rows; removing the lookahead carve-out reds the three carve-out rows plus &notreal;; restoring the property-name fallback reds the renamed-attribute test and the Bun script; and reading the entity table by indexing the object rather than through the Map reds all seven prototype-name tests.
• Conventions: webjs check clean on examples/blog and website; webjs doctor 11 passed, 2 warnings, 0 failed on both (the two warnings are pre-existing).
• Dogfood: website boots in prod mode with /, /docs/components, /ui, /ui/button all 200 and no broken modulepreload (7, 12, 12, and 50 preload hints probed). Blog covered by the e2e suite and its smoke tests, green.
• Browser bundle: render-server.js and html-entities.js are both absent from the built dist/webjs-core-browser.js, so the table costs the browser nothing.
• e2e / smoke / webjs check rules: N/A. The change is two pure functions plus a data table inside renderToString and one class method. No route, network path, navigation, or streaming involvement; the only user-observable surface is SSR-versus-upgrade agreement, which the browser layer asserts directly and more strongly. No scaffold or example-app surface changes, and no webjs check rule is added or changed.

Docs

• website/app/docs/components/page.ts: a coercion bullet for decoding on every type including the legacy rule, the kebab-case rule, the renamed-attribute rule, and the state: true / .prop note.
• .agents/skills/webjs/references/components.md: the state row, the same-attribute-set sentence, and a paragraph on writing attributes in markup.
• Root AGENTS.md: one sentence after the property options.
• Source comments: the two Object / Array branch paragraphs now say the cases are closed, with identical text in both files, and decodeAttrEntities and attribute-reader.js carry their own reasoning.
• packages/cli/templates/.agents/skills/webjs/references/components.md does not exist (it is copied at prepack), so there is no scaffold copy to sync. README.md needs nothing.

[vivek7405]

Went looking for whether the two readers really do agree now, and three places they still do not.

The one that matters most is the resolver fallback. It reads like a shared rule and is not one: the browser only ever calls the client reader with a name from observedAttributes, and that list is exactly d.attribute || hyphenate(k), so the props[name] arm after the loop is dead on the client and live on the server. That is the same read-more-than-the-platform bug as the camelCase case, sitting in the function written to remove it, with a new test pinning it as intended.

The legacy semicolon-less names are the same shape. I took the non-goal on faith from the plan and the plan is wrong about the mechanism: the rule is a one-character lookahead, not tokenizer state, and browsers decode &nbsp rather than tolerating it. So it is a live value divergence, documented as a deliberate omission.

The third is smaller but it reds CI, and the comment I left recording why is wrong about there being no alternative.

[vivek7405]

Design rationale: why the resolver derives its match from observedAttributes, and why the legacy entities came back in scope

Two calls here went against the issue's plan, and both came from the same realisation, so they are worth writing down together.

The plan framed the fix as three edits plus a shared resolver, and settled each of them by asking what a browser does. That is the right question, and the place it was answered from is what turned out to be unreliable: for both the property-name fallback and the legacy entity names, the plan reasoned about the browser from the client reader's SOURCE. The client's code carried a props[name] fallback, so the plan concluded the two sides agreed on it and the fallback should stay. But the client reader is not the whole client path. observedAttributes sits in front of it and decides which names are ever delivered, and that list is exactly d.attribute || hyphenate(k), which is what the resolver's loop already matches. So the fallback was dead code on the client and live code on the server, and keeping it left <my-el open> SSR'ing true and upgrading to false inside the function written to stop exactly that.

That is why the resolver now matches that one expression and stops. It is not a smaller version of the old resolver; it is the same expression observedAttributes is built from, so the two sides agree because they are computed from one source rather than because two copies were kept in step. Any future divergence has to go through that expression, which is the property the issue wanted and the reason it asked for a shared resolver in the first place.

The legacy semicolon-less names went the same way. The plan recorded them as a deliberate non-goal on two grounds: that nothing decodes them because they are HTML5 parse errors, and that the rule needs tokenizer state a standalone function cannot have. Both are wrong. s="&nbsp" reaches a reader holding U+00A0 in Chromium, Firefox, and WebKit alike, and the rule is a one-character lookahead: consume the longest name in the table, decode unless the next character is = or ASCII alphanumeric. A parse error is not the same thing as a no-op. Left alone it would have been a plain value divergence, in a PR whose subject is value divergences, documented in the source and the user docs as intentional.

What I take from both: when a decision rests on "the browser does X", the evidence has to be the browser. Every claim in this PR that names browser behaviour was run against all three engines, and a throwaway differential harness compared the SSR decoder against getAttribute() over 32 entity shapes before I trusted any of it.

The third change is smaller and purely structural. resolveAttributeProperty sits in its own attribute-reader.js rather than in component.js because ./component is a mapped exports entry, and two guards bracket those: one requires every runtime named export to be declared in the overlay, the other requires every declaration reachable from index.d.ts to exist on index.js. Since index.d.ts re-exports the overlay wholesale, an internal named export on component.js is either published as root API or wears the _ test-only prefix. Neither fits a seam between two shipping modules. The value half of the reader will hit the same wall when it arrives, so the module is sized for both.

[vivek7405]

Checked the fixes from the last round and traced what they touched. The parity work itself holds, but the wider decoder brought in a crash that is worse than the gap it closed.

Reading a 2125-key object literal by name means every lookup goes through Object.prototype, and seven of those inherited names are ordinary-looking references. &constructor; in any attribute of any custom element takes down the component. seedServerAttrs decodes every source attribute, not just the prop-mapped ones, so the blast radius is every element on the page, and the failure mode is the SSR error box rather than a wrong value.

The other two are smaller: a branch that cannot run with a comment saying it handles a real case, and a header comment pointing at the file the resolver just moved out of.

[vivek7405]

Read the crash fix and traced what it left behind. The fix itself is right: I ran the old and new resolver differentially over roughly 29,000 inputs, every table name and every legacy name against a spread of suffixes, and the only behaviour that moved is the fourteen cases it set out to move.

What is wrong is everything around it. Deleting the prefix walk changed WHY several cases come out literal, and none of the prose caught up: two docblocks in the same file now describe two different algorithms, and four test comments explain rows by a mechanism that no longer exists. Worth fixing rather than shrugging at, because the next person to touch the carve-out will read those comments and conclude the wrong thing about which shapes exercise it.

The coverage gap matters more. The regression this commit fixes is a render-server change with no cross-runtime row and no browser assertion, in a PR where every other entity claim was measured against three engines.

[vivek7405]

Two more from the same read, both on lines outside this commit's own hunks so they would not anchor inline.

test/bun/attribute-reader-parity.mjs has no row for the shape the crash fix exists to fix. The commit stages render-server.js, which is on the runtime-sensitive list the Bun parity hook matches, and stages no test/bun change, so the regression had no cross-runtime cover at all. Same for the browser layer: ssr-client-parity.test.js had no prototype-name case, while the SSR test file's own header promises a browser half for every case under it. That left "a browser leaves those literal" resting on reasoning in a PR where every other entity claim was measured against three engines.

And packages/core/src/html-entities.js still says it is imported ONLY by render-server.js, which stopped being true when the test started importing it to assert the entry counts.

Both fixed in e29cf437. The prototype names now have eight rows in the cross-runtime script and a browser test that reads getAttribute() off a real upgrade and compares it against the SSR render, and the header names its second importer. Worth noting for whoever looks at the hook: it is wired in .claude/settings.json and did not fire on that commit, so the gate it describes is not currently holding.