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

Hash change re-renders the entire app, _app, page and components involved #34729

Open
1 task done
gurkerl83 opened this issue Feb 23, 2022 · 4 comments
Open
1 task done
Labels
bug Issue was opened via the bug report template. Linking and Navigating Related to Next.js linking (e.g., <Link>) and navigation.

Comments

@gurkerl83
Copy link
Contributor

gurkerl83 commented Feb 23, 2022

Verify canary release

  • I verified that the issue exists in Next.js canary release

Provide environment information

Operating System:
Platform: darwin
Arch: x64
Version: Darwin Kernel Version 21.3.0: Wed Jan 5 21:37:58 PST 2022; root:xnu-8019.80.24~20/RELEASE_X86_64
Binaries:
Node: 16.13.0
npm: 8.1.0
Yarn: 3.2.0
pnpm: N/A
Relevant packages:
next: 12.1.0
react: 18.0.0-rc.0
react-dom: 18.0.0-rc.0

What browser are you using? (if relevant)

Chrome 101.0.4899.0 same in FF and Safari

How are you deploying your application? (if relevant)

next dev, next start

Describe the Bug

Navigation to anchors should not cause any re-render; however, changing the hash through NextJs building blocks Link and Router does cause a re-render.

Re-rendering an entire app from _app and the respective page with the page's content can get quite substantial.

E.g., in our case, MDX and quite large React components get re-rendered. On the other hand, the re-rendering leads us to identify a hard-to-track problem in loading MDX components, but this is off-topic to the current issue.

Looking at the sources, it seems the function of the router does not lead to a state change, which causes a re-rendering because the operations target Html primitives primarily. Logging the hashChangeStart and hashChangeComplete events seems something is causing a re-render afterward; maybe a new page-props object gets created, but this is just a vague assumption.

Another thing we see is when triggering an identical hash change several times using NextJs Link and Router primitives; the re-rendering occurs multiple times, one for each interaction. In this case, the browser history seems to be intact; a second click on the same link does not cause a push of another state to browser history; the browser's back button works; in any case, a re-render of the app gets executed.

Expected Behavior

Do not perform re-rendering if it is not needed.

To Reproduce

The sources of the reproducer are here. https://github.com/gurkerl83/NextJs-Hash-Bug

  1. Clone the repo
  2. Install dependencies
  • yarn
  1. Run development respectively production
  • yarn dev
  • yarn start
  1. Follow the instructions in the apps index page and see the console / http://localhost:3000

The link to the deployed version
https://next-js-hash-bug.vercel.app

Thx!

@gurkerl83 gurkerl83 added the bug Issue was opened via the bug report template. label Feb 23, 2022
@balazsorban44 balazsorban44 added the Linking and Navigating Related to Next.js linking (e.g., <Link>) and navigation. label Mar 3, 2022
@gurkerl83
Copy link
Contributor Author

Memoizing the Component generated in the _app can stop the constant re-rendering on hash change; this also works with dynamic pages.

Maybe a memoizing option using a subset of the properties of the router object (locale and slug) as dependables can be integrated directly in NextJs or highlighted in the docs.

in _app

const {
   locale,
   query: { slug }
} = useRouter();

const component = useMemo(() => {
   return <Component {...pageProps} />;
}, [locale, slug]);

return <>{component}</>

Hope this helps!

Thx!

@gurkerl83
Copy link
Contributor Author

gurkerl83 commented May 25, 2022

The current implementation of useRouter returns the entire context object to the consumer.

export function useRouter(): NextRouter {
  return React.useContext(RouterContext)
}

NextJs does not export the router context directly. Accessing the context is only possible using the useRouter hook. Exporting the context directly would allow us to build a custom useRouter hook with memo capabilities in user-land.

Part of the context is asPath which includes the current hash, this seems to cause the problem described in the opening.
It is not ideal, especially when you rely on hashes; each time a hash change happens, a re-render occurs, see here, e.g., in _app, but this is also triggering re-renders in custom components which is more severe from a performance standpoint.

Thx!

@dmwallace
Copy link

Second this. Any component that uses the useRouter hook is forced to re-render on shallow route changes which is not ideal for storing application state in the url

@phips28
Copy link

phips28 commented Feb 2, 2024

same for us.

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. Linking and Navigating Related to Next.js linking (e.g., <Link>) and navigation.
Projects
None yet
Development

No branches or pull requests

4 participants