From e1ef7d3354a0eb4553d93838b3a5eba7e431f737 Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 26 Aug 2026 10:59:19 +0700 Subject: [PATCH 1/4] fix(overlay): give CSS sole visibility ownership --- src/components/_shared/overlayPosition.ts | 16 ++--- .../components/popover/Popover.anchor.test.ts | 61 ++++++++++++++++++- 2 files changed, 67 insertions(+), 10 deletions(-) diff --git a/src/components/_shared/overlayPosition.ts b/src/components/_shared/overlayPosition.ts index 5e3ea960..8fc472a4 100644 --- a/src/components/_shared/overlayPosition.ts +++ b/src/components/_shared/overlayPosition.ts @@ -103,16 +103,18 @@ export const resolveOverlayAnchorRect = ( export const createOverlayPosition = ( options: CreateOverlayPositionOptions, ) => { - const [style, setStyle] = createSignal({ - visibility: "hidden", - }); + // Visibility belongs to each overlay component's `data-open` CSS contract. + // Positioning must never write an inline visibility override: portal refs can + // settle after the open signal, and an override left behind here makes a + // semantically open Select or Popover impossible to see or operate. + const [style, setStyle] = createSignal({}); const [resolvedPlacement, setResolvedPlacement] = createSignal(options.placement()); createTrackedEffect(() => { if (!options.open()) { setResolvedPlacement(options.placement()); - setStyle({ visibility: "hidden" }); + setStyle({}); return; } @@ -215,10 +217,8 @@ export const createOverlayPosition = ( frame = requestAnimationFrame(update); }; - // Opening must make the overlay paint in the same reactive turn. Keeping - // the initial inline `visibility: hidden` until requestAnimationFrame - // overrides the component's `[data-open="true"]` CSS and leaves a valid - // select or popover invisible whenever the next frame is delayed. + // Opening must position the overlay in the same reactive turn. Visibility + // is intentionally absent from this style object; data-open CSS owns it. update(); window.addEventListener("resize", schedule); diff --git a/tests/components/popover/Popover.anchor.test.ts b/tests/components/popover/Popover.anchor.test.ts index 96b865d4..303bd064 100644 --- a/tests/components/popover/Popover.anchor.test.ts +++ b/tests/components/popover/Popover.anchor.test.ts @@ -49,7 +49,7 @@ describe("Popover virtual anchoring", () => { expect(resolveOverlayAnchorRect(trigger, undefined)).toBe(triggerRect); }); - it("removes the hidden inline style before the next animation frame", async () => { + it("never overrides the component visibility contract", async () => { const requestFrame = mock(() => 1); Object.defineProperty(globalThis, "window", { configurable: true, @@ -90,7 +90,7 @@ describe("Popover virtual anchoring", () => { offset: () => 6, }); - expect(position.style()).toEqual({ visibility: "hidden" }); + expect(position.style()).toEqual({}); setOpen(true); await Promise.resolve(); expect(position.style()).toMatchObject({ @@ -103,4 +103,61 @@ describe("Popover virtual anchoring", () => { dispose(); }); }); + + it("stays visibility-safe while a portal ref settles after opening", async () => { + Object.defineProperty(globalThis, "window", { + configurable: true, + value: { + innerWidth: 800, + innerHeight: 600, + addEventListener: mock(() => {}), + removeEventListener: mock(() => {}), + }, + }); + Object.defineProperty(globalThis, "requestAnimationFrame", { + configurable: true, + value: mock(() => 1), + }); + Object.defineProperty(globalThis, "cancelAnimationFrame", { + configurable: true, + value: mock(() => {}), + }); + + await createRoot(async (dispose) => { + const [open, setOpen] = createSignal(false); + const [overlay, setOverlay] = createSignal(); + const position = createOverlayPosition({ + open, + triggerRef: () => undefined, + anchorRect: () => virtualRect, + overlayRef: overlay, + placement: () => "bottom", + offset: () => 6, + }); + + setOpen(true); + await Promise.resolve(); + expect(position.style()).toEqual({}); + + setOverlay({ + getBoundingClientRect: () => ({ + top: 0, + left: 0, + width: 160, + height: 120, + right: 160, + bottom: 120, + }), + } as HTMLElement); + await Promise.resolve(); + + expect(position.style()).toMatchObject({ + position: "fixed", + top: "56px", + left: "8px", + }); + expect(position.style()).not.toHaveProperty("visibility"); + dispose(); + }); + }); }); From d4e0fd0aa8b1681994d5f86edec06beba610e07c Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 26 Aug 2026 11:12:39 +0700 Subject: [PATCH 2/4] fix(select): defer compound children under context --- src/components/select/Select.layout.tsx | 2 +- tests/component-state-contract.test.ts | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/components/select/Select.layout.tsx b/src/components/select/Select.layout.tsx index 6766488f..fcd7a8df 100644 --- a/src/components/select/Select.layout.tsx +++ b/src/components/select/Select.layout.tsx @@ -436,7 +436,7 @@ const SelectRoot: Layout = () => { data-disabled={disabled() ? "true" : "false"} data-selection-mode={selectionMode()} > - {props.children} + {children} ); diff --git a/tests/component-state-contract.test.ts b/tests/component-state-contract.test.ts index b0f41c43..42ed4ddf 100644 --- a/tests/component-state-contract.test.ts +++ b/tests/component-state-contract.test.ts @@ -3,6 +3,7 @@ import { missingRecipeFlagUsages, recipeFlagKeys, } from "../scripts/component-state-contract"; +import { readFileSync } from "node:fs"; const inlineEditRecipe = ` const CLASSES = { @@ -52,4 +53,18 @@ describe("component state contract", () => { expect(missingRecipeFlagUsages(inlineEditRecipe, layout)).toEqual([]); }); + + it("constructs Select compound children through the deferred context channel", () => { + const generated = readFileSync( + new URL("../src/components/select/Select.generated.tsx", import.meta.url), + "utf8", + ); + const root = generated.slice( + generated.indexOf("const __solidLayoutSelectRoot"), + generated.indexOf("const SelectRoot =", generated.indexOf("const __solidLayoutSelectRoot")), + ); + + expect(root).toContain("{_stable.children}"); + expect(root).not.toContain("{p.children}"); + }); }); From 8f825a4ed28bf6eddef4b5a4ff24128ac97ad284 Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 26 Aug 2026 11:29:17 +0700 Subject: [PATCH 3/4] fix(select): keep listbox in the renderer tree --- src/components/select/Select.layout.tsx | 2 +- tests/component-state-contract.test.ts | 15 --------------- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/src/components/select/Select.layout.tsx b/src/components/select/Select.layout.tsx index fcd7a8df..6766488f 100644 --- a/src/components/select/Select.layout.tsx +++ b/src/components/select/Select.layout.tsx @@ -436,7 +436,7 @@ const SelectRoot: Layout = () => { data-disabled={disabled() ? "true" : "false"} data-selection-mode={selectionMode()} > - {children} + {props.children} ); diff --git a/tests/component-state-contract.test.ts b/tests/component-state-contract.test.ts index 42ed4ddf..b0f41c43 100644 --- a/tests/component-state-contract.test.ts +++ b/tests/component-state-contract.test.ts @@ -3,7 +3,6 @@ import { missingRecipeFlagUsages, recipeFlagKeys, } from "../scripts/component-state-contract"; -import { readFileSync } from "node:fs"; const inlineEditRecipe = ` const CLASSES = { @@ -53,18 +52,4 @@ describe("component state contract", () => { expect(missingRecipeFlagUsages(inlineEditRecipe, layout)).toEqual([]); }); - - it("constructs Select compound children through the deferred context channel", () => { - const generated = readFileSync( - new URL("../src/components/select/Select.generated.tsx", import.meta.url), - "utf8", - ); - const root = generated.slice( - generated.indexOf("const __solidLayoutSelectRoot"), - generated.indexOf("const SelectRoot =", generated.indexOf("const __solidLayoutSelectRoot")), - ); - - expect(root).toContain("{_stable.children}"); - expect(root).not.toContain("{p.children}"); - }); }); From 1c82ddb6380dff916f1f729c1f7b45783367c368 Mon Sep 17 00:00:00 2001 From: meh Date: Tue, 1 Sep 2026 00:33:40 +0700 Subject: [PATCH 4/4] fix(qa): build the harness without solid-refresh Every harness page rendered an empty body. The console said "Cannot read properties of undefined (reading 'spec')" from inside the runtime, which is a component being invoked with no props at all, and the reactive system halting on the way. `@rsbuild/plugin-solid` injects solid-refresh, and its `$$component` wrapper calls `createSignal(component)`. Solid 2 reads a function initialiser as a derivation and builds a computed for it, and a computed needs an owner -- so the wrapper breaks every component the harness defines, `Harness` included. Babel does the same transform without the refresh runtime. `moduleName` stays, for the reason the old comment gave: the transform has to emit `@solidjs/web`, since Solid 2 dropped the `solid-js/web` subpath. This is the tool that is supposed to catch a component rendering nothing, and it could not render anything itself. With it working, the overlay fix on this branch is verifiable rather than merely tested: Select goes from `data-open="false"`, `visibility: hidden`, height 0 to `data-open="true"`, `visibility: visible`, height 86 with no inline visibility override, and Popover's panel paints at 40px. --- tests/qa-harness/rsbuild.config.ts | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/tests/qa-harness/rsbuild.config.ts b/tests/qa-harness/rsbuild.config.ts index 57cef283..5beb99de 100644 --- a/tests/qa-harness/rsbuild.config.ts +++ b/tests/qa-harness/rsbuild.config.ts @@ -6,7 +6,6 @@ * the working tree rather than the last publish. */ import { defineConfig } from "@rsbuild/core"; -import { pluginSolid } from "@rsbuild/plugin-solid"; import { pluginBabel } from "@rsbuild/plugin-babel"; import { pluginSolidLayoutsApplication } from "rsbuild-plugin-solid-layouts"; import { resolve } from "node:path"; @@ -19,13 +18,27 @@ export default defineConfig({ // compiles `*.layout.tsx` sources; this one wires `solid-layouts` for a // consumer, which is what the harness is. pluginSolidLayoutsApplication(), - pluginBabel({ include: /\.(?:jsx|tsx)$/ }), - // `moduleName`/`generate` for the same reason `rslib.config.ts` sets them: - // `@rsbuild/plugin-solid` resolves a nested Solid 1 preset whose transform - // emits `solid-js/web`, a subpath Solid 2 dropped. Without this the harness - // builds and then fails to resolve at runtime. - pluginSolid({ - solidPresetOptions: { moduleName: "@solidjs/web", generate: "dom" }, + /* + * Babel does the Solid transform; `@rsbuild/plugin-solid` is deliberately + * absent. It injects solid-refresh, whose `$component` wrapper calls + * `createSignal(component)` — and Solid 2 reads a function initialiser as a + * derivation, so it builds a computed that needs an owner. Every harness + * page rendered an empty body and reported "Cannot read properties of + * undefined (reading 'spec')": the component was being invoked with no + * props at all. + * + * `moduleName` stays for the original reason — the transform must emit + * `@solidjs/web`, since Solid 2 dropped the `solid-js/web` subpath. + */ + pluginBabel({ + include: /\.(?:jsx|tsx)$/, + babelLoaderOptions: (config) => { + config.presets ??= []; + config.presets.push([ + "babel-preset-solid", + { moduleName: "@solidjs/web", generate: "dom" }, + ]); + }, }), ], /*