test(ui): package-level lifecycle + multi-instance + reconnect semantics (#1227, B2.2) - #1299
Merged
Merged
Conversation
…ics (#1227, B2.2) Close independent-audit findings L4/L5 for @openelement/ui: - L4 theme-toggle: per-instance priority-chain resolution, document-channel convergence between instances, dual-state transitions on all three channels (host attribute, document data-theme, colorScheme), no-op re-apply, reconnect exactly-once (initDone guard), teardown-free dispose with fresh replacement init, attribute-change-never-persists (#804 complement). - Dispose/reconnect pins for dialog (open-effect teardown + top-layer re-entry per session), tabs (click wiring once per tab across reconnect, stale-ARIA re-sync), dropdown (focus-return wiring once, stable anchor name), input (activation id stability, open-focus/open-blur events). - L5: manifest attribute/event contract test (WC Package Protocol surface); shared fake-DOM harness extracted to __tests__/harness.ts. - Documented not-unit-observable semantics (computed reactivity, ElementInternals channels, connectedCallback scheduling) with pointers to the ui-dogfood + www browser evidence instead of fake-passing unit tests.
Contributor
|
APIError: Insufficient Balance |
This was referenced Sep 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
#1227 (stage #1288, Beta.2 / B2.2): re-verify and close two independent-audit findings against
@openelement/ui:packages/ui/__tests__did not cover several package exports' real semantics, and there was no exports-vs-tests accounting.B2.1 (#1226, PR #1298) added browser-level qualification (ui-dogfood fixture, 3 engines). This PR adds the package-level semantics layer only — no browser coverage is duplicated; where unit-level cannot observe a semantic, the test file says so and points at the browser evidence instead of fake-passing.
Owner
Beta.2 B2.2 (#1227). One bounded slice:
packages/ui/__tests__only. No production code changed.Before — exports vs package-level tests
openPropsTokenSheet/openPropsRootSheetmanifest/registerOpenUiOpenButtonOpenInputOpenCodeBlockOpenBadgeOpenCardOpenCalloutOpenThemeToggleOpenDialogOpenDropdownOpenTabsAfter — what was added
New
packages/ui/__tests__/lifecycle.test.ts(17 tests) + 1 manifest-contract test incomponents.test.ts; the fake-DOM harness was extracted topackages/ui/__tests__/harness.ts(shared, idempotent install) instead of being duplicated.L4 — open-theme-toggle semantics now pinned (8 tests):
initTheme's priority chain:applyThemewritesdocumentElement[data-theme], which the next instance's chain reads).data-themeattribute,documentElement[data-theme],style.colorScheme; exactly oneopen:theme-changeper actual change.old === val, repeatapplyTheme) dispatches nothing and persists nothing.initThemeruns its body exactly once per instance — repeated lifecycle scheduling (connect rAF + DSD rAF + reconnect) performs zero re-resolution, zero document rewrites, zero re-dispatch, zero storage writes, even when every resolution source changed while detached.lightvalues resolve todark.Dispose/reconnect pins for the other primitives:
open-dialog: dispose runs the compiled-effect teardown exactly once and clears the instance-state slot; double-dispose is safe; reconnect re-subscribes. Each open session enters the top layer exactly once (modalActiveguard) and reopen after close re-enters exactly once.open-tabs: activation decorates the full WAI-ARIA wiring (role/aria-selected/aria-controls/aria-labelledby/tabindex/hidden pairing); click wiring attaches exactly once per tab across dispose→reconnect; reconnect re-syncs stale ARIA (unit-level analog of the ui-dogfood browser test); two instances get independent id prefixes and selection state.open-dropdown: focus-return listeners (focusin/toggle) attach exactly once across dispose→reconnect; the realm-unique anchor name is never re-assigned.open-input: activation assigns the realm-unique control id once — reconnect keeps it;open-focus/open-blurdispatch composed+bubbling.instance-state: the shared per-host store itself is pinned (initializer once per host, zero cross-host leakage) — the foundation every lifecycle guard builds on.L5 — package contract: the generated manifest's per-declaration attribute and event names are now asserted against the published contract (the WC Package Protocol surface adapter-vite compiles from); drift fails the suite.
Deliberately NOT unit-tested (would fake-pass), with evidence pointers:
computed()reactivity (open-callout icon fallback, open-inputinputClass, open-dropdownanchorStyle): uncompiled,@propertyfields are plain values so computeds snapshot the initializer and never re-derive — verified by probe. Compiled-sink reactivity is covered by the element package's compiled facade/claim suites.setFormValue/setValidity/:state):_internalsis a kernel-owned getter (open-element-implementation.ts:232) with no kernel attached in Deno tests. Browser evidence: ui-dogfoodui-form.spec.ts(FormData, valueMissing, fieldset disable/enable),ui-dialog.spec.ts(:state(open/closed)).connectedCallback → rAF → initThemescheduling:_requestAnimationFrameis kernel/detached-lifecycle owned. Browser evidence: wwwe2e/theme-system.spec.ts(init priority, toggle, persistence, multiple toggles) and ui-dogfoodui-tabs.spec.ts(disconnect→reconnect effect re-sync on the real claim path).CI gating: package-level tests already run in CI — the root
deno testdiscoverspackages/ui/__tests__(verified: 4 test files + harness type-checked viadeno test --no-run), and the AutoFlowtest:coverage:checkgate (ci + release tiers, triggers onpackages/**) runs the full root test task. No wiring change needed.Why-not-second-owner
Test-only change inside
packages/ui/__tests__. The harness extraction, the new lifecycle suite and the manifest contract test are one coherent unit — splitting them would land either half unverifiable (the new suite needs the shared harness; the harness refactor needs the suite to justify it). No ADR-0122 frozen paths touched; no production code touched.Evidence
cd packages/ui && deno task test→ 47 passed, 0 failed (was 29) — exit 0deno task test→ 1810 passed (105 steps) + 150 passed, 0 failed — exit 0deno task pack:dry-run→ exit 0;deno task package-artifacts:check→ exit 0 (5 packages)deno lint src/ __tests__/+deno fmt --check(packages/ui) → cleandeno test --no-runlists all 4packages/ui/__tests__test filesgit statusshows only__tests__changes):open-theme-toggle.tsxinitDone guard removed →reconnect re-runs initTheme zero times…FAILED ✔open-dialog.tsxmodalActivereset removed →each open session enters the top layer exactly once; reopen re-entersFAILED ✔open-tabs.tsxwiredTabsguard removed →click wiring attaches once per tab across dispose/reconnect…FAILED ✔Scope
In:
packages/ui/__tests__/{harness.ts,lifecycle.test.ts}(new),components.test.ts(harness refactor + manifest contract test). Out: production code (zero diff), browser-level fixtures (B2.1 owns them), new components, Beta.3 formal UI qualification breadth.Risk
Minimal: additive test files + a mechanical harness extraction from an existing green suite. The harness
CustomEventstub now forwardsEventInit(bubbles/composed) — required for the focus/blur assertions; existing theme tests only readdetailand are unaffected (all 21 pre-existing component tests still green).Closes #1227