fix(solid-router): don't install intent-preload listeners when preloading is off - #8179
fix(solid-router): don't install intent-preload listeners when preloading is off#8179russelgal wants to merge 2 commits into
Conversation
…ding is off useLinkProps handed out onFocus/onBlur/onMouseEnter/onMouseLeave (plus the mouseover/mouseout/touchstart trio) unconditionally, with the `preload() !== 'intent'` check inside each handler. Solid does not delegate mouseenter, mouseleave, focus or blur, so every anchor installed four real listeners whose only job was to bail out — four per row on list views. Resolve those props through getters instead: with intent preloading off the property yields the consumer's own handler (or undefined, which spread()/assign() treats as removal), so nothing is attached. The getters stay reactive, so switching preload back to 'intent' re-runs the consuming spread and attaches the composed handler. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
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:
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 |
|
View your CI Pipeline Execution ↗ for commit a5eda23 ☁️ Nx Cloud last updated this comment at |
Problem
useLinkPropsassigns the preload handlers unconditionally (link.tsx):The
preloadcheck lives inside the handlers (if (preload() !== 'intent') return), so with preloading off they are still attached and still fire — they just return.That is not free. Solid's
DelegatedEventsset does not includemouseenter,mouseleave,focusorblur(they don't bubble), soassignPropinstalls a realaddEventListenerper anchor for each of them.click,mouseover,mouseoutandtouchstartare delegated and cost nothing.Net effect: four listeners per
<Link>that do nothing at all wheneverpreloadisfalse,'viewport'or'render'.I hit this on a booking board where every row is a link. Measured in Chrome on a real page, counting
addEventListenercalls for one client-side navigation onto the screen:<Link preload={false}>blur/focus/mouseenter/mouseleave× 165)Fix
Resolve those props through getters (the file already relies on this: "values that no longer apply resolve to undefined, which spread()/assign() treats as attribute removal"). With intent preloading off, the property yields whatever the consumer passed — or
undefined, and nothing gets attached:Behaviour is unchanged:
preload: 'intent'— identical to before.'viewport'— preloading runs through the IntersectionObserver; the events were already no-ops there.'render'— preloading runs in an effect; same.onMouseEnter/onFocus/… still fires, because the getter falls back to it.preloadback to'intent're-runs the consumingspread(), which attaches the composed handler then (andassignPropremoves a stale non-delegated listener before adding).Tests
Added to
tests/link.test.tsx, alongside the existing preload/IntersectionObserver ones:Router.preload="false" | "viewport" | "render"— nomouseenter/mouseleave/focus/blurlisteners on the anchor (spying onHTMLAnchorElement.prototype.addEventListener).Router.preload="intent"— the listeners are installed, as before.Link.preload={false}with the consumer's ownonMouseEnter/onFocus— both still fire.Verified the new tests fail without the fix (3 of them) and pass with it.
Ran locally on
solid-router-v2-pre:eslint src/link.tsx tests/link.test.tsx— cleantsc -p tsconfig.legacy.json— cleanvitest run— 59 files, 870 passed / 2 skippedvitest run --mode server— 3 files, 4 passedChangeset included (patch).