Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support touchstart, focus, blur events in link prefetch #2134

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
4 changes: 2 additions & 2 deletions packages/core/src/shouldIntercept.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
export default function shouldIntercept(event: MouseEvent | KeyboardEvent): boolean {
export default function shouldIntercept(event: MouseEvent | KeyboardEvent | import('react').MouseEvent<Element>): boolean {
const isLink = (event.currentTarget as HTMLElement).tagName.toLowerCase() === 'a'
return !(
(event.target && (event?.target as HTMLElement).isContentEditable) ||
event.defaultPrevented ||
(isLink && event.which > 1) ||
(isLink && 'which' in event && event.which > 1) ||
(isLink && event.altKey) ||
(isLink && event.ctrlKey) ||
(isLink && event.metaKey) ||
Expand Down
13 changes: 9 additions & 4 deletions packages/react/src/Link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ const Link = forwardRef<unknown, InertiaLinkProps>(
}, prefetchModes)

const regularEvents = {
onClick: (event) => {
onClick: (event: Parameters<InertiaLinkProps['onClick']>[0]) => {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a bit ugly but it's basically reusing the type of the first parameter of the onClick property defined in InertiaLinkProps above

onClick(event)

if (shouldIntercept(event)) {
Expand All @@ -174,9 +174,6 @@ const Link = forwardRef<unknown, InertiaLinkProps>(
router.visit(href, visitParams)
}
},
}

const prefetchHoverEvents = {
onMouseEnter: () => {
hoverTimeout.current = window.setTimeout(() => {
doPrefetch()
Expand All @@ -185,6 +182,14 @@ const Link = forwardRef<unknown, InertiaLinkProps>(
onMouseLeave: () => {
clearTimeout(hoverTimeout.current)
},
}

const prefetchHoverEvents = {
onMouseEnter: regularEvents.onMouseEnter,
onMouseLeave: regularEvents.onMouseLeave,
onTouchStart: regularEvents.onMouseEnter,
onFocus: regularEvents.onMouseEnter,
onBlur: regularEvents.onMouseLeave,
onClick: regularEvents.onClick,
}

Expand Down
27 changes: 18 additions & 9 deletions packages/svelte/src/link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,22 +54,31 @@ function link(
let baseParams: VisitOptions
let visitParams: VisitOptions

const regularEvents: ActionEventHandlers = {
const regularEvents = {
click: (event) => {
if (shouldIntercept(event)) {
event.preventDefault()
router.visit(href, visitParams)
}
},
}

const prefetchHoverEvents: ActionEventHandlers = {
mouseenter: () => (hoverTimeout = setTimeout(() => prefetch(), 75)),
mouseleave: () => clearTimeout(hoverTimeout),
mouseenter: () => {
hoverTimeout = setTimeout(() => prefetch(), 75)
},
mouseleave: () => {
clearTimeout(hoverTimeout)
},
} satisfies ActionEventHandlers

const prefetchHoverEvents = {
mouseenter: regularEvents.mouseenter,
mouseleave: regularEvents.mouseleave,
touchstart: regularEvents.mouseenter,
focus: regularEvents.mouseenter,
blur: regularEvents.mouseleave,
click: regularEvents.click,
}
} satisfies ActionEventHandlers
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


const prefetchClickEvents: ActionEventHandlers = {
const prefetchClickEvents = {
mousedown: (event) => {
if (shouldIntercept(event)) {
event.preventDefault()
Expand All @@ -86,7 +95,7 @@ function link(
event.preventDefault()
}
},
}
} satisfies ActionEventHandlers

function update({ cacheFor = 0, prefetch = false, ...params }: ActionParameters) {
prefetchModes = (() => {
Expand Down
13 changes: 9 additions & 4 deletions packages/vue3/src/link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,15 +227,12 @@ const Link: InertiaLink = defineComponent({
}

const regularEvents = {
onClick: (event) => {
onClick: (event: Parameters<InertiaLinkProps['onClick']>[0]) => {
if (shouldIntercept(event)) {
event.preventDefault()
router.visit(href.value, visitParams)
}
},
}

const prefetchHoverEvents = {
onMouseenter: () => {
hoverTimeout.value = setTimeout(() => {
prefetch()
Expand All @@ -244,6 +241,14 @@ const Link: InertiaLink = defineComponent({
onMouseleave: () => {
clearTimeout(hoverTimeout.value)
},
}

const prefetchHoverEvents = {
onMouseenter: regularEvents.onMouseenter,
onMouseleave: regularEvents.onMouseleave,
onTouchstart: regularEvents.onMouseenter,
onFocus: regularEvents.onMouseenter,
onBlur: regularEvents.onMouseleave,
onClick: regularEvents.onClick,
}

Expand Down
97 changes: 70 additions & 27 deletions tests/prefetch.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ test('will not prefetch current page', async ({ page }) => {
await page.getByRole('link', { name: 'On Hover (Default)' }).hover()
await page.waitForTimeout(100)
// This is the page we're already on, so it shouldn't make a request
await expect(requests.requests.length).toBe(0)
expect(requests.requests.length).toBe(0)
})

test('can prefetch using link props', async ({ page }) => {
Expand All @@ -51,11 +51,11 @@ test('can prefetch using link props', async ({ page }) => {

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)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awaits here were not necessary, expect().toBe() doesn't return a promise, it's all synchronous


await page.getByRole('link', { name: 'On Hover + Mount' }).click()
await isPrefetchPage(page, 4)
await expect(requests.requests.length).toBe(0)
expect(requests.requests.length).toBe(0)

await page.getByRole('link', { name: 'On Click' }).hover()
await page.mouse.down()
Expand All @@ -64,13 +64,13 @@ test('can prefetch using link props', async ({ page }) => {
requests.listen(page)
await page.mouse.up()
await isPrefetchPage(page, 3)
await expect(requests.requests.length).toBe(0)
expect(requests.requests.length).toBe(0)

requests.listen(page)
await page.getByRole('link', { name: 'On Hover (Default)' }).hover()
await page.getByRole('link', { name: 'On Click' }).hover()
// If they just do a quick hover, it shouldn't make the request
await expect(requests.requests.length).toBe(0)
expect(requests.requests.length).toBe(0)

await page.getByRole('link', { name: 'On Hover (Default)' }).hover()
await page.waitForResponse('prefetch/1')
Expand All @@ -79,7 +79,7 @@ test('can prefetch using link props', async ({ page }) => {
requests.listen(page)
await page.getByRole('link', { name: 'On Hover (Default)' }).click()
await isPrefetchPage(page, 1)
await expect(requests.requests.length).toBe(0)
expect(requests.requests.length).toBe(0)

// Wait for the cache to timeout on the combo link
await page.waitForTimeout(1200)
Expand All @@ -91,7 +91,50 @@ test('can prefetch using link props', async ({ page }) => {
requests.listen(page)
await page.getByRole('link', { name: 'On Hover + Mount' }).click()
await isPrefetchPage(page, 4)
await expect(requests.requests.length).toBe(0)
expect(requests.requests.length).toBe(0)
})

test('can prefetch link on focus', async ({ page, browser }) => {
await page.goto('prefetch/2')
await isPrefetchPage(page, 2)
requests.listen(page)
// If they just do a quick focus, it shouldn't make the request
const link = page.getByRole('link', { exact: true, name: 'On Hover (Default)' })
await link.focus()
await link.blur()
expect(requests.requests.length).toBe(0)

await link.focus()
await page.waitForTimeout(80)
expect(requests.requests.length).toBe(1)
await isPrefetchPage(page, 2)
await page.keyboard.down('Enter')
await isPrefetchPage(page, 1)
expect(requests.requests.length).toBe(1)

// Create a new context to simulate touchscreen
const context2 = await browser.newContext({ hasTouch: true })
const page2 = await context2.newPage()

// These two prefetch requests should be made on mount
const prefetch2 = page2.waitForResponse('prefetch/2')
const prefetch4 = page2.waitForResponse('prefetch/4')

await page2.goto('prefetch/2')

// These two prefetch requests should be made on mount
await prefetch2
await prefetch4

requests.listen(page2)

const box = await page2.getByRole('link', { exact: true, name: 'On Hover (Default)' }).boundingBox()
expect(box).not.toBeNull()
page2.touchscreen.tap(box!.x, box!.y)
await page2.waitForTimeout(75)
expect(requests.requests.length).toBe(1)
await isPrefetchPage(page2, 1)
await context2.close()
})

test('can cache links with single cache value', async ({ page }) => {
Expand All @@ -101,42 +144,42 @@ test('can cache links with single cache value', async ({ page }) => {

// Click back and forth a couple of times to ensure no requests go out
await hoverAndClick(page, '1s Expired (Number)', 3)
await expect(requests.requests.length).toBe(1)
expect(requests.requests.length).toBe(1)
const lastLoaded1 = await page.locator('#last-loaded').textContent()

await hoverAndClick(page, '1s Expired', 2)
await isPrefetchSwrPage(page, 2)
await expect(requests.requests.length).toBe(2)
expect(requests.requests.length).toBe(2)
const lastLoaded2 = await page.locator('#last-loaded').textContent()

requests.listen(page)

await page.getByRole('link', { exact: true, name: '1s Expired (Number)' }).click()
await isPrefetchSwrPage(page, 3)
await expect(requests.requests.length).toBe(0)
expect(requests.requests.length).toBe(0)
const lastLoaded1New = await page.locator('#last-loaded').textContent()
await expect(lastLoaded1).toBe(lastLoaded1New)
expect(lastLoaded1).toBe(lastLoaded1New)

await page.getByRole('link', { exact: true, name: '1s Expired' }).click()
await isPrefetchSwrPage(page, 2)
await expect(requests.requests.length).toBe(0)
expect(requests.requests.length).toBe(0)
const lastLoaded2New = await page.locator('#last-loaded').textContent()
await expect(lastLoaded2).toBe(lastLoaded2New)
expect(lastLoaded2).toBe(lastLoaded2New)

// Wait for cache to expire
await page.waitForTimeout(1200)

requests.listenForFinished(page)

await hoverAndClick(page, '1s Expired (Number)', 3)
await expect(requests.finished.length).toBe(1)
expect(requests.finished.length).toBe(1)
const lastLoaded1Fresh = await page.locator('#last-loaded').textContent()
await expect(lastLoaded1).not.toBe(lastLoaded1Fresh)
expect(lastLoaded1).not.toBe(lastLoaded1Fresh)

await hoverAndClick(page, '1s Expired', 2)
await expect(requests.finished.length).toBe(2)
expect(requests.finished.length).toBe(2)
const lastLoaded2Fresh = await page.locator('#last-loaded').textContent()
await expect(lastLoaded2).not.toBe(lastLoaded2Fresh)
expect(lastLoaded2).not.toBe(lastLoaded2Fresh)
})

test.skip('can do SWR when the link cacheFor prop has two values', async ({ page }) => {
Expand All @@ -145,27 +188,27 @@ test.skip('can do SWR when the link cacheFor prop has two values', async ({ page
requests.listen(page)

await hoverAndClick(page, '1s Stale, 2s Expired (Number)', 5)
await expect(requests.requests.length).toBe(1)
expect(requests.requests.length).toBe(1)
const lastLoaded1 = await page.locator('#last-loaded').textContent()

await hoverAndClick(page, '1s Stale, 2s Expired', 4)
await expect(requests.requests.length).toBe(2)
expect(requests.requests.length).toBe(2)
const lastLoaded2 = await page.locator('#last-loaded').textContent()

requests.listen(page)

// Click back and forth a couple of times to ensure no requests go out
await page.getByRole('link', { exact: true, name: '1s Stale, 2s Expired (Number)' }).click()
await isPrefetchSwrPage(page, 5)
await expect(requests.requests.length).toBe(0)
expect(requests.requests.length).toBe(0)
const lastLoaded1New = await page.locator('#last-loaded').textContent()
await expect(lastLoaded1).toBe(lastLoaded1New)
expect(lastLoaded1).toBe(lastLoaded1New)

await page.getByRole('link', { exact: true, name: '1s Stale, 2s Expired' }).click()
await isPrefetchSwrPage(page, 4)
await expect(requests.requests.length).toBe(0)
expect(requests.requests.length).toBe(0)
const lastLoaded2New = await page.locator('#last-loaded').textContent()
await expect(lastLoaded2).toBe(lastLoaded2New)
expect(lastLoaded2).toBe(lastLoaded2New)

// Wait for stale time to pass
await page.waitForTimeout(1200)
Expand All @@ -178,23 +221,23 @@ test.skip('can do SWR when the link cacheFor prop has two values', async ({ page
await page.getByRole('link', { exact: true, name: '1s Stale, 2s Expired (Number)' }).click()
await isPrefetchSwrPage(page, 5)
const lastLoaded1Stale = await page.locator('#last-loaded').textContent()
await expect(lastLoaded1).toBe(lastLoaded1Stale)
expect(lastLoaded1).toBe(lastLoaded1Stale)
await promiseFor5

// await expect(requests.finished.length).toBe(1)
await page.waitForTimeout(600)
const lastLoaded1Fresh = await page.locator('#last-loaded').textContent()
await expect(lastLoaded1).not.toBe(lastLoaded1Fresh)
expect(lastLoaded1).not.toBe(lastLoaded1Fresh)

const promiseFor4 = page.waitForResponse('prefetch/swr/4')
await page.getByRole('link', { exact: true, name: '1s Stale, 2s Expired' }).click()
await isPrefetchSwrPage(page, 4)
const lastLoaded2Stale = await page.locator('#last-loaded').textContent()
await expect(lastLoaded2).toBe(lastLoaded2Stale)
expect(lastLoaded2).toBe(lastLoaded2Stale)

await promiseFor4
// await expect(requests.finished.length).toBe(2)
await page.waitForTimeout(100)
const lastLoaded2Fresh = await page.locator('#last-loaded').textContent()
await expect(lastLoaded2).not.toBe(lastLoaded2Fresh)
expect(lastLoaded2).not.toBe(lastLoaded2Fresh)
})
Loading