Support touchstart, focus, blur events in link prefetch
#2134
Conversation
|
|
||
| const regularEvents = { | ||
| onClick: (event) => { | ||
| onClick: (event: Parameters<InertiaLinkProps['onClick']>[0]) => { |
There was a problem hiding this comment.
This looks a bit ugly but it's basically reusing the type of the first parameter of the onClick property defined in InertiaLinkProps above
| blur: regularEvents.mouseleave, | ||
| click: regularEvents.click, | ||
| } | ||
| } satisfies ActionEventHandlers |
There was a problem hiding this comment.
I replaced the type assignment with the satisfies operator to make autocomplete smarter. Now typescript knows which properties from ActionEventHandlers are defined and which aren't while still validating the types are correct.
| await page.getByRole('link', { name: 'On Mount' }).click() | ||
| await isPrefetchPage(page, 2) | ||
| await expect(requests.requests.length).toBe(0) | ||
| expect(requests.requests.length).toBe(0) |
There was a problem hiding this comment.
awaits here were not necessary, expect().toBe() doesn't return a promise, it's all synchronous
m5r
force-pushed
the
expand-prefetch-behavior
branch
from
4d726f2 to
c9e0aa2
Compare
joetannenbaum
left a comment
joetannenbaum
left a comment
There was a problem hiding this comment.
This looks cool, I just have a couple of questions here that I need clarification on. Thanks!
packages/core/src/shouldIntercept.ts
Outdated
| @@ -1,9 +1,9 @@ | |||
| export default function shouldIntercept(event: MouseEvent | KeyboardEvent): boolean { | |||
| export default function shouldIntercept(event: MouseEvent | KeyboardEvent | import('react').MouseEvent<Element>): boolean { | |||
There was a problem hiding this comment.
Might be a silly question, but doesn't this make react a dependency in the core library now?
| } | ||
|
|
||
| const prefetchHoverEvents = { | ||
| onMouseEnter: () => { |
There was a problem hiding this comment.
Why did you move these events into the regularEvents? These shouldn't be fired if they don't intend to prefetch, correct?
|
@m5r Thanks for this, looks like a great PR! Could you maybe check out Joe's comments about the React import and the |
pascalbaljet
added
the
needs more info/work
|
I'm closing this pull request due to lack of activity. If you're still working on this or have updates to share, feel free to open a new PR. We'd be happy to take another look. Thanks! |
3 participants
Add support for
touchstart,focus, andblurevents in link prefetchhoverbehavior.This is hugely inspired from React Router's link prefetching code. Their event handlers composition is pretty neat too but I don't think it's necessary here.