From bf2cdcb9a3143ba35c3f4a6df1d1017fc4b17a98 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Tue, 19 Jul 2022 12:23:31 -0500 Subject: [PATCH 1/2] Add handling for prefetching onTouchStart --- packages/next/client/link.tsx | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index d32089a71b19..ceea17414c9b 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -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 */ @@ -215,6 +220,7 @@ const Link = React.forwardRef( locale: true, onClick: true, onMouseEnter: true, + onTouchStart: true, legacyBehavior: true, } as const const optionalProps: LinkPropsOptional[] = Object.keys( @@ -239,7 +245,11 @@ const Link = React.forwardRef( actual: valType, }) } - } else if (key === 'onClick' || key === 'onMouseEnter') { + } else if ( + key === 'onClick' || + key === 'onMouseEnter' || + key === 'onTouchStart' + ) { if (props[key] && valType !== 'function') { throw createPropError({ key, @@ -296,6 +306,7 @@ const Link = React.forwardRef( locale, onClick, onMouseEnter, + onTouchStart, legacyBehavior = Boolean(process.env.__NEXT_NEW_LINK_BEHAVIOR) !== true, ...restProps } = props @@ -411,6 +422,7 @@ const Link = React.forwardRef( }, [as, href, isVisible, locale, p, router]) const childProps: { + onTouchStart: React.TouchEventHandler onMouseEnter: React.MouseEventHandler onClick: React.MouseEventHandler href?: string @@ -466,6 +478,23 @@ const Link = React.forwardRef( prefetch(router, href, as, { priority: true }) } }, + onTouchStart: (e: React.TouchEvent) => { + 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 tag and doesn't have a href attribute, or if the 'passHref' property is From dfb1fc9fcfd1f30bb62db9d2c5f68f5e3f66bc9b Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 25 Jul 2022 11:36:57 -0500 Subject: [PATCH 2/2] migrate safarit testing to playwright and add mobile tests --- .github/workflows/build_test_deploy.yml | 15 +-- .../integration/production/test/index.test.js | 2 +- test/lib/browsers/base.ts | 3 + test/lib/browsers/playwright.ts | 20 +++- .../prerender-prefetch/index.test.ts | 106 ++++++++++++------ 5 files changed, 102 insertions(+), 44 deletions(-) diff --git a/.github/workflows/build_test_deploy.yml b/.github/workflows/build_test_deploy.yml index c4685e243ad2..299e922df924 100644 --- a/.github/workflows/build_test_deploy.yml +++ b/.github/workflows/build_test_deploy.yml @@ -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 @@ -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: diff --git a/test/integration/production/test/index.test.js b/test/integration/production/test/index.test.js index d17ebed09b77..8056768dec19 100644 --- a/test/integration/production/test/index.test.js +++ b/test/integration/production/test/index.test.js @@ -750,7 +750,7 @@ describe('Production Usage', () => { .elementByCss('a') .click() .waitForElementByCss('.about-page') - .elementByCss('div') + .elementByCss('.about-page') .text() expect(text).toBe('About Page') diff --git a/test/lib/browsers/base.ts b/test/lib/browsers/base.ts index 6a156d2fae79..499f4855353c 100644 --- a/test/lib/browsers/base.ts +++ b/test/lib/browsers/base.ts @@ -31,6 +31,9 @@ export class BrowserInterface { elementById(selector: string): BrowserInterface { return this } + touchStart(): BrowserInterface { + return this + } click(opts?: { modifierKey?: boolean }): BrowserInterface { return this } diff --git a/test/lib/browsers/playwright.ts b/test/lib/browsers/playwright.ts index 71ca46975de8..772441482c45 100644 --- a/test/lib/browsers/playwright.ts +++ b/test/lib/browsers/playwright.ts @@ -8,6 +8,7 @@ import { BrowserContext, Page, ElementHandle, + devices, } from 'playwright-chromium' import path from 'path' @@ -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 }) @@ -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 { @@ -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) => { diff --git a/test/production/prerender-prefetch/index.test.ts b/test/production/prerender-prefetch/index.test.ts index 3d89fc122a0a..27d203c4bced 100644 --- a/test/production/prerender-prefetch/index.test.ts +++ b/test/production/prerender-prefetch/index.test.ts @@ -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, '/')