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 87bc2f3
☁️ Nx Cloud last updated this comment at |
|
@birkskyum thoughts? |
Hooray! CodSpeed harness just leveled up!The base and head of this comparison were measured with different runner settings, so their benchmark values are not directly comparable. What changed between base and head:
Re-run the base with the same settings to get a valid performance comparison. Comparing 
Footnotes |
2 participants
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).