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

Link Component does not prefetch on network recovery. #69999

Open
refirst11 opened this issue Sep 11, 2024 · 0 comments
Open

Link Component does not prefetch on network recovery. #69999

refirst11 opened this issue Sep 11, 2024 · 0 comments
Labels
bug Issue was opened via the bug report template. create-next-app Related to our CLI tool for quickly starting a new Next.js application. Performance Anything with regards to Next.js performance.

Comments

@refirst11
Copy link

Link to the code that reproduces this issue

https://github.com/refirst11/reproduction-app

To Reproduce

Sometimes unnecessary mounts are run.
And canary release e2e testing is special.

Let's say the start of the route is "/" and it goes offline in this state.
"/a", "/b", and "/c" are in the viewport at this point, so there will be a smooth transition when transitioning to them.
And it is not possible to transition to links "/d", "/e", and "/f" on pages a, b, and c at this point, but let's say the network is restored, for example in a subway or train tunnel. In that case, when transitioning to pages d, e, and f, a network request will be sent and the page will be reloaded as in the a tag.
I call this mounting on recovery in Next.js.

However, since prefetch only needs to hit when the network restarts,
I thought it would be fixed by adding the following to the dependencies in React.useEffect.

In my environment, when I tried e2e testing next.js,
pnpm test-dev test/e2e/app-dir/app-prefetch/prefetching.test.ts

It's possible that I overlooked this, but all of the test code in the describe block of prefetching.test.ts passes (even if it fails), so I decided to write this as an issue here.

Current vs. Expected behavior

When the network becomes online, link components in the viewport are prefetched.

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 24.0.0: Sat Jul 13 00:56:26 PDT 2024; root:xnu-11215.0.165.0.4~50/RELEASE_ARM64_T8103
  Available memory (MB): 16384
  Available CPU cores: 8
Binaries:
  Node: 22.8.0
  npm: 10.8.2
  Yarn: N/A
  pnpm: 9.6.0
Relevant Packages:
  next: 14.2.5 // There is a newer version (14.2.9) available, upgrade recommended! 
  eslint-config-next: 13.5.6
  react: 18.3.1
  react-dom: 18.3.1
  typescript: 5.5.4
Next.js Config:
  output: N/A

Which area(s) are affected? (Select all that apply)

create-next-app, Performance

Which stage(s) are affected? (Select all that apply)

next dev (local), next start (local), Vercel (Deployed), Other (Deployed)

Additional context

Deploy is with vercel and next start.
I've been using Next.js since version 11, but even back then there was no prefetching during network restarts.

I want to improve

// use-network.ts
import { useEffect, useState } from 'react'

export function useNetwork() {
  const isClient =
    typeof window !== 'undefined' && typeof navigator.onLine === 'boolean'

  const readOnlineStatus = isClient && window.navigator.onLine
  const [isOnline, setIsOnline] = useState(readOnlineStatus)

  useEffect(() => {
    function updateOnlineStatus() {
      setIsOnline(window.navigator.onLine)
    }

    window.addEventListener('online', updateOnlineStatus)
    window.addEventListener('offline', updateOnlineStatus)

    return () => {
      window.removeEventListener('online', updateOnlineStatus)
      window.removeEventListener('offline', updateOnlineStatus)
    }
  }, [])
  return isOnline
}
// link.tsx
...
    const [setIntersectionRef, isVisible, resetVisible] = useIntersection({
      rootMargin: '200px',
    })

    const isOnline = useNetwork()
    
    React.useEffect(() => {
      // in dev, we only prefetch on hover to avoid wasting resources as the prefetch will trigger compiling the page.
      if (process.env.NODE_ENV !== 'production') {
        return
      }

      if (!router) {
        return
      }

      // If we don't need to prefetch the URL, don't do prefetch.
      if (!isVisible || isOnline || !prefetchEnabled) {
        return
      }

      // Prefetch the URL.
      prefetch(
        router,
        href,
        as,
        { locale },
        {
          kind: appPrefetchKind,
        },
        isAppRouter
      )
    }, [
      as,
      href,
      isVisible,
      locale,
      prefetchEnabled,
      pagesRouter?.locale,
      router,
      isAppRouter,
      appPrefetchKind,
      isOnline,
    ])

e2e test case

describe('online/offline transitions', () => {
  it('should handle transition from offline to online correctly', async () => {
    const browser = await next.browser('/static-page')
    let requests = []

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

    // prefetch wait time
    await waitFor(1000)
    await browser.eval('navigator.onLine = false')

    // Link component "/" click
    // and request dashboard.
    await browser
      .elementByCss('#to-home')
      .click()
      .waitForElementByCss('#to-dashboard')

    await browser.eval('navigator.onLine = true')
    // prefetch wait time
    await waitFor(1000)
    expect(requests.filter((req) => req.includes('/dashboard')).length).toBe(0)

    await waitFor(1000)

    const before = Date.now()
    await browser
      .elementByCss('#to-dashboard')
      .click()
      .waitForElementByCss('#dashboard-layout')
    const after = Date.now()
    const timeToComplete = after - before

    // Ensure the dashboard page is prefetched
    expect(timeToComplete).toBeLessThan(20)
  })
})
@refirst11 refirst11 added the bug Issue was opened via the bug report template. label Sep 11, 2024
@github-actions github-actions bot added create-next-app Related to our CLI tool for quickly starting a new Next.js application. Performance Anything with regards to Next.js performance. labels Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue was opened via the bug report template. create-next-app Related to our CLI tool for quickly starting a new Next.js application. Performance Anything with regards to Next.js performance.
Projects
None yet
Development

No branches or pull requests

1 participant