Skip to content

Commit

Permalink
Add handling for prefetching onTouchStart and initial mobile testing (#…
Browse files Browse the repository at this point in the history
…38805)

This adds handling for prefetching `onTouchStart` as this gives a little more time to start parsing required scripts for a page transition if not already done that can help make the transition faster. This is based on research showing the touch start event firing on average `90ms` before click (x-ref: [source](https://instant.page/#:~:text=in%20the%20world.-,On%20mobile,-A%20user%20starts))

This also adds testing safari with playwright so we can run these in PRs instead of only after merge and adds initial mobile testing as well. 

x-ref: [slack thread](https://vercel.slack.com/archives/C7PDM7X2M/p1658250170774989?thread_ts=1658249275.178349&cid=C7PDM7X2M)

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm lint`
- [ ] The examples guidelines are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing.md#adding-examples)
  • Loading branch information
ijjk committed Jul 25, 2022
1 parent 05ba790 commit 135a4cf
Show file tree
Hide file tree
Showing 6 changed files with 132 additions and 45 deletions.
15 changes: 6 additions & 9 deletions .github/workflows/build_test_deploy.yml
Expand Up @@ -816,13 +816,9 @@ jobs:
runs-on: ubuntu-latest
needs: [build, build-native-test]
env:
BROWSERSTACK: true
BROWSER_NAME: 'safari'
NEXT_TELEMETRY_DISABLED: 1
NEXT_TEST_MODE: 'start'
SKIP_LOCAL_SELENIUM_SERVER: true
BROWSERSTACK_USERNAME: ${{ secrets.BROWSERSTACK_USERNAME }}
BROWSERSTACK_ACCESS_KEY: ${{ secrets.BROWSERSTACK_ACCESS_KEY }}
NEXT_TELEMETRY_DISABLED: 1
steps:
- name: Setup node
uses: actions/setup-node@v3
Expand Down Expand Up @@ -851,12 +847,13 @@ jobs:
- run: npm i -g pnpm@${PNPM_VERSION}
if: ${{needs.build.outputs.docsChange == 'nope'}}

# TODO: use macos runner so that we can use playwright to test against
# PRs instead of only running on canary?
- run: '[[ -z "$BROWSERSTACK_ACCESS_KEY" ]] && echo "Skipping for PR" || npm i -g browserstack-local@1.4.0'
- run: npx playwright install-deps && npx playwright install webkit
if: ${{needs.build.outputs.docsChange == 'nope'}}

- run: node run-tests.js -c 1 test/integration/production/test/index.test.js test/e2e/basepath.test.ts
if: ${{needs.build.outputs.docsChange == 'nope'}}

- run: '[[ -z "$BROWSERSTACK_ACCESS_KEY" ]] && echo "Skipping for PR" || node run-tests.js -c 1 test/integration/production/test/index.test.js test/e2e/basepath.test.ts'
- run: DEVICE_NAME='iPhone XR' node run-tests.js -c 1 test/production/prerender-prefetch/index.test.ts
if: ${{needs.build.outputs.docsChange == 'nope'}}

testSafariOld:
Expand Down
31 changes: 30 additions & 1 deletion packages/next/client/link.tsx
Expand Up @@ -48,6 +48,11 @@ type InternalLinkProps = {
*/
onMouseEnter?: (e: any) => void
// e: any because as it would otherwise overlap with existing types
/**
* requires experimental.newNextLinkBehavior
*/
onTouchStart?: (e: any) => void
// e: any because as it would otherwise overlap with existing types
/**
* requires experimental.newNextLinkBehavior
*/
Expand Down Expand Up @@ -215,6 +220,7 @@ const Link = React.forwardRef<HTMLAnchorElement, LinkPropsReal>(
locale: true,
onClick: true,
onMouseEnter: true,
onTouchStart: true,
legacyBehavior: true,
} as const
const optionalProps: LinkPropsOptional[] = Object.keys(
Expand All @@ -239,7 +245,11 @@ const Link = React.forwardRef<HTMLAnchorElement, LinkPropsReal>(
actual: valType,
})
}
} else if (key === 'onClick' || key === 'onMouseEnter') {
} else if (
key === 'onClick' ||
key === 'onMouseEnter' ||
key === 'onTouchStart'
) {
if (props[key] && valType !== 'function') {
throw createPropError({
key,
Expand Down Expand Up @@ -296,6 +306,7 @@ const Link = React.forwardRef<HTMLAnchorElement, LinkPropsReal>(
locale,
onClick,
onMouseEnter,
onTouchStart,
legacyBehavior = Boolean(process.env.__NEXT_NEW_LINK_BEHAVIOR) !== true,
...restProps
} = props
Expand Down Expand Up @@ -411,6 +422,7 @@ const Link = React.forwardRef<HTMLAnchorElement, LinkPropsReal>(
}, [as, href, isVisible, locale, p, router])

const childProps: {
onTouchStart: React.TouchEventHandler
onMouseEnter: React.MouseEventHandler
onClick: React.MouseEventHandler
href?: string
Expand Down Expand Up @@ -466,6 +478,23 @@ const Link = React.forwardRef<HTMLAnchorElement, LinkPropsReal>(
prefetch(router, href, as, { priority: true })
}
},
onTouchStart: (e: React.TouchEvent<HTMLAnchorElement>) => {
if (!legacyBehavior && typeof onTouchStart === 'function') {
onTouchStart(e)
}

if (
legacyBehavior &&
child.props &&
typeof child.props.onTouchStart === 'function'
) {
child.props.onTouchStart(e)
}

if (isLocalURL(href)) {
prefetch(router, href, as, { priority: true })
}
},
}

// If child is an <a> tag and doesn't have a href attribute, or if the 'passHref' property is
Expand Down
2 changes: 1 addition & 1 deletion test/integration/production/test/index.test.js
Expand Up @@ -750,7 +750,7 @@ describe('Production Usage', () => {
.elementByCss('a')
.click()
.waitForElementByCss('.about-page')
.elementByCss('div')
.elementByCss('.about-page')
.text()

expect(text).toBe('About Page')
Expand Down
3 changes: 3 additions & 0 deletions test/lib/browsers/base.ts
Expand Up @@ -31,6 +31,9 @@ export class BrowserInterface {
elementById(selector: string): BrowserInterface {
return this
}
touchStart(): BrowserInterface {
return this
}
click(opts?: { modifierKey?: boolean }): BrowserInterface {
return this
}
Expand Down
20 changes: 19 additions & 1 deletion test/lib/browsers/playwright.ts
Expand Up @@ -8,6 +8,7 @@ import {
BrowserContext,
Page,
ElementHandle,
devices,
} from 'playwright-chromium'
import path from 'path'

Expand Down Expand Up @@ -49,6 +50,17 @@ class Playwright extends BrowserInterface {
async setup(browserName: string, locale?: string) {
if (browser) return
const headless = !!process.env.HEADLESS
let device

if (process.env.DEVICE_NAME) {
device = devices[process.env.DEVICE_NAME]

if (!device) {
throw new Error(
`Invalid playwright device name ${process.env.DEVICE_NAME}`
)
}
}

if (browserName === 'safari') {
browser = await webkit.launch({ headless })
Expand All @@ -57,7 +69,7 @@ class Playwright extends BrowserInterface {
} else {
browser = await chromium.launch({ headless, devtools: !headless })
}
context = await browser.newContext({ locale })
context = await browser.newContext({ locale, ...device })
}

async get(url: string): Promise<void> {
Expand Down Expand Up @@ -284,6 +296,12 @@ class Playwright extends BrowserInterface {
})
}

touchStart() {
return this.chain((el: ElementHandle) => {
return el.dispatchEvent('touchstart').then(() => el)
})
}

elementsByCss(sel) {
return this.chain(() =>
page.$$(sel).then((els) => {
Expand Down
106 changes: 73 additions & 33 deletions test/production/prerender-prefetch/index.test.ts
Expand Up @@ -134,43 +134,83 @@ describe('Prerender prefetch', () => {
expect(isNaN(newTime)).toBe(false)
})

it('should attempt cache update on link hover', async () => {
const browser = await webdriver(next.url, '/')
const timeRes = await fetchViaHTTP(
next.url,
`/_next/data/${next.buildId}/blog/first.json`,
undefined,
{
headers: {
purpose: 'prefetch',
},
}
)
const startTime = (await timeRes.json()).pageProps.now

// ensure stale data is used by default
await browser.elementByCss('#to-blog-first').click()
await check(() => browser.elementByCss('#page').text(), 'blog/[slug]')
if (process.env.DEVICE_NAME) {
it('should attempt cache update on touchstart', async () => {
const browser = await webdriver(next.url, '/')
const timeRes = await fetchViaHTTP(
next.url,
`/_next/data/${next.buildId}/blog/first.json`,
undefined,
{
headers: {
purpose: 'prefetch',
},
}
)
const startTime = (await timeRes.json()).pageProps.now

expect(JSON.parse(await browser.elementByCss('#props').text()).now).toBe(
startTime
)
await browser.back().waitForElementByCss('#to-blog-first')
const requests = []
// ensure stale data is used by default
await browser.elementByCss('#to-blog-first').click()
await check(() => browser.elementByCss('#page').text(), 'blog/[slug]')

browser.on('request', (req) => {
requests.push(req.url())
expect(JSON.parse(await browser.elementByCss('#props').text()).now).toBe(
startTime
)
await browser.back().waitForElementByCss('#to-blog-first')
const requests = []

browser.on('request', (req) => {
requests.push(req.url())
})

// now trigger cache update and navigate again
await check(async () => {
await browser.elementByCss('#to-blog-second').touchStart()
await browser.elementByCss('#to-blog-first').touchStart()
return requests.some((url) => url.includes('/blog/first.json'))
? 'success'
: requests
}, 'success')
})
} else {
it('should attempt cache update on link hover', async () => {
const browser = await webdriver(next.url, '/')
const timeRes = await fetchViaHTTP(
next.url,
`/_next/data/${next.buildId}/blog/first.json`,
undefined,
{
headers: {
purpose: 'prefetch',
},
}
)
const startTime = (await timeRes.json()).pageProps.now

// now trigger cache update and navigate again
await check(async () => {
await browser.elementByCss('#to-blog-second').moveTo()
await browser.elementByCss('#to-blog-first').moveTo()
return requests.some((url) => url.includes('/blog/first.json'))
? 'success'
: requests
}, 'success')
})
// ensure stale data is used by default
await browser.elementByCss('#to-blog-first').click()
await check(() => browser.elementByCss('#page').text(), 'blog/[slug]')

expect(JSON.parse(await browser.elementByCss('#props').text()).now).toBe(
startTime
)
await browser.back().waitForElementByCss('#to-blog-first')
const requests = []

browser.on('request', (req) => {
requests.push(req.url())
})

// now trigger cache update and navigate again
await check(async () => {
await browser.elementByCss('#to-blog-second').moveTo()
await browser.elementByCss('#to-blog-first').moveTo()
return requests.some((url) => url.includes('/blog/first.json'))
? 'success'
: requests
}, 'success')
})
}

it('should handle failed data fetch and empty cache correctly', async () => {
const browser = await webdriver(next.url, '/')
Expand Down

0 comments on commit 135a4cf

Please sign in to comment.