patch: general improvements and fixes#993
Conversation
🦋 Changeset detectedLatest commit: b7ef2f3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This PR is derived from some research and comments that came out of a HopeUI document I stumbled on: https://github.com/hope-ui/hope-ui/blob/main/docs/solid-primitives-eval.md. These were not reported but to be proactive in fixing details I've attempted exploring the issues and correcting them.
Responds to hope-ui's
solid-primitives-eval.md, which flagged a suspected "transform-boundary hydration hazard" increateSignal(fn)/createMemo-backed primitives. Traced actualsolid-js@2.0.0-beta.20/@solidjs/signalsinternals and built a real server-render→hydrate round-trip test harness (spawns a genuinenodesubprocess so the package under test resolves its publisheddistbuild, not workspacesrc) to prove/disprove it empirically rather than by inspection alone. Result: the pattern hydrates cleanly acrosscontrolled-signal,a11y,pagination, andstate-machine— evidence points at hope-ui's own non-standard SSR harness, not a general solid-primitives/solid-js defect.Along the way, found and fixed real bugs via an 8-angle self-review of the diff:
interaction: opt-ininertoption oncreateHideOutside/ariaHideOutside, ref-counted alongsidearia-hidden. Fixed a bug whereinertwas applied even to nodes deliberately left alone because they already carry an author-managedaria-hidden— broke that "not ours to manage" invariant.focus: new standalonecreateFocusRestoreprimitive (restore focus without trapping, for non-modal Popover/Tooltip). Fixed a stale-closure race shared withcreateFocusTrap— a fast reactivate-before-restore-fires cycle could restore focus to the wrong element — via one sharedscheduleFocusRestorehelper instead of two copies of the same bug.scroll+interaction: module-scope ref-count/observer state hardened against duplicate installed package copies via a new sharedglobalRegistry()helper in@solid-primitives/utils(also fixed an instance-id collision the extraction would otherwise have introduced).a11y:createFormControl'sdatasetconverted fromcreateMemoto a plain getter — removes a hydration-id-consuming call for a value cheap enough that memoizing it buys nothing.dist-build false positive during development, now documented as a known footgun), reentrancy guard, and caching so repeated fixtures don't re-spawn subprocesses.contains()helper inscrollin favor of the existing one in@solid-primitives/utils.The only new public export across all of this is
globalRegistryin@solid-primitives/utils— everything else is internal refactoring or bug fixes with no signature changes.Some additional adjustments were added:
notification
**'screateNotificationPermission.requestPermissiongainedaffects(permission)soisPending(permission)now works for callers who want the standard idiom, alongside the existingpending` accessor (kept for backward compatibility — pure addition, no breaking change).clipboard— I initially added the same pattern, but discovered through direct experimentation thataffects()only holds its pending mark for a single flush cycle unless it's called inside a realaction()generator. Clipboard'swriteis a plain async function, so the fix would have looked adopted but silently done nothing across the real async gap. I reverted it rather than ship something ineffective. This constraint isn't clearly documented anywhere, so I wrote it up in project memory.controlled-props
**'sBoolProp/NumberProp/RangeProp/StringPropretyped fromComponenttoVoidComponent`, since none of them accept or render children.
All four packages pass their full test suites and typecheck cleanly, each fix verified by reverting it and confirming the corresponding test fails, then restoring.
Also a number of banner comments were removed