From 7c3ef83fb6b19fc1e4bbfe5ad21804cbdb452f6c Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Thu, 27 Feb 2025 13:49:06 +0100 Subject: [PATCH 1/3] Unflake `parallel-routes-revalidation` test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [flakiness metric](https://app.datadoghq.com/ci/test/runs?query=test_level%3Atest%20env%3Aci%20%40git.repository.id%3Agithub.com%2Fvercel%2Fnext.js%20%40test.service%3Anextjs%20%40test.status%3Afail%20%28-%40git.branch%3A%2A%3F%2A%20OR%20%40git.branch%3Acanary%29%20%40test.name%3A%22parallel-routes-revalidation%20should%20not%20trigger%20the%20intercepted%20route%20when%20lazy-fetching%20missing%20data%22&agg_m=count&agg_m_source=base&agg_t=count&citest_explorer_sort=timestamp%2Casc&cols=%40test.status%2Ctimestamp%2C%40test.suite%3A83%2C%40test.name%3A607%2C%40git.branch¤tTab=overview&eventStack=&fromUser=false&graphType=flamegraph&index=citest&start=1738068572273&end=1740660572273&paused=false) The latest canary release [failed](https://github.com/vercel/next.js/actions/runs/13555525831/job/37890389323) because the deploy test `parallel-routes-revalidation › should not trigger the intercepted route when lazy-fetching missing data` kept failing even with retries. According to my local testing using `pnpm test-deploy test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts -t 'should not trigger the intercepted route when lazy-fetching missing data'`, the forward navigation was sometimes triggered too early after the page was reloaded. This PR introduces an option to wait for hydration (same as we do for the initial load) when reloading a page, which fixed the test for me. Note: I don't like the option name `waitHydration`, but went with it anyways because we're already using it for the `next.browser` options. --- .../parallel-routes-revalidation.test.ts | 5 +- test/lib/browsers/base.ts | 8 ++- test/lib/browsers/playwright.ts | 65 ++++++++++++++++++- test/lib/next-webdriver.ts | 48 +------------- 4 files changed, 75 insertions(+), 51 deletions(-) diff --git a/test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts b/test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts index cdbda05745b21..ce05f1f114bcf 100644 --- a/test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts +++ b/test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts @@ -174,8 +174,9 @@ describe('parallel-routes-revalidation', () => { // go back to the previous page await browser.back() - // reload the page, which will cause the router to no longer have cache nodes - await browser.refresh() + // reload the page, which will cause the router to no longer have cache + // nodes. make sure the page is hydrated before going forward. + await browser.refresh({ waitHydration: true }) // go forward, this will trigger a lazy fetch for the missing data, and should restore the detail page await browser.forward() diff --git a/test/lib/browsers/base.ts b/test/lib/browsers/base.ts index 77ab9f3751457..3b364f5c4b6bd 100644 --- a/test/lib/browsers/base.ts +++ b/test/lib/browsers/base.ts @@ -85,7 +85,10 @@ export abstract class BrowserInterface { * Use browsers `go forward` functionality. Inverse of back. */ abstract forward(options?: any): BrowserInterface & Promise - abstract refresh(): BrowserInterface & Promise + abstract refresh(options?: { + waitHydration?: boolean + retryWaitHydration?: boolean + }): BrowserInterface & Promise abstract setDimensions(opts: { height: number width: number @@ -129,4 +132,7 @@ export abstract class BrowserInterface { abstract websocketFrames(): Promise abstract url(): Promise abstract waitForIdleNetwork(): Promise + abstract waitForHydration(options?: { + retryWaitHydration?: boolean + }): Promise } diff --git a/test/lib/browsers/playwright.ts b/test/lib/browsers/playwright.ts index d0a19e75d97eb..8d37634c8a096 100644 --- a/test/lib/browsers/playwright.ts +++ b/test/lib/browsers/playwright.ts @@ -302,9 +302,19 @@ export class Playwright extends BrowserInterface { await page.goForward(options) }) } - refresh() { + refresh({ + waitHydration, + retryWaitHydration, + }: { + waitHydration?: boolean + retryWaitHydration?: boolean + } = {}) { return this.chain(async () => { await page.reload() + + if (waitHydration) { + this.waitForHydration({ retryWaitHydration }) + } }) } setDimensions({ width, height }: { height: number; width: number }) { @@ -518,6 +528,59 @@ export class Playwright extends BrowserInterface { }) } + async waitForHydration({ + retryWaitHydration = false, + }: { retryWaitHydration?: boolean } = {}): Promise { + const fullUrl = page.url() + console.log(`\n> Waiting hydration for ${fullUrl}\n`) + + const checkHydrated = async () => { + await this.evalAsync(function () { + var callback = arguments[arguments.length - 1] + + // if it's not a Next.js app return + if ( + !document.documentElement.innerHTML.includes('__NEXT_DATA__') && + // @ts-ignore next exists on window if it's a Next.js page. + typeof ((window as any).next && (window as any).next.version) === + 'undefined' + ) { + console.log('Not a next.js page, resolving hydrate check') + callback() + } + + // TODO: should we also ensure router.isReady is true + // by default before resolving? + if ((window as any).__NEXT_HYDRATED) { + console.log('Next.js page already hydrated') + callback() + } else { + var timeout = setTimeout(callback, 10 * 1000) + ;(window as any).__NEXT_HYDRATED_CB = function () { + clearTimeout(timeout) + console.log('Next.js hydrate callback fired') + callback() + } + } + }) + } + + try { + await checkHydrated() + } catch (err) { + if (retryWaitHydration) { + // re-try in case the page reloaded during check + await new Promise((resolve) => setTimeout(resolve, 2000)) + await checkHydrated() + } else { + console.error('failed to check hydration') + throw err + } + } + + console.log(`\n> Hydration complete for ${fullUrl}\n`) + } + locateRedbox(): Locator { return page.locator( 'nextjs-portal [aria-labelledby="nextjs__container_errors_label"]' diff --git a/test/lib/next-webdriver.ts b/test/lib/next-webdriver.ts index 091f44551f366..17bfa1bc67016 100644 --- a/test/lib/next-webdriver.ts +++ b/test/lib/next-webdriver.ts @@ -145,53 +145,7 @@ export default async function webdriver( // Wait for application to hydrate if (waitHydration) { - console.log(`\n> Waiting hydration for ${fullUrl}\n`) - - const checkHydrated = async () => { - await browser.evalAsync(function () { - var callback = arguments[arguments.length - 1] - - // if it's not a Next.js app return - if ( - !document.documentElement.innerHTML.includes('__NEXT_DATA__') && - // @ts-ignore next exists on window if it's a Next.js page. - typeof ((window as any).next && (window as any).next.version) === - 'undefined' - ) { - console.log('Not a next.js page, resolving hydrate check') - callback() - } - - // TODO: should we also ensure router.isReady is true - // by default before resolving? - if ((window as any).__NEXT_HYDRATED) { - console.log('Next.js page already hydrated') - callback() - } else { - var timeout = setTimeout(callback, 10 * 1000) - ;(window as any).__NEXT_HYDRATED_CB = function () { - clearTimeout(timeout) - console.log('Next.js hydrate callback fired') - callback() - } - } - }) - } - - try { - await checkHydrated() - } catch (err) { - if (retryWaitHydration) { - // re-try in case the page reloaded during check - await new Promise((resolve) => setTimeout(resolve, 2000)) - await checkHydrated() - } else { - console.error('failed to check hydration') - throw err - } - } - - console.log(`\n> Hydration complete for ${fullUrl}\n`) + await browser.waitForHydration({ retryWaitHydration }) } // This is a temporary workaround for turbopack starting watching too late. From 220a7cf888fbd6aeaa25ff0e61f7f168d529b077 Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Thu, 27 Feb 2025 14:16:37 +0100 Subject: [PATCH 2/3] Revert "Unflake `parallel-routes-revalidation` test" This reverts commit 7c3ef83fb6b19fc1e4bbfe5ad21804cbdb452f6c. --- .../parallel-routes-revalidation.test.ts | 5 +- test/lib/browsers/base.ts | 8 +-- test/lib/browsers/playwright.ts | 65 +------------------ test/lib/next-webdriver.ts | 48 +++++++++++++- 4 files changed, 51 insertions(+), 75 deletions(-) diff --git a/test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts b/test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts index ce05f1f114bcf..cdbda05745b21 100644 --- a/test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts +++ b/test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts @@ -174,9 +174,8 @@ describe('parallel-routes-revalidation', () => { // go back to the previous page await browser.back() - // reload the page, which will cause the router to no longer have cache - // nodes. make sure the page is hydrated before going forward. - await browser.refresh({ waitHydration: true }) + // reload the page, which will cause the router to no longer have cache nodes + await browser.refresh() // go forward, this will trigger a lazy fetch for the missing data, and should restore the detail page await browser.forward() diff --git a/test/lib/browsers/base.ts b/test/lib/browsers/base.ts index 3b364f5c4b6bd..77ab9f3751457 100644 --- a/test/lib/browsers/base.ts +++ b/test/lib/browsers/base.ts @@ -85,10 +85,7 @@ export abstract class BrowserInterface { * Use browsers `go forward` functionality. Inverse of back. */ abstract forward(options?: any): BrowserInterface & Promise - abstract refresh(options?: { - waitHydration?: boolean - retryWaitHydration?: boolean - }): BrowserInterface & Promise + abstract refresh(): BrowserInterface & Promise abstract setDimensions(opts: { height: number width: number @@ -132,7 +129,4 @@ export abstract class BrowserInterface { abstract websocketFrames(): Promise abstract url(): Promise abstract waitForIdleNetwork(): Promise - abstract waitForHydration(options?: { - retryWaitHydration?: boolean - }): Promise } diff --git a/test/lib/browsers/playwright.ts b/test/lib/browsers/playwright.ts index 8d37634c8a096..d0a19e75d97eb 100644 --- a/test/lib/browsers/playwright.ts +++ b/test/lib/browsers/playwright.ts @@ -302,19 +302,9 @@ export class Playwright extends BrowserInterface { await page.goForward(options) }) } - refresh({ - waitHydration, - retryWaitHydration, - }: { - waitHydration?: boolean - retryWaitHydration?: boolean - } = {}) { + refresh() { return this.chain(async () => { await page.reload() - - if (waitHydration) { - this.waitForHydration({ retryWaitHydration }) - } }) } setDimensions({ width, height }: { height: number; width: number }) { @@ -528,59 +518,6 @@ export class Playwright extends BrowserInterface { }) } - async waitForHydration({ - retryWaitHydration = false, - }: { retryWaitHydration?: boolean } = {}): Promise { - const fullUrl = page.url() - console.log(`\n> Waiting hydration for ${fullUrl}\n`) - - const checkHydrated = async () => { - await this.evalAsync(function () { - var callback = arguments[arguments.length - 1] - - // if it's not a Next.js app return - if ( - !document.documentElement.innerHTML.includes('__NEXT_DATA__') && - // @ts-ignore next exists on window if it's a Next.js page. - typeof ((window as any).next && (window as any).next.version) === - 'undefined' - ) { - console.log('Not a next.js page, resolving hydrate check') - callback() - } - - // TODO: should we also ensure router.isReady is true - // by default before resolving? - if ((window as any).__NEXT_HYDRATED) { - console.log('Next.js page already hydrated') - callback() - } else { - var timeout = setTimeout(callback, 10 * 1000) - ;(window as any).__NEXT_HYDRATED_CB = function () { - clearTimeout(timeout) - console.log('Next.js hydrate callback fired') - callback() - } - } - }) - } - - try { - await checkHydrated() - } catch (err) { - if (retryWaitHydration) { - // re-try in case the page reloaded during check - await new Promise((resolve) => setTimeout(resolve, 2000)) - await checkHydrated() - } else { - console.error('failed to check hydration') - throw err - } - } - - console.log(`\n> Hydration complete for ${fullUrl}\n`) - } - locateRedbox(): Locator { return page.locator( 'nextjs-portal [aria-labelledby="nextjs__container_errors_label"]' diff --git a/test/lib/next-webdriver.ts b/test/lib/next-webdriver.ts index 17bfa1bc67016..091f44551f366 100644 --- a/test/lib/next-webdriver.ts +++ b/test/lib/next-webdriver.ts @@ -145,7 +145,53 @@ export default async function webdriver( // Wait for application to hydrate if (waitHydration) { - await browser.waitForHydration({ retryWaitHydration }) + console.log(`\n> Waiting hydration for ${fullUrl}\n`) + + const checkHydrated = async () => { + await browser.evalAsync(function () { + var callback = arguments[arguments.length - 1] + + // if it's not a Next.js app return + if ( + !document.documentElement.innerHTML.includes('__NEXT_DATA__') && + // @ts-ignore next exists on window if it's a Next.js page. + typeof ((window as any).next && (window as any).next.version) === + 'undefined' + ) { + console.log('Not a next.js page, resolving hydrate check') + callback() + } + + // TODO: should we also ensure router.isReady is true + // by default before resolving? + if ((window as any).__NEXT_HYDRATED) { + console.log('Next.js page already hydrated') + callback() + } else { + var timeout = setTimeout(callback, 10 * 1000) + ;(window as any).__NEXT_HYDRATED_CB = function () { + clearTimeout(timeout) + console.log('Next.js hydrate callback fired') + callback() + } + } + }) + } + + try { + await checkHydrated() + } catch (err) { + if (retryWaitHydration) { + // re-try in case the page reloaded during check + await new Promise((resolve) => setTimeout(resolve, 2000)) + await checkHydrated() + } else { + console.error('failed to check hydration') + throw err + } + } + + console.log(`\n> Hydration complete for ${fullUrl}\n`) } // This is a temporary workaround for turbopack starting watching too late. From 4cb56b7a033497a991f96ad9f7ed6ceaeaddb7b4 Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Thu, 27 Feb 2025 14:20:24 +0100 Subject: [PATCH 3/3] =?UTF-8?q?(=E2=95=AF=C2=B0=E2=96=A1=C2=B0)=E2=95=AF?= =?UTF-8?q?=EF=B8=B5=20=E2=94=BB=E2=94=81=E2=94=BB=20wait=20for=20idle=20n?= =?UTF-8?q?etwork=20instead=3F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../parallel-routes-revalidation.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts b/test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts index cdbda05745b21..43dcda4961646 100644 --- a/test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts +++ b/test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts @@ -176,6 +176,7 @@ describe('parallel-routes-revalidation', () => { // reload the page, which will cause the router to no longer have cache nodes await browser.refresh() + await browser.waitForIdleNetwork() // go forward, this will trigger a lazy fetch for the missing data, and should restore the detail page await browser.forward()