Skip to content

Commit ba991cd

Browse files
author
DevBot
committed
fix(element): attribute promotion must not be clobbered by pre-upgrade null sets (#1318)
Hydration on every non-default-locale page failed with `[compiled-claim] item attribute drift on "href"`: the claim-time re-derivation of computed shell chrome (sidebar rows, footer columns) saw default-value signals while the SSR DOM was rendered from locale-injected props. Root cause chain (facade-host): 1. A pending pre-upgrade property write of `locale: null` (no real assignment intent) was coerced by applyPendingOwnValues into the compiled default ('en') and applied AFTER syncAttributesToSignals — clobbering the attribute-promoted 'zh'. 2. Generated field initializers that merely restate the compiled default were likewise captured as pending pre-upgrade sets. Fixes: - restatesDefault(): generated initializers that restate the compiled default (their documented contract, see reconcileOwnProperties) no longer enter pendingOwnValues. - applyPendingOwnValues(): null/undefined pending values carry no pre-upgrade intent and are skipped instead of being coerced to the default over the promoted attribute value. Element suite: 254 passed. WWW e2e chromium: 219 passed including public-routes zh sweep and visual baselines.
1 parent 2f0780b commit ba991cd

3 files changed

Lines changed: 53 additions & 1 deletion

File tree

packages/element/src/internal/compiled/facade-host.ts

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,22 @@ function ownDataValue(element: object, name: string): { found: boolean; value?:
126126
return { found: true, value: descriptor.value };
127127
}
128128

129+
/**
130+
* Whether an own data property value merely restates the compiled default
131+
* (the generated field initializer's contract, see reconcileOwnProperties)
132+
* rather than carrying a real pre-upgrade assignment. Arrays/objects compare
133+
* structurally because the compiler default for them is typically an inline
134+
* literal evaluated per instance.
135+
*/
136+
function restatesDefault(record: CompiledPropertyMetadata, value: unknown): boolean {
137+
const fallback = record.default;
138+
if (value === fallback) return true;
139+
if (Array.isArray(fallback) || (typeof fallback === 'object' && fallback !== null)) {
140+
return JSON.stringify(value) === JSON.stringify(fallback);
141+
}
142+
return false;
143+
}
144+
129145
/**
130146
* Create the per-instance property state: one signal per compiled property at
131147
* its compiled default, plus any property values set as own data properties
@@ -150,7 +166,19 @@ export function createFacadePropertyState(
150166
if (property.computed) continue;
151167
const own = ownDataValue(element, property.name);
152168
if (own.found) {
153-
pending.set(property.name, own.value);
169+
// Generated class-field initializers restate the compiled default
170+
// (reconcileOwnProperties' own contract). They are NOT pre-upgrade JS
171+
// sets: capturing a restated default into pendingOwnValues would let it
172+
// clobber the SSR attribute promotion at connect (applyPendingOwnValues
173+
// runs after syncAttributesToSignals), so a computed field deriving from
174+
// the property would claim against default-value-derived chrome while
175+
// the SSR DOM was rendered from the injected props — the
176+
// "[compiled-claim] item attribute drift" seen on every non-default
177+
// locale page (#1318). Only a value that actually differs from the
178+
// compiled default is a real pre-upgrade set.
179+
if (!restatesDefault(property, own.value)) {
180+
pending.set(property.name, own.value);
181+
}
154182
delete record[property.name];
155183
}
156184
signals[property.name] = signal<unknown>(property.default);
@@ -294,6 +322,12 @@ export function applyPendingOwnValues(state: FacadePropertyState): void {
294322
const record = state.properties.find((candidate) => candidate.name === name);
295323
if (record?.computed) continue;
296324
const sig = record ? state.signals[record.name] : undefined;
325+
// A pending null/undefined carries no pre-upgrade assignment intent
326+
// (coercePropertyValue would map it to the compiled default): letting it
327+
// through would clobber the attribute-promoted value at connect and
328+
// desync every computed field deriving from that property (#1318 — the
329+
// "[compiled-claim] item attribute drift" on non-default locales).
330+
if (value === null || value === undefined) continue;
297331
if (record && sig) sig.value = coercePropertyValue(record, value);
298332
}
299333
}

www/__tests__/site-ui.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ Deno.test('open-layout is an explicitly hydrated compiled app-shell island', asy
3333
// Regions: header nav (desktop + mobile panel), sidebar rows (desktop +
3434
// mobile disclosure panel) and the four footer link columns.
3535
assertEquals(result.program.regions.length, 8);
36+
// Injected shell props plus the derived chrome state as computed signal
37+
// properties (the list-Region grammar requires `.map()` over
38+
// `this.<property>`, so the derived sidebar rows / footer columns are
39+
// compiled computed fields rather than render locals or accessors).
3640
assertEquals(
3741
result.program.metadata.properties.map((property) => property.name),
3842
[

www/app/islands/open-layout.tsx

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,7 +447,12 @@ export default class OpenLayout extends OpenElement {
447447
@property({ reflect: false })
448448
currentPath = '';
449449

450+
// Re-declares the optional base field (OpenElementConfiguration.locale) as a
451+
// compiled @property. The `override` modifier that tsc would demand here is
452+
// rejected by the compiled-element grammar (OEC9005 — property fields must be
453+
// ordinary initialized fields), so the TS diagnostic is expected.
450454
@property({ reflect: false })
455+
// @ts-expect-error compiled @property shadows the optional base field
451456
locale = 'en';
452457

453458
@property({ reflect: false })
@@ -456,6 +461,15 @@ export default class OpenLayout extends OpenElement {
456461
@property({ reflect: true })
457462
home = false;
458463

464+
// ─── Derived chrome state ────────────────────────────────────────────
465+
// The list-Region grammar (OEC9013) requires `.map()` to run over a
466+
// `this.<property>`, so the derived rows/links live in computed() signal
467+
// properties over the plain shell props. The property-contract layer
468+
// guarantees attribute promotion happens before claim-time reads and that
469+
// generated field initializers (which only restate the compiled default)
470+
// never clobber the promoted attribute values — see facade-host.ts
471+
// restatesDefault.
472+
459473
@property({ reflect: false, attribute: false })
460474
headerNavItems = computed(() =>
461475
decorateHeaderNav(this.headerNav, this.currentPath, this.locale, this.locales)

0 commit comments

Comments
 (0)