From db6e80e519b2a601cebd5d25bebf4cfb5c75a3cf Mon Sep 17 00:00:00 2001 From: David Di Biase <1168397+davedbase@users.noreply.github.com> Date: Sat, 18 Jul 2026 15:30:28 -0400 Subject: [PATCH 1/4] Applied adjustments and tests to improve hydration concerns --- .gitignore | 1 + CONTRIBUTING.md | 13 ++ packages/a11y/src/form-control.ts | 8 +- packages/a11y/test/hydration.test.tsx | 59 +++++ .../controlled-signal/test/hydration.test.tsx | 50 ++++ packages/focus/README.md | 30 +++ packages/focus/src/focusRestore.ts | 74 ++++++ packages/focus/src/focusTrap.ts | 28 ++- packages/focus/src/index.ts | 2 + packages/focus/src/restoreFocus.ts | 48 ++++ packages/focus/test/index.test.tsx | 213 ++++++++++++++++-- packages/interaction/src/index.ts | 108 +++++++-- .../interaction/test/hide-outside.test.ts | 76 ++++++- packages/pagination/test/hydration.test.tsx | 50 ++++ packages/scroll/src/preventScroll.ts | 66 +++--- .../state-machine/test/hydration.test.tsx | 54 +++++ packages/utils/src/index.ts | 25 +- packages/utils/test/index.test.ts | 36 ++- scripts/test-utils/hydration-harness.ts | 205 +++++++++++++++++ 19 files changed, 1047 insertions(+), 99 deletions(-) create mode 100644 packages/a11y/test/hydration.test.tsx create mode 100644 packages/controlled-signal/test/hydration.test.tsx create mode 100644 packages/focus/src/focusRestore.ts create mode 100644 packages/focus/src/restoreFocus.ts create mode 100644 packages/pagination/test/hydration.test.tsx create mode 100644 packages/state-machine/test/hydration.test.tsx create mode 100644 scripts/test-utils/hydration-harness.ts diff --git a/.gitignore b/.gitignore index 82c77b3bc..b8260cf90 100644 --- a/.gitignore +++ b/.gitignore @@ -129,6 +129,7 @@ pages # temp _temp_* +.hydration-harness-* # Local Netlify folder .netlify diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d8bc7c92c..ede83dc22 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -51,6 +51,19 @@ Solid uses the `create` prefix to define a primitive that provides reactive util Solid Primitives is mostly about supplying 80-90% of the common-use cases for the end-user. We prefer to be less prescriptive than other hook libraries such as VueUse and supply granular solutions as opposed to monolithic primitives. The remaining 10-20% of complex use cases are likely not to be covered with this library. This is on purpose to limit the potential of bloat and extended complexity. This project strives to provide foundations and not cumulative solutions. We expect the broader ecosystem will fill the remaining need as further composition to this projects effort. This allows for just the right amount of prescription and opinion. +### Hydration Safety + +A primitive that creates a **compute-form** `createSignal(fn)` or `createMemo(fn)` in a component's render body participates in Solid's per-owner hydration id allocation, the same as a DOM element the JSX compiler stamps a hydration key onto. Most primitives that return a derived reactive value (`createPagination`, `createHideOutside`'s internals, anything backed by `createMemo`) fall into this category — it's the norm, not the exception, and on its own isn't a defect. Classify each package's primitives as one of: + +- **Effect-only** — no render-body `createSignal(fn)`/`createMemo`, only `createEffect`/`createSignal(value)`/plain functions. Always hydration-safe. +- **Creates a render-body compute signal/memo** — returns state derived via `createSignal(fn)` or `createMemo`. Verify it with a real hydration round trip (see below) rather than assuming it's fine. + +Prefer a plain (non-memoized) getter function over `createMemo` when the derived value is cheap to recompute (a handful of object fields, a short string) — it avoids consuming a hydration id for no measurable perf benefit. Reach for `createMemo` when the computation is genuinely non-trivial. + +Use [`scripts/test-utils/hydration-harness.ts`](scripts/test-utils/hydration-harness.ts)'s `renderHydrationRoundTrip()` to verify a package end-to-end: it renders a small fixture component through a real `node` subprocess (so `@solid-primitives/*` and `@solidjs/web` resolve to their published `dist` builds, exactly as an installed consumer gets them — not this monorepo's `@solid-primitives/source` workspace alias that regular `test/server.test.ts` files resolve through), then hydrates the result in-process and asserts no console error/warning. See `packages/controlled-signal/test/hydration.test.tsx` and `packages/a11y/test/hydration.test.tsx` for examples. Add one of these for any primitive whose public API returns a `createMemo`/compute-form-`createSignal` value that a consumer is expected to read directly in a render body. + +**Rebuild before trusting a result.** The harness's server half resolves the target package's published `dist`, but its client half resolves workspace `src` (through this monorepo's own alias). If you edit a package's `src` without rebuilding `dist`, the two halves are running genuinely different code, which can desync owner-id allocation for real and produce a false-positive "unclaimed server-rendered node" warning that has nothing to do with the hazard under test. Run `pnpm -w build` (or scope it to the packages you touched) before running a hydration test that depends on your changes. + ## NPM Release and Repository Structure Solid Primitives is a large and growing project and the way we manage and release updates has been setup to suit the projects scope. The approach we've taken with organizing our packages and npm releases is more granular than other projects such as VueUse which ship all updates in a single release. diff --git a/packages/a11y/src/form-control.ts b/packages/a11y/src/form-control.ts index d58607bb3..b5a7a73e4 100644 --- a/packages/a11y/src/form-control.ts +++ b/packages/a11y/src/form-control.ts @@ -16,7 +16,6 @@ import type { Context } from "solid-js"; import { createContext, createEffect, - createMemo, createSignal, createUniqueId, useContext, @@ -140,13 +139,16 @@ export function createFormControl(props: CreateFormControlProps): FormControlCon ); }; - const dataset = createMemo(() => ({ + // Plain getter, not createMemo: the object is cheap to rebuild on every read, and a + // render-body compute-form memo would consume a hydration id in every consuming app — + // see the transform-boundary hydration hazard this pattern is designed to avoid. + const dataset = (): FormControlDataSet => ({ "data-valid": access(props.validationState) === "valid" ? "" : undefined, "data-invalid": access(props.validationState) === "invalid" ? "" : undefined, "data-required": access(props.required) ? "" : undefined, "data-disabled": access(props.disabled) ? "" : undefined, "data-readonly": access(props.readOnly) ? "" : undefined, - })); + }); return { name: () => access(props.name) ?? id(), diff --git a/packages/a11y/test/hydration.test.tsx b/packages/a11y/test/hydration.test.tsx new file mode 100644 index 000000000..5e9392dba --- /dev/null +++ b/packages/a11y/test/hydration.test.tsx @@ -0,0 +1,59 @@ +import { describe, it, expect, afterEach } from "vitest"; +import { + renderHydrationRoundTrip, + type HydrationRoundTripResult, +} from "../../../scripts/test-utils/hydration-harness.ts"; + +// createFormControl backs its labelId/fieldId/descriptionId/errorMessageId with +// createSignal(undefined, { ownedWrite: true }) internal signals created in the render body — +// the same class of construct hope-ui's solid-primitives-eval.md flags as a hydration-id hazard. +// Renders through the real, standard @solidjs/web renderToString + hydrate pipeline (not our own +// SSR harness) with the package resolved from its published `dist` build. +const APP_SOURCE = ` +import { createFormControl, FormControlContext } from "@solid-primitives/a11y"; + +export default function App() { + const ctx = createFormControl({ id: "email", required: true }); + ctx.registerLabel("email-label"); + + return ( + +
+ + +
+ + ); +} +`; + +describe("createFormControl hydration round trip", () => { + let result: HydrationRoundTripResult | undefined; + + afterEach(() => { + result?.cleanup(); + result = undefined; + }); + + it("server-renders the required dataset attribute", async () => { + result = await renderHydrationRoundTrip(APP_SOURCE, import.meta.dirname); + expect(result.html).toContain('data-required=""'); + }); + + it("hydrates without any console error or warning", async () => { + result = await renderHydrationRoundTrip(APP_SOURCE, import.meta.dirname); + expect(result.consoleMessages).toEqual([]); + }); + + it("hydrates the DOM with the correct aria attributes", async () => { + result = await renderHydrationRoundTrip(APP_SOURCE, import.meta.dirname); + const input = result.container.querySelector("#email-input"); + expect(input?.getAttribute("aria-labelledby")).toBe("email-label"); + expect(input?.getAttribute("data-required")).toBe(""); + }); +}); diff --git a/packages/controlled-signal/test/hydration.test.tsx b/packages/controlled-signal/test/hydration.test.tsx new file mode 100644 index 000000000..a46774eda --- /dev/null +++ b/packages/controlled-signal/test/hydration.test.tsx @@ -0,0 +1,50 @@ +import { describe, it, expect, afterEach } from "vitest"; +import { + renderHydrationRoundTrip, + type HydrationRoundTripResult, +} from "../../../scripts/test-utils/hydration-harness.ts"; + +// `createControllableSignal` backs its state with a compute-form `createSignal(fn, { ownedWrite: true })` +// created in the component's render body — exactly the pattern hope-ui's solid-primitives-eval.md flags +// as a hydration-id hazard when the primitive lives in an un-transformed node_modules dependency. This +// renders through the real, standard @solidjs/web renderToString + hydrate pipeline (not our own SSR +// harness) with the package resolved from its published `dist` build, to check whether that hazard +// actually reproduces here. +const APP_SOURCE = ` +import { createControllableSignal } from "@solid-primitives/controlled-signal"; + +export default function App() { + const [value] = createControllableSignal({ defaultValue: () => "hello" }); + return ( +
+ {value()} + +
+ ); +} +`; + +describe("createControllableSignal hydration round trip", () => { + let result: HydrationRoundTripResult | undefined; + + afterEach(() => { + result?.cleanup(); + result = undefined; + }); + + it("server-renders the defaultValue", async () => { + result = await renderHydrationRoundTrip(APP_SOURCE, import.meta.dirname); + expect(result.html).toContain("hello"); + }); + + it("hydrates without any console error or warning", async () => { + result = await renderHydrationRoundTrip(APP_SOURCE, import.meta.dirname); + expect(result.consoleMessages).toEqual([]); + }); + + it("hydrates the DOM into an interactive tree with the correct value", async () => { + result = await renderHydrationRoundTrip(APP_SOURCE, import.meta.dirname); + const span = result.container.querySelector("span"); + expect(span?.textContent).toBe("hello"); + }); +}); diff --git a/packages/focus/README.md b/packages/focus/README.md index 194c2bf74..2cff8a32d 100644 --- a/packages/focus/README.md +++ b/packages/focus/README.md @@ -16,6 +16,7 @@ The native `autofocus` attribute only works on page load, which makes it incompa - [`autofocus`](#autofocus) - Ref callback factory to autofocus an element on render. - [`createAutofocus`](#createautofocus) - Reactive primitive to autofocus an element on render. - [`createFocusTrap`](#createfocustrap) - Traps focus inside a given DOM element. +- [`createFocusRestore`](#createfocusrestore) - Restores focus to the previously focused element, without trapping. ## Installation @@ -151,6 +152,35 @@ createFocusTrap({ }); ``` +## `createFocusRestore` + +`createFocusRestore` saves the currently focused element while active and restores focus to it once deactivated — without trapping focus or managing tab order. Use it for non-modal surfaces (Popover, Tooltip, Menu) that should return focus to their trigger on close but must not intercept Tab navigation while open. For modal dialogs that need both behaviors, use [`createFocusTrap`](#createfocustrap)'s `restoreFocus` option instead. + +### How to use it + +```tsx +import { createFocusRestore } from "@solid-primitives/focus"; + +const Popover: Component<{ open: boolean }> = props => { + createFocusRestore({ enabled: () => props.open }); + + return ( + +
...
+ + ); +}; +``` + +### Props + +| Prop | Type | Default | Description | +| ------------------- | ----------------------------------- | -------------------------- | ------------------------------------------------------------------------ | +| `enabled` | `MaybeAccessor` | `true` | Whether focus-restore is active. | +| `element` | `MaybeAccessor` | `document.body` | Element to dispatch the `onFinalFocus` event on. | +| `finalFocusElement` | `MaybeAccessor` | Previously focused element | Element to focus when deactivated. | +| `onFinalFocus` | `(event: Event) => void` | — | Callback when focus restores. Call `event.preventDefault()` to cancel. | + ## Credits `createFocusTrap` is ported from [solid-focus-trap](https://github.com/corvudev/corvu/tree/main/packages/solid-focus-trap), part of the [corvu](https://corvu.dev) UI toolkit by [Jasmin Noetzli (GiyoMoon)](https://github.com/GiyoMoon). Licensed under the MIT License. diff --git a/packages/focus/src/focusRestore.ts b/packages/focus/src/focusRestore.ts new file mode 100644 index 000000000..68aea5ecc --- /dev/null +++ b/packages/focus/src/focusRestore.ts @@ -0,0 +1,74 @@ +/* + * Adapted for @solid-primitives/focus by the Solid Primitives Working Group. + * Reuses the restore-focus half of createFocusTrap (see focusTrap.ts) as a + * standalone primitive for non-modal surfaces (Popover, Tooltip) that must + * return focus to their trigger on close but must not intercept Tab + * navigation the way a full focus trap does. + */ + +import { access, type MaybeAccessor } from "@solid-primitives/utils"; +import { createEffect } from "solid-js"; +import { scheduleFocusRestore } from "./restoreFocus.ts"; + +const EVENT_FINAL_FOCUS = "focusRestore.finalFocus"; + +export type CreateFocusRestoreProps = { + /** Whether focus-restore is active. Default: `true` */ + enabled?: MaybeAccessor; + /** + * Element to dispatch the `onFinalFocus` event on. Defaults to `document.body`. + */ + element?: MaybeAccessor; + /** + * Element to focus when deactivated. + * Default: the element that was focused when activated. + */ + finalFocusElement?: MaybeAccessor; + /** + * Callback fired when focus is restored. + * Call `event.preventDefault()` to suppress the focus move. + */ + onFinalFocus?: (event: Event) => void; +}; + +/** + * Saves the currently focused element while active and restores focus to it + * once deactivated — without trapping focus or managing tab order. + * + * For a non-modal Popover/Tooltip/Menu that should return focus to its trigger + * on close but must not intercept Tab navigation while open. For the full + * trap-and-restore behavior (modal dialogs) use {@link createFocusTrap}, whose + * `restoreFocus` option covers the same restore behavior alongside trapping. + * + * @example + * ```tsx + * const [ref, setRef] = createSignal(); + * createFocusRestore({ enabled: () => isOpen(), element: ref }); + *
...
+ * ``` + */ +export const createFocusRestore = (props: CreateFocusRestoreProps = {}): void => { + let originalFocusedElement: HTMLElement | null = null; + + createEffect( + () => access(props.enabled ?? true), + enabled => { + if (!enabled) return; + + originalFocusedElement = document.activeElement as HTMLElement | null; + + return () => { + scheduleFocusRestore({ + resolveEventTarget: () => access(props.element) ?? document.body, + eventName: EVENT_FINAL_FOCUS, + resolveTarget: () => access(props.finalFocusElement) ?? null, + // Snapshot now, synchronously — the restore is scheduled via afterPaint (deferred by a + // couple of animation frames), and a later activation before it fires would otherwise + // overwrite originalFocusedElement out from under the still-pending restore. + fallbackTarget: originalFocusedElement, + onFinalFocus: props.onFinalFocus, + }); + }; + }, + ); +}; diff --git a/packages/focus/src/focusTrap.ts b/packages/focus/src/focusTrap.ts index 06391279e..22b40e76a 100644 --- a/packages/focus/src/focusTrap.ts +++ b/packages/focus/src/focusTrap.ts @@ -6,6 +6,7 @@ import { access, afterPaint, INTERNAL_OPTIONS, type MaybeAccessor } from "@solid-primitives/utils"; import { createEffect, createMemo, createSignal } from "solid-js"; +import { scheduleFocusRestore } from "./restoreFocus.ts"; const FOCUSABLE_SELECTOR = 'a[href]:not([tabindex="-1"]), button:not([tabindex="-1"]), input:not([tabindex="-1"]), textarea:not([tabindex="-1"]), select:not([tabindex="-1"]), details:not([tabindex="-1"]), [tabindex]:not([tabindex="-1"])'; @@ -107,20 +108,14 @@ export const createFocusTrap = (props: CreateFocusTrapProps): void => { }); }; - const triggerRestoreFocus = (container: HTMLElement) => { - afterPaint(() => { - if (!access(props.restoreFocus ?? true)) return; - const target = access(props.finalFocusElement ?? null) ?? originalFocusedElement; - if (!target) return; - const { onFinalFocus } = props; - if (onFinalFocus) { - const event = new CustomEvent(EVENT_FINAL_FOCUS, EVENT_OPTIONS); - container.addEventListener(EVENT_FINAL_FOCUS, onFinalFocus); - container.dispatchEvent(event); - container.removeEventListener(EVENT_FINAL_FOCUS, onFinalFocus); - if (event.defaultPrevented) return; - } - target.focus(); + const triggerRestoreFocus = (container: HTMLElement, fallbackTarget: HTMLElement | null) => { + scheduleFocusRestore({ + resolveEventTarget: () => container, + eventName: EVENT_FINAL_FOCUS, + shouldRestore: () => access(props.restoreFocus ?? true), + resolveTarget: () => access(props.finalFocusElement) ?? null, + fallbackTarget, + onFinalFocus: props.onFinalFocus, }); }; @@ -177,7 +172,10 @@ export const createFocusTrap = (props: CreateFocusTrapProps): void => { return () => { if (observeChanges) observer.disconnect(); setFocusableElements(undefined); - triggerRestoreFocus(container); + // Snapshot now, synchronously — the restore is scheduled via afterPaint (deferred by a + // couple of animation frames), and a later activation before it fires would otherwise + // overwrite originalFocusedElement out from under the still-pending restore. + triggerRestoreFocus(container, originalFocusedElement); }; }, ); diff --git a/packages/focus/src/index.ts b/packages/focus/src/index.ts index 3c4f9b9d4..a9f0aa245 100644 --- a/packages/focus/src/index.ts +++ b/packages/focus/src/index.ts @@ -2,4 +2,6 @@ export { autofocus, createAutofocus } from "./autofocus.ts"; export type { E } from "./autofocus.ts"; export { createFocusTrap } from "./focusTrap.ts"; export type { CreateFocusTrapProps } from "./focusTrap.ts"; +export { createFocusRestore } from "./focusRestore.ts"; +export type { CreateFocusRestoreProps } from "./focusRestore.ts"; export { makeFocusListener, createFocusSignal } from "./focusSignal.ts"; diff --git a/packages/focus/src/restoreFocus.ts b/packages/focus/src/restoreFocus.ts new file mode 100644 index 000000000..c466f86b3 --- /dev/null +++ b/packages/focus/src/restoreFocus.ts @@ -0,0 +1,48 @@ +/** + * Shared "restore focus on deactivation" ceremony used by both `createFocusTrap` (trap + restore) + * and the standalone `createFocusRestore` (restore only, no trap). + */ + +import { afterPaint } from "@solid-primitives/utils"; + +const EVENT_OPTIONS = { bubbles: false, cancelable: true } as const; + +export type ScheduleFocusRestoreOptions = { + /** Element to dispatch the cancelable `onFinalFocus` custom event on. Read live (at fire time). */ + resolveEventTarget: () => Element; + eventName: string; + /** Read live (at fire time). Return `false` to abort the restore for this cycle. */ + shouldRestore?: () => boolean; + /** Live override target (e.g. an explicit `finalFocusElement` option); wins over `fallbackTarget`. */ + resolveTarget: () => HTMLElement | null; + /** + * The element to fall back to when `resolveTarget()` returns `null` — must be captured by the + * caller *synchronously*, at the moment deactivation begins (before scheduling this), not read + * lazily from a mutable outer variable. Otherwise a later activation cycle can reassign that + * variable before this runs, restoring focus to the wrong element. + */ + fallbackTarget: HTMLElement | null; + onFinalFocus?: (event: Event) => void; +}; + +/** + * Schedules (via `afterPaint`) a cancelable-custom-event-gated focus restore. + */ +export function scheduleFocusRestore(options: ScheduleFocusRestoreOptions): void { + const { resolveEventTarget, eventName, shouldRestore, resolveTarget, fallbackTarget, onFinalFocus } = + options; + afterPaint(() => { + if (shouldRestore && !shouldRestore()) return; + const target = resolveTarget() ?? fallbackTarget; + if (!target) return; + if (onFinalFocus) { + const eventTarget = resolveEventTarget(); + const event = new CustomEvent(eventName, EVENT_OPTIONS); + eventTarget.addEventListener(eventName, onFinalFocus); + eventTarget.dispatchEvent(event); + eventTarget.removeEventListener(eventName, onFinalFocus); + if (event.defaultPrevented) return; + } + target.focus(); + }); +} diff --git a/packages/focus/test/index.test.tsx b/packages/focus/test/index.test.tsx index d1c8aa578..5a1f7b26c 100644 --- a/packages/focus/test/index.test.tsx +++ b/packages/focus/test/index.test.tsx @@ -1,8 +1,6 @@ import { describe, test, expect, vi, beforeEach, afterAll, beforeAll } from "vitest"; import { createRoot, createSignal, flush } from "solid-js"; -import { autofocus, createAutofocus, createFocusTrap } from "../src/index.js"; - -// ─── Shared focus tracking ──────────────────────────────────────────────────── +import { autofocus, createAutofocus, createFocusTrap, createFocusRestore } from "../src/index.js"; let focused: HTMLElement | null = null; @@ -11,8 +9,6 @@ HTMLElement.prototype.focus = function (this: HTMLElement) { focused = this; }; -// ─── Fake timers + rAF stub ─────────────────────────────────────────────────── - beforeAll(() => { vi.useFakeTimers(); // afterPaint uses double rAF; stub it as setTimeout so vi.runAllTimers() drives it. @@ -33,16 +29,12 @@ afterAll(() => { HTMLElement.prototype.focus = original_focus; }); -// ─── Helper ─────────────────────────────────────────────────────────────────── - /** Run all pending effects then drain all timers (including nested rAFs). */ const settle = () => { flush(); vi.runAllTimers(); }; -// ─── autofocus ──────────────────────────────────────────────────────────────── - describe("autofocus", () => { test("focuses the element with autofocus attribute", () => { const el = document.createElement("button"); @@ -74,8 +66,6 @@ describe("autofocus", () => { }); }); -// ─── createAutofocus ────────────────────────────────────────────────────────── - describe("createAutofocus", () => { const el = document.createElement("button"), el2 = document.createElement("button"); @@ -118,8 +108,6 @@ describe("createAutofocus", () => { }); }); -// ─── createFocusTrap ────────────────────────────────────────────────────────── - /** Build a container with `n` focusable buttons and return them. */ function makeContainer(n: number): { container: HTMLElement; buttons: HTMLButtonElement[] } { const container = document.createElement("div"); @@ -436,3 +424,202 @@ describe("createFocusTrap", () => { dispose(); }); }); + +describe("createFocusRestore", () => { + test("does not move focus on activation", () => { + const trigger = document.createElement("button"); + document.body.appendChild(trigger); + trigger.focus(); + focused = null; // reset after the manual .focus() above + + const dispose = createRoot(dispose => { + createFocusRestore({ enabled: true }); + return dispose; + }); + + settle(); + expect(focused).toBe(null); // createFocusRestore never focuses anything itself + dispose(); + trigger.remove(); + }); + + test("restores focus to the previously focused element on deactivation", () => { + const trigger = document.createElement("button"); + const [enabled, setEnabled] = createSignal(true); + + const origActiveElement = Object.getOwnPropertyDescriptor(Document.prototype, "activeElement")!; + Object.defineProperty(document, "activeElement", { + get: () => trigger, + configurable: true, + }); + + const dispose = createRoot(dispose => { + createFocusRestore({ enabled }); + return dispose; + }); + + settle(); + Object.defineProperty(document, "activeElement", origActiveElement); + + setEnabled(false); + settle(); + expect(focused).toBe(trigger); + dispose(); + }); + + test("a reactivation before a pending restore fires does not corrupt that restore's target", () => { + // Regression test: the restore target used to be read lazily from a shared outer variable + // inside the afterPaint-deferred callback. If something reactivated focus-restore (capturing + // a *new* "currently focused" element) before the still-pending restore from the *previous* + // deactivation had a chance to fire, that pending restore would incorrectly pick up the new + // value instead of the one that was current when it was scheduled. + const trigger1 = document.createElement("button"); + const trigger2 = document.createElement("button"); + const [enabled, setEnabled] = createSignal(true); + + const origActiveElement = Object.getOwnPropertyDescriptor(Document.prototype, "activeElement")!; + Object.defineProperty(document, "activeElement", { get: () => trigger1, configurable: true }); + + const dispose = createRoot(dispose => { + createFocusRestore({ enabled }); + return dispose; + }); + + settle(); // originalFocusedElement captured as trigger1 + + setEnabled(false); // schedules a restore that should target trigger1 + flush(); // runs the deactivation synchronously — the afterPaint restore is now pending, not yet fired + + // Before that pending restore fires, something reactivates focus-restore against a + // different currently-focused element, and stays enabled (no second deactivation, so no + // second restore gets queued — isolates exactly the race being tested). + Object.defineProperty(document, "activeElement", { get: () => trigger2, configurable: true }); + setEnabled(true); + flush(); + + Object.defineProperty(document, "activeElement", origActiveElement); + + vi.runAllTimers(); // drains the still-pending first restore + expect(focused).toBe(trigger1); // not trigger2 + dispose(); + }); + + test("restores focus on dispose even without enabled changing first", () => { + const trigger = document.createElement("button"); + + const origActiveElement = Object.getOwnPropertyDescriptor(Document.prototype, "activeElement")!; + Object.defineProperty(document, "activeElement", { + get: () => trigger, + configurable: true, + }); + + const dispose = createRoot(dispose => { + createFocusRestore(); + return dispose; + }); + + settle(); + Object.defineProperty(document, "activeElement", origActiveElement); + + dispose(); + settle(); + expect(focused).toBe(trigger); + }); + + test("uses finalFocusElement when provided", () => { + const trigger = document.createElement("button"); + const customFinal = document.createElement("button"); + const [enabled, setEnabled] = createSignal(true); + + const origActiveElement = Object.getOwnPropertyDescriptor(Document.prototype, "activeElement")!; + Object.defineProperty(document, "activeElement", { + get: () => trigger, + configurable: true, + }); + + const dispose = createRoot(dispose => { + createFocusRestore({ enabled, finalFocusElement: customFinal }); + return dispose; + }); + + settle(); + Object.defineProperty(document, "activeElement", origActiveElement); + + setEnabled(false); + settle(); + expect(focused).toBe(customFinal); + dispose(); + }); + + test("onFinalFocus callback is called on deactivation", () => { + const trigger = document.createElement("button"); + const [enabled, setEnabled] = createSignal(true); + const onFinalFocus = vi.fn(); + + const origActiveElement = Object.getOwnPropertyDescriptor(Document.prototype, "activeElement")!; + Object.defineProperty(document, "activeElement", { + get: () => trigger, + configurable: true, + }); + + const dispose = createRoot(dispose => { + createFocusRestore({ enabled, onFinalFocus }); + return dispose; + }); + + settle(); + Object.defineProperty(document, "activeElement", origActiveElement); + + setEnabled(false); + settle(); + expect(onFinalFocus).toHaveBeenCalledOnce(); + dispose(); + }); + + test("onFinalFocus preventDefault suppresses focus restore", () => { + const trigger = document.createElement("button"); + const [enabled, setEnabled] = createSignal(true); + + const origActiveElement = Object.getOwnPropertyDescriptor(Document.prototype, "activeElement")!; + Object.defineProperty(document, "activeElement", { + get: () => trigger, + configurable: true, + }); + + const dispose = createRoot(dispose => { + createFocusRestore({ enabled, onFinalFocus: e => e.preventDefault() }); + return dispose; + }); + + settle(); + Object.defineProperty(document, "activeElement", origActiveElement); + + focused = null; + setEnabled(false); + settle(); + expect(focused).toBe(null); // prevented + dispose(); + }); + + test("does not restore focus when enabled is false throughout", () => { + const trigger = document.createElement("button"); + + const origActiveElement = Object.getOwnPropertyDescriptor(Document.prototype, "activeElement")!; + Object.defineProperty(document, "activeElement", { + get: () => trigger, + configurable: true, + }); + + const dispose = createRoot(dispose => { + createFocusRestore({ enabled: false }); + return dispose; + }); + + settle(); + Object.defineProperty(document, "activeElement", origActiveElement); + + dispose(); + settle(); + expect(focused).toBe(null); + }); +}); diff --git a/packages/interaction/src/index.ts b/packages/interaction/src/index.ts index e2901e3d9..baa26fbdf 100644 --- a/packages/interaction/src/index.ts +++ b/packages/interaction/src/index.ts @@ -21,7 +21,7 @@ */ import { type Accessor, createEffect, onCleanup } from "solid-js"; -import { type MaybeAccessor, access, isServer, noop } from "@solid-primitives/utils"; +import { type MaybeAccessor, access, globalRegistry, isServer, noop } from "@solid-primitives/utils"; // ---- ariaHideOutside / createHideOutside ---- @@ -40,11 +40,64 @@ export interface CreateHideOutsideOptions { * target containment (e.g. top-layer elements, live announcers). Optional. */ alwaysVisibleSelector?: string; + /** + * Also set the `inert` attribute on hidden elements, in addition to `aria-hidden`. + * `aria-hidden` alone only affects the accessibility tree — the background stays focusable + * and clickable; `inert` additionally removes it from focus order, tab order, and pointer/ + * text-selection interaction. *Default = `false`* (matches prior behavior). + */ + inert?: MaybeAccessor; } -// Keeps a ref count of all hidden elements so nested usages don't fight. -const refCountMap = new WeakMap(); -const observerStack: Array<{ observe(): void; disconnect(): void }> = []; +// Keeps a ref count of all hidden/inerted elements so nested usages don't fight, and tracks +// the stack of active observers. Uses globalRegistry (Symbol.for(...) on globalThis, not +// module-scope bindings) so this stays correct even if the app's dependency graph ends up with +// more than one copy of this package installed — module-scope state would otherwise be split +// across copies (e.g. one copy's cleanup revealing content another copy still needs hidden). +type HideOutsideRegistry = { + refCountMap: WeakMap; + inertRefCountMap: WeakMap; + observerStack: Array<{ observe(): void; disconnect(): void }>; +}; + +const getHideOutsideRegistry = (): HideOutsideRegistry => + globalRegistry("@solid-primitives/interaction:hide-outside", () => ({ + refCountMap: new WeakMap(), + inertRefCountMap: new WeakMap(), + observerStack: [], + })); + +/** + * Applies `apply` to `node` the first time it's touched (or observes that a pre-existing native + * state already covers it, in which case `node` is left alone entirely and untracked). Ref-counts + * nested calls; returns whether `node` is now managed by us (`false` means "not ours — untouched"). + */ +function refCountedApply( + map: WeakMap, + tracked: Set, + node: Element, + alreadySet: boolean, + apply: () => void, +): boolean { + const count = map.get(node) ?? 0; + if (alreadySet && count === 0) return false; + if (count === 0) apply(); + map.set(node, count + 1); + tracked.add(node); + return true; +} + +/** Reverts `unapply` once the last ref-counted hold on `node` in `map` is released. */ +function refCountedRelease(map: WeakMap, node: Element, unapply: () => void): void { + const count = map.get(node); + if (count == null) return; + if (count === 1) { + unapply(); + map.delete(node); + } else { + map.set(node, count - 1); + } +} /** * Hides all elements in the DOM outside the given `targets` from screen readers by setting @@ -61,7 +114,8 @@ const observerStack: Array<{ observe(): void; disconnect(): void }> = []; * @param root - Root element to walk. Defaults to `document.body`. * @param alwaysVisibleSelector - Optional CSS selector for elements that must never be hidden * (e.g. `"[aria-live]"` for live-region announcers or top-layer elements). - * @returns A cleanup function that removes all `aria-hidden` attributes added by this call. + * @param inert - Also set the `inert` attribute on hidden elements. *Default = `false`*. + * @returns A cleanup function that removes all `aria-hidden`/`inert` changes added by this call. * * @example * ```ts @@ -81,9 +135,12 @@ export function ariaHideOutside( targets: Element[], root: Element = document.body, alwaysVisibleSelector?: string, + inert = false, ): () => void { + const { refCountMap, inertRefCountMap, observerStack } = getHideOutsideRegistry(); const visibleNodes = new Set(targets); const hiddenNodes = new Set(); + const inertedNodes = new Set(); const walk = (root: Element) => { if (alwaysVisibleSelector) { @@ -117,12 +174,22 @@ export function ariaHideOutside( } }; + // `inert` is only ever applied to nodes hide() actually manages (returns `true`) — a node + // left alone because it already carries an author-set aria-hidden must be left alone for + // `inert` too, for the same "not ours to manage" reason. const hide = (node: Element) => { - const refCount = refCountMap.get(node) ?? 0; - if (node.getAttribute("aria-hidden") === "true" && refCount === 0) return; - if (refCount === 0) node.setAttribute("aria-hidden", "true"); - hiddenNodes.add(node); - refCountMap.set(node, refCount + 1); + const managed = refCountedApply( + refCountMap, + hiddenNodes, + node, + node.getAttribute("aria-hidden") === "true", + () => node.setAttribute("aria-hidden", "true"), + ); + if (managed && inert && node instanceof HTMLElement) { + refCountedApply(inertRefCountMap, inertedNodes, node, node.inert, () => { + node.inert = true; + }); + } }; observerStack[observerStack.length - 1]?.disconnect(); @@ -156,16 +223,12 @@ export function ariaHideOutside( return () => { observer.disconnect(); - hiddenNodes.forEach(node => { - const count = refCountMap.get(node); - if (count == null) return; - if (count === 1) { - node.removeAttribute("aria-hidden"); - refCountMap.delete(node); - } else { - refCountMap.set(node, count - 1); - } - }); + hiddenNodes.forEach(node => refCountedRelease(refCountMap, node, () => node.removeAttribute("aria-hidden"))); + inertedNodes.forEach(node => + refCountedRelease(inertRefCountMap, node, () => { + (node as HTMLElement).inert = false; + }), + ); if (wrapper === observerStack[observerStack.length - 1]) { observerStack.pop(); observerStack[observerStack.length - 1]?.observe(); @@ -210,10 +273,11 @@ export function createHideOutside(options: CreateHideOutsideOptions): void { disabled: !!access(options.disabled), targets: access(options.targets), root: access(options.root), + inert: !!access(options.inert), }), - ({ disabled, targets, root }) => { + ({ disabled, targets, root, inert }) => { if (disabled) return; - return ariaHideOutside(targets, root, options.alwaysVisibleSelector); + return ariaHideOutside(targets, root, options.alwaysVisibleSelector, inert); }, ); } diff --git a/packages/interaction/test/hide-outside.test.ts b/packages/interaction/test/hide-outside.test.ts index b19622246..7e028005c 100644 --- a/packages/interaction/test/hide-outside.test.ts +++ b/packages/interaction/test/hide-outside.test.ts @@ -2,8 +2,6 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; import { createRoot, createSignal, flush } from "solid-js"; import { ariaHideOutside, createHideOutside } from "../src/index.js"; -// ─── ariaHideOutside ────────────────────────────────────────────────────────── - describe("ariaHideOutside", () => { let container: HTMLDivElement; let target: HTMLDivElement; @@ -110,9 +108,58 @@ describe("ariaHideOutside", () => { expect(sibling.getAttribute("aria-hidden")).toBe("true"); sibling.removeAttribute("aria-hidden"); }); -}); -// ─── createHideOutside ──────────────────────────────────────────────────────── + it("does not set inert by default", () => { + const cleanup = ariaHideOutside([target]); + expect(sibling.inert).toBeFalsy(); + cleanup(); + }); + + it("sets inert on hidden elements when inert=true", () => { + const cleanup = ariaHideOutside([target], document.body, undefined, true); + expect(sibling.inert).toBe(true); + expect(sibling.getAttribute("aria-hidden")).toBe("true"); + cleanup(); + expect(sibling.inert).toBeFalsy(); + }); + + it("ref-counts inert across nested calls", () => { + const cleanup1 = ariaHideOutside([target], document.body, undefined, true); + const cleanup2 = ariaHideOutside([target], document.body, undefined, true); + + expect(sibling.inert).toBe(true); + cleanup2(); + expect(sibling.inert).toBe(true); // outer still holds the ref + cleanup1(); + expect(sibling.inert).toBeFalsy(); + }); + + it("does not manage pre-existing inert elements", () => { + sibling.inert = true; + + const cleanup = ariaHideOutside([target], document.body, undefined, true); + cleanup(); + + expect(sibling.inert).toBe(true); + sibling.inert = false; + }); + + it("does not set inert on a node left alone due to pre-existing aria-hidden", () => { + // Regression test: a node with an author-set aria-hidden is left untouched by the + // aria-hidden ref-counting (see "does not manage pre-existing aria-hidden elements" above). + // inert=true must respect that same "not ours to manage" boundary — it must not be applied + // to this node just because inert=true was requested for the call as a whole. + sibling.setAttribute("aria-hidden", "true"); + + const cleanup = ariaHideOutside([target], document.body, undefined, true); + expect(sibling.inert).toBeFalsy(); + cleanup(); + expect(sibling.inert).toBeFalsy(); + expect(sibling.getAttribute("aria-hidden")).toBe("true"); // still untouched + + sibling.removeAttribute("aria-hidden"); + }); +}); describe("createHideOutside", () => { let container: HTMLDivElement; @@ -253,4 +300,25 @@ describe("createHideOutside", () => { dispose(); sibling.removeAttribute("aria-live"); }); + + it("sets inert alongside aria-hidden when inert=true", () => { + const dispose = createRoot(d => { + createHideOutside({ targets: [target], inert: true }); + return d; + }); + flush(); + expect(sibling.inert).toBe(true); + dispose(); + expect(sibling.inert).toBeFalsy(); + }); + + it("does not set inert when inert is omitted", () => { + const dispose = createRoot(d => { + createHideOutside({ targets: [target] }); + return d; + }); + flush(); + expect(sibling.inert).toBeFalsy(); + dispose(); + }); }); diff --git a/packages/pagination/test/hydration.test.tsx b/packages/pagination/test/hydration.test.tsx new file mode 100644 index 000000000..9532761f8 --- /dev/null +++ b/packages/pagination/test/hydration.test.tsx @@ -0,0 +1,50 @@ +import { describe, it, expect, afterEach } from "vitest"; +import { + renderHydrationRoundTrip, + type HydrationRoundTripResult, +} from "../../../scripts/test-utils/hydration-harness.ts"; + +// createPagination backs its return value with several render-body createMemo calls +// (opts, page, pages, start/showFirst/showPrev/showNext/showLast, paginationProps) — the same +// class of construct hope-ui's solid-primitives-eval.md flags as a hydration-id hazard, and the +// single most memo-heavy example across the packages that pattern was found in. Renders through +// the real, standard @solidjs/web renderToString + hydrate pipeline with the package resolved +// from its published `dist` build. +const APP_SOURCE = ` +import { createPagination } from "@solid-primitives/pagination"; +import { For } from "solid-js"; + +export default function App() { + const [paginationProps] = createPagination({ pages: 5, initialPage: 2 }); + return ( + + ); +} +`; + +describe("createPagination hydration round trip", () => { + let result: HydrationRoundTripResult | undefined; + + afterEach(() => { + result?.cleanup(); + result = undefined; + }); + + it("server-renders the pagination buttons", async () => { + result = await renderHydrationRoundTrip(APP_SOURCE, import.meta.dirname); + expect(result.html).toContain(" { + result = await renderHydrationRoundTrip(APP_SOURCE, import.meta.dirname); + expect(result.consoleMessages).toEqual([]); + }); + + it("hydrates the DOM with the correct current-page button marked aria-current", async () => { + result = await renderHydrationRoundTrip(APP_SOURCE, import.meta.dirname); + const current = result.container.querySelector('[aria-current="page"]'); + expect(current?.textContent).toBe("2"); + }); +}); diff --git a/packages/scroll/src/preventScroll.ts b/packages/scroll/src/preventScroll.ts index d5b772201..174ed6a61 100644 --- a/packages/scroll/src/preventScroll.ts +++ b/packages/scroll/src/preventScroll.ts @@ -10,20 +10,9 @@ * https://github.com/theKashey/react-remove-scroll */ -import { createEffect, createSignal } from "solid-js"; +import { createEffect, createSignal, type Signal } from "solid-js"; import { isServer } from "@solidjs/web"; -import { access, type MaybeAccessor } from "@solid-primitives/utils"; - -function contains(wrapper: HTMLElement, target: HTMLElement): boolean { - if (wrapper.contains(target)) return true; - let current: HTMLElement | null = target; - while (current) { - if (current === wrapper) return true; - // @ts-expect-error: _$host is a SolidJS-internal property set on portal roots - current = current._$host ?? current.parentElement; - } - return false; -} +import { access, contains, globalRegistry, type MaybeAccessor } from "@solid-primitives/utils"; type Axis = "x" | "y"; @@ -44,21 +33,11 @@ export type CreatePreventScrollProps = { allowPinchZoom?: MaybeAccessor; }; -// ─── Module-level stack ─────────────────────────────────────────────────────── -// Tracks active instances; only the topmost one installs wheel/touch handlers. - -const [preventScrollStack, setPreventScrollStack] = createSignal([], { - ownedWrite: true, -}); - -const isActive = (id: string): boolean => { - const stack = preventScrollStack(); - return stack.length > 0 && stack[stack.length - 1] === id; -}; - -// ─── Body style tracker ─────────────────────────────────────────────────────── -// Multiple nested instances share a key; the original styles are only restored -// once the last instance cleans up. +// Uses globalRegistry (Symbol.for(...) on globalThis, not module-scope bindings) so the +// active-instance stack and body-style ref-counts stay correct even if the app's dependency +// graph ends up with more than one copy of this package installed — module-scope state would +// otherwise be split across copies and the ref-counting would break (e.g. the "topmost +// instance" check disagreeing between copies). type ActiveStyle = { activeCount: number; @@ -66,7 +45,24 @@ type ActiveStyle = { properties: string[]; }; -const activeBodyStyles = new Map(); +type PreventScrollRegistry = { + stack: Signal; + activeBodyStyles: Map; + /** Shared across duplicate package copies so instance ids never collide on the shared stack. */ + nextId: number; +}; + +const getRegistry = (): PreventScrollRegistry => + globalRegistry("@solid-primitives/scroll:prevent-scroll", () => ({ + stack: createSignal([], { ownedWrite: true }), + activeBodyStyles: new Map(), + nextId: 0, + })); + +const isActive = (id: string): boolean => { + const stack = getRegistry().stack[0](); + return stack.length > 0 && stack[stack.length - 1] === id; +}; function applyBodyStyle( key: string, @@ -74,6 +70,8 @@ function applyBodyStyle( style: Partial, properties: { key: string; value: string }[], ): () => void { + const activeBodyStyles = getRegistry().activeBodyStyles; + const originalStyles: Partial = {}; for (const k in style) { originalStyles[k] = element.style[k as keyof CSSStyleDeclaration] as string; @@ -116,8 +114,6 @@ function applyBodyStyle( }; } -// ─── Scroll helpers ─────────────────────────────────────────────────────────── - function getScrollDimensions(element: HTMLElement, axis: Axis): [number, number, number] { return axis === "x" ? [element.clientWidth, element.scrollLeft, element.scrollWidth] @@ -185,10 +181,6 @@ function wouldScroll( return true; } -// ─── Primitive ──────────────────────────────────────────────────────────────── - -let _nextId = 0; - /** * Prevents scrolling outside of the given element. * @@ -198,7 +190,9 @@ let _nextId = 0; export const createPreventScroll = (props: CreatePreventScrollProps = {}): void => { if (isServer) return; - const id = String(_nextId++); + const registry = getRegistry(); + const id = String(registry.nextId++); + const [, setPreventScrollStack] = registry.stack; let currentTouchStart: [number, number] = [0, 0]; let currentTouchStartAxis: Axis | undefined; diff --git a/packages/state-machine/test/hydration.test.tsx b/packages/state-machine/test/hydration.test.tsx new file mode 100644 index 000000000..714a90d7f --- /dev/null +++ b/packages/state-machine/test/hydration.test.tsx @@ -0,0 +1,54 @@ +import { describe, it, expect, afterEach } from "vitest"; +import { + renderHydrationRoundTrip, + type HydrationRoundTripResult, +} from "../../../scripts/test-utils/hydration-harness.ts"; + +// createMachine backs its return value with a render-body createMemo — the same class of +// construct hope-ui's solid-primitives-eval.md flags as a hydration-id hazard. Renders through +// the real, standard @solidjs/web renderToString + hydrate pipeline with the package resolved +// from its published `dist` build. +const APP_SOURCE = ` +import { createMachine } from "@solid-primitives/state-machine"; + +export default function App() { + const state = createMachine({ + initial: "idle", + states: { + idle: () => "foo", + loading: () => "bar", + }, + }); + return ( +
+ {state().type} + {state().value} +
+ ); +} +`; + +describe("createMachine hydration round trip", () => { + let result: HydrationRoundTripResult | undefined; + + afterEach(() => { + result?.cleanup(); + result = undefined; + }); + + it("server-renders the initial state", async () => { + result = await renderHydrationRoundTrip(APP_SOURCE, import.meta.dirname); + expect(result.html).toContain("foo"); + }); + + it("hydrates without any console error or warning", async () => { + result = await renderHydrationRoundTrip(APP_SOURCE, import.meta.dirname); + expect(result.consoleMessages).toEqual([]); + }); + + it("hydrates the DOM with the correct initial state", async () => { + result = await renderHydrationRoundTrip(APP_SOURCE, import.meta.dirname); + expect(result.container.querySelector("#type")?.textContent).toBe("idle"); + expect(result.container.querySelector("#value")?.textContent).toBe("foo"); + }); +}); diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 434439a5e..1e8b2e7a7 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -16,7 +16,6 @@ import { DEV, } from "solid-js"; - // isServer moved from solid-js/web (1.x) to @solidjs/web (2.x). // typeof window is a universal fallback compatible with both versions. const isServer: boolean = typeof window === "undefined"; @@ -398,8 +397,6 @@ export const afterPaint = (fn: () => void): void => { } }; -// ─── String transforms ──────────────────────────────────────────────────────── - /** * Parse a string as a single JSON value. * @@ -484,8 +481,6 @@ export function pipe(a: (raw: string) => A, b: (a: A) => B): (raw: string) return (raw: string): B => b(a(raw)); } -// ─── DOM helpers ───────────────────────────────────────────────────────────── - /** * Check if a wrapper element contains a target element. * Portal-aware: follows SolidJS `_$host` links so elements rendered inside @@ -501,6 +496,26 @@ export const contains = (wrapper: HTMLElement, target: HTMLElement): boolean => } return false; }; +/** + * Returns a singleton value keyed by `key`, shared across every copy of the calling module that + * ends up loaded in the same JS realm (a `Symbol.for(key)` slot on `globalThis`, not a module-scope + * binding). Use this instead of a plain module-scope `let`/`const` for state that must stay + * consistent (ref-counts, active-instance stacks) even if the app's dependency graph ends up with + * more than one installed copy of the package — module-scope state would otherwise be split across + * copies and go out of sync (e.g. a "topmost instance" check disagreeing between copies). + * + * ```ts + * const registry = globalRegistry("@solid-primitives/my-package:my-state", () => ({ + * stack: createSignal([], { ownedWrite: true }), + * nextId: 0, + * })); + * ``` + */ +export function globalRegistry(key: string, init: () => T): T { + const g = globalThis as Record; + return (g[Symbol.for(key)] ??= init()); +} + /** * Wraps a setter function of any signal or store * diff --git a/packages/utils/test/index.test.ts b/packages/utils/test/index.test.ts index 41bb2e82d..f82606519 100644 --- a/packages/utils/test/index.test.ts +++ b/packages/utils/test/index.test.ts @@ -1,6 +1,12 @@ import { describe, test, expect, assert, vi } from "vitest"; import { createSignal, createStore, flush } from "solid-js"; -import { handleDiffArray, arrayEquals, createHydratableSignal, wrapSetter } from "../src/index.js"; +import { + handleDiffArray, + arrayEquals, + createHydratableSignal, + wrapSetter, + globalRegistry, +} from "../src/index.js"; describe("handleDiffArray", () => { test("handleAdded called for new array", () => { @@ -104,6 +110,34 @@ describe("createHydratableSignal", () => { }); }); +describe("globalRegistry", () => { + test("calls init exactly once and returns the same instance on subsequent calls", () => { + const init = vi.fn(() => ({ count: 0 })); + const key = `test:globalRegistry:${Math.random()}`; + const first = globalRegistry(key, init); + const second = globalRegistry(key, init); + expect(first).toBe(second); + expect(init).toHaveBeenCalledOnce(); + }); + + test("survives a fresh call site sharing the same key — simulates duplicate installed copies", () => { + const key = `test:globalRegistry:duplicate-copy:${Math.random()}`; + // Simulates two separate module instances of the same package (e.g. a version-skewed + // duplicate dependency) each calling globalRegistry with the same key: both must observe + // the same mutations, matching real singleton semantics across copies. + const registryA = globalRegistry(key, () => ({ stack: [] as string[] })); + const registryB = globalRegistry(key, () => ({ stack: ["should never be used"] })); + registryA.stack.push("from-a"); + expect(registryB.stack).toEqual(["from-a"]); + }); + + test("different keys get independent instances", () => { + const a = globalRegistry(`test:globalRegistry:a:${Math.random()}`, () => ({ value: 1 })); + const b = globalRegistry(`test:globalRegistry:b:${Math.random()}`, () => ({ value: 2 })); + expect(a).not.toBe(b); + }); +}); + describe("wrapSetter", () => { test("wraps a signal", () => { const wrapped = vi.fn((x) => x); diff --git a/scripts/test-utils/hydration-harness.ts b/scripts/test-utils/hydration-harness.ts new file mode 100644 index 000000000..065bd530a --- /dev/null +++ b/scripts/test-utils/hydration-harness.ts @@ -0,0 +1,205 @@ +/** + * Real server-render → client-hydrate round trip, for catching hydration hazards that + * per-package `test/server.test.ts` files structurally cannot: those run under this + * monorepo's `@solid-primitives/source` resolve condition, so every `@solid-primitives/*` + * import resolves to workspace `src` instead of the published `dist` build a real consumer + * gets from node_modules. + * + * The SERVER half genuinely closes that gap: it's rendered in a real `node` subprocess with no + * special resolve conditions, so `@solid-primitives/*` and `@solidjs/web` resolve exactly as + * they would for a real npm consumer (dist builds, the real "node" export condition). + * + * The CLIENT half does not close it the same way. `hydrate()` runs back in the calling Vitest + * worker, so the dom-compiled fixture's own `@solid-primitives/*` imports still resolve through + * Vite's module graph — this project's `@solid-primitives/source` condition is active there, + * i.e. workspace `src`, not `dist` (confirmed empirically: a marker export added only to a + * package's `src` was visible from inside the dynamically-imported dom module). For packages + * with no JSX of their own — the common case for `create*` primitives — this doesn't affect the + * property under test: Solid's owner-id allocation lives entirely in `@solidjs/signals`'s + * runtime and doesn't depend on whether the *calling* code was pre-compiled. It would matter for + * a package whose own public surface includes JSX. + * + * ⚠️ Because the server half resolves `dist` and the client half resolves `src`, a **stale** + * `dist` build (edited `src` without rebuilding) makes the two halves run genuinely different + * code — e.g. one side still has an old `createMemo` the other doesn't — which desyncs owner-id + * allocation for real and surfaces as a false-positive "unclaimed server-rendered node" warning + * that has nothing to do with the hazard under test. Rebuild (`pnpm -w build`, or scoped to the + * touched packages) after editing a package's `src` and before trusting a hydration test result. + * + * See the hope-ui solid-primitives-eval.md "transform-boundary hydration hazard": a + * render-body `createSignal(fn)`/`createMemo(fn)` living in an un-transformed dependency was + * reported to desync hydration ids between server and client. Whether that's a general + * solid-js limitation or specific to a non-standard SSR harness, this test proves (or + * disproves) it for each package using the real, standard `@solidjs/web` renderToString + + * hydrate pipeline — no custom harness of our own to second-guess. + */ + +import { transformAsync } from "@babel/core"; +import babelTypescript from "@babel/preset-typescript"; +import babelSolid from "babel-preset-solid"; +import { execFileSync } from "node:child_process"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { pathToFileURL } from "node:url"; + +async function compile(source: string, generate: "ssr" | "dom"): Promise { + const result = await transformAsync(source, { + filename: "hydration-app.tsx", + // Presets apply last-to-first: typescript strips types before babel-preset-solid sees JSX. + presets: [ + [ + babelSolid, + { + moduleName: "@solidjs/web", + generate, + hydratable: true, + omitNestedClosingTags: false, + }, + ], + [babelTypescript, { isTSX: true, allExtensions: true }], + ], + ast: false, + sourceMaps: false, + configFile: false, + babelrc: false, + parserOpts: { plugins: ["jsx", "typescript"] }, + }); + if (!result?.code) throw new Error("[hydration-harness] babel-preset-solid produced no output"); + return result.code; +} + +type PreparedFixture = { html: string; domFile: string }; + +// Compiling + spawning the SSR subprocess is deterministic given (appSource, cwd), so it's +// cached and reused across every `it()` block in a test file that shares the same fixture +// source — each test still gets its own fresh container/hydrate() call, only the expensive +// "produce the server HTML" step is shared. Temp files are cleaned up once, at process exit, +// rather than per-call, since the cached domFile path is reused across calls. +const fixtureCache = new Map>(); +const tempDirsPendingCleanup = new Set(); +let exitCleanupRegistered = false; + +function registerExitCleanup(): void { + if (exitCleanupRegistered) return; + exitCleanupRegistered = true; + process.once("exit", () => { + for (const dir of tempDirsPendingCleanup) { + try { + rmSync(dir, { recursive: true, force: true }); + } catch { + // best-effort — the process is exiting regardless + } + } + }); +} + +async function prepareFixture(appSource: string, cwd: string): Promise { + const key = `${cwd}\0${appSource}`; + let entry = fixtureCache.get(key); + if (!entry) { + entry = (async () => { + const [ssrCode, domCode] = await Promise.all([ + compile(appSource, "ssr"), + compile(appSource, "dom"), + ]); + + const serverEntry = `${ssrCode}\nimport { renderToString } from "@solidjs/web";\nprocess.stdout.write(renderToString(() => App({})));\n`; + + const html = execFileSync(process.execPath, ["--input-type=module"], { + cwd, + input: serverEntry, + encoding: "utf8", + stdio: ["pipe", "pipe", "inherit"], + }); + + const tmpDir = mkdtempSync(join(cwd, ".hydration-harness-")); + tempDirsPendingCleanup.add(tmpDir); + registerExitCleanup(); + const domFile = join(tmpDir, "app.dom.mjs"); + writeFileSync(domFile, domCode); + + return { html, domFile }; + })(); + fixtureCache.set(key, entry); + } + return entry; +} + +export type HydrationRoundTripResult = { + /** The HTML produced by the real `renderToString`, run in a real `node` subprocess. */ + html: string; + /** `container` element the server HTML was hydrated into — still attached to `document.body`. */ + container: HTMLElement; + /** Every `console.error`/`console.warn` message emitted while `hydrate()` ran. */ + consoleMessages: string[]; + /** Removes `container` from the document. Call in test cleanup. */ + cleanup: () => void; +}; + +// The console.error/console.warn monkey-patch and the shared `globalThis._$HY` reset below are +// not reentrant — serialize calls (across concurrent `it()`s/files sharing this module) through +// one chain so a second call can't restore/reassign globals out from under a first call still +// mid-hydrate. +let runQueue: Promise = Promise.resolve(); + +async function runHydrationRoundTrip(appSource: string, cwd: string): Promise { + const { html, domFile } = await prepareFixture(appSource, cwd); + + const container = document.createElement("div"); + container.innerHTML = html; + document.body.appendChild(container); + + const messages: string[] = []; + const originalError = console.error; + const originalWarn = console.warn; + console.error = (...args: unknown[]) => { + messages.push(args.map(String).join(" ")); + }; + console.warn = (...args: unknown[]) => { + messages.push(args.map(String).join(" ")); + }; + + try { + const { default: App } = await import(pathToFileURL(domFile).href); + const { hydrate } = await import("@solidjs/web"); + // Equivalent of executing the inline