next.js icon indicating copy to clipboard operation
next.js copied to clipboard

Missing `.dark` class when `notFound()` is invoked

Open loqusion opened this issue 2 years ago • 5 comments

Link to the code that reproduces this issue

https://github.com/loqusion/next-bug-class-removed

To Reproduce

  1. Start the application with pnpm run build && pnpm run start (bug can also be reproduced in development)
  2. Navigate to localhost:3000/notfound (page invokes notFound())
  3. Open developer tools to inspect HTML
  4. <html> tag does not have .dark class

Control group

  1. Navigate to localhost:3000/any (non-existent page)
  2. In production, <html> tag has class="dark" as expected

Current vs. Expected behavior

A <script> tag containing document.documentElement.classList.add('dark') is present in <head>, which should update the root document tag (<html>) to have the .dark class. This works on most pages, but not on pages where notFound() is called, even though the <script> is still present. (In the development build, any "404 not found" page also exhibits this bug.)

Verify canary release

  • [x] I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
  Platform: linux
  Arch: x64
  Version: #1 ZEN SMP PREEMPT_DYNAMIC Mon, 04 Dec 2023 00:28:58 +0000
Binaries:
  Node: 21.4.0
  npm: 10.2.5
  Yarn: 1.22.21
  pnpm: 8.12.0
Relevant Packages:
  next: 14.0.5-canary.4
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 5.1.3
Next.js Config:
  output: N/A

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

App Router

Additional context

The reproduction example is not very useful for real life applications, but the same bug occurs for a theme hydration script that is used to prevent theme flicker on initial page load:

try {
  if (localStorage.theme === 'dark' || (!('theme' in localStorage) && window.matchMedia('(prefers-color-scheme: dark)').matches)) {
    document.documentElement.classList.add('dark')
    document.querySelector('meta[name="theme-color"]').setAttribute('content', '#0B1120')
  } else {
    document.documentElement.classList.remove('dark')
  }
} catch (_) {}

The script is from tailwindcss.com (source). The bug does not appear on the site (possibly because it uses the pages router, or because it uses an older version of Next.js, or because it never invokes notFound()). However, the bug does occur on my own site where I use the same script (I use app router).

I've reproduced the issue locally, and via Vercel deployment, on both Firefox and Chromium.

loqusion avatar Dec 09 '23 16:12 loqusion

  1. The browser API call should only be in useEffect
  2. You're obviously doing shit.

m4x1m3-l4mb3rt avatar Dec 09 '23 20:12 m4x1m3-l4mb3rt

@maximlambov

  1. The browser API call should only be in useEffect
  2. You're obviously doing shit.
  1. The browser API call is being made in a <script> tag, not in the render function of a React client component. It needs to be done this way to avoid screen flicker, which you would know if you had read what I wrote.
  2. That was uncalled for.

loqusion avatar Dec 09 '23 21:12 loqusion

The markup's state created by inlined scripts is not a part of NextJS server/client component tree, so it can easily be lost on any change. I guess the closest thing to a solution here is using router.replace("/404") to retain the state of layout, assuming notFound() in there is supposed to emulate client-side transition and therefore you don't care about the status code of this "redirect".

GabenGar avatar Dec 16 '23 15:12 GabenGar

For this issue, I've found a couple of workarounds, each of which has its own trade-offs:

  • router.replace('/404') (from useRouter()) (thanks @GabenGar)
    • Has 404 status, but doesn't retain browser URL
    • Must be done in a client component, and requires client JavaScript
  • return <NotFound /> (imported from custom not-found component)
    • Retains the browser URL, but doesn't have 404 status
    • Works without client JavaScript
    • Retains nested layout UI, which may or may not be what you want
      • If done in a descendant of the main page.js component, all of the other page.js UI will also be retained
  • notFound() (for comparison)
    • Has 404 status, and retains the browser URL
    • Doesn't need to be called directly in a React component
    • Triggers Error Boundaries
      • The .dark class also disappears in error boundaries after triggering notFound()
    • Can be worked around with an additional <Script> (with strategy="afterInteractive") that does backup theme hydration, though this can cause theme flicker on pages where the .dark class disappears and I'm not sure how consistent this workaround is

It would be nice if there were an officially supported way to do theme hydration, though I'm not sure how much development effort that would require or if it would be worth it.

loqusion avatar Dec 17 '23 00:12 loqusion

This issue has been automatically marked as stale due to two years of inactivity. It will be closed in 7 days unless there’s further input. If you believe this issue is still relevant, please leave a comment or provide updated details. Thank you.

nextjs-bot avatar Jun 14 '25 23:06 nextjs-bot

This issue has been automatically closed due to two years of inactivity. If you’re still experiencing a similar problem or have additional details to share, please open a new issue following our current issue template. Your updated report helps us investigate and address concerns more efficiently. Thank you for your understanding!

nextjs-bot avatar Jun 22 '25 23:06 nextjs-bot

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

github-actions[bot] avatar Jul 07 '25 00:07 github-actions[bot]