Global CSS not removed between app router pages

controversial commented 10 months ago

Link to the code that reproduces this issue

https://codesandbox.io/p/sandbox/loving-roman-scc0uh

To Reproduce

Visit the homepage
Click the link to “page 2”
Click the link back to the homepage

Current vs. Expected behavior

Currently, the global stylesheet added by page 2 is not removed once page 2 is unloaded.

I expect the global styles imported on page 2 to be removed when the user navigates back to the home page (which does not import those styles), and did not display those styles before page 2 was loaded.

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
Binaries:
  Node: 18.18.2
  npm: 9.8.1
  Yarn: 1.22.19
  pnpm: N/A
Relevant Packages:
  next: 14.0.4-canary.2
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 4.9.4
Next.js Config:
  output: N/A

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

App Router, Routing (next/router, next/navigation, next/link)

Additional context

Reopening as a duplicate of #46817, which was closed as “please verify canary” despite a next@canary reproduction

pacocoursey commented 7 months ago

Still happening on next@14.1.0.

Edit by maintainer bot: Comment was automatically minimized because it was considered unhelpful. (If you think this was by mistake, let us know). Please only comment if it adds context to the issue. If you want to express that you have the same problem, use the upvote 👍 on the issue description or subscribe to the issue for updates. Thanks!

abencun-symphony commented 7 months ago

We're also observing this behavior on Next 14.0.1

nahtnam commented 5 months ago

We are having the same problem between two route groups that import different reset.css files

controversial commented 4 months ago

I’ve updated the next.js version in the reproduction to 14.3.0-canary.9 and confirmed the issue still exists.

samcx commented 2 months ago

Hi everyone! Going through a lot of CSS-related issues as of recent—

To clarify, this is the expected behavior—if you navigate between Pages in App Router, both stylesheets will be active (I believe done for performance reasons). You will need to write CSS in a way that doesn't conflict, e.g., by using classNames that don't conflict. One such was is using CSS Modules.

I believe we need to update our documentation here to make that clearer.

Let me know if that makes sense! cc @controversial

controversial commented 2 months ago

I believe done for performance reasons—you don't want to continually re-request a Page when going back and forth

This explanation doesn’t make any sense to me—why would there be a performance implication? Removing and re-adding global stylesheets from the DOM that are imported on only one page wouldn't need to involve network requests.

I don't think it can be the “expected behavior” can be that page B is subject to global styles that page A imports (but page B does not)—this causes page B to display differently depending on whether or not the user has visited page A previously.

This behavior makes page components to behave in brittle, unpredictable ways, and is contrary to a reasonable developer’s expectation.

@samcx What leads you to think this is the expected behavior? And why would removing unloaded pages’ global stylesheets from the DOM require “re-requesting” anything?

samcx commented 2 months ago

@controversial You're right about the re-requesting not being a legitimate reason—let me ask the team again why such is the case!

gnoff commented 2 months ago

Hi @controversial Next.js does not support global styles unless those styles are actually global (meaning apply to all pages and can live for the lifetime of the application). Earlier we actually enforced no global styles at all but added support for global styles because of the times that you legitimately want to use styles globally. However for page specific styles you need to use CSS Modules (or another styling technique).

One of the reasons we do not support global styles is the style ordering issue. Even with modules it can be tricky to ensure that stylehsheets always load in the correct order but this problem is compounded by global styles when any stylesheet can select any element within the document.

Another reason is that Next.js uses React's builtin support for stylesheets to integrate with Suspense. React does not remove stylesheets that it manages the moment you no longer render them because there are edge cases related to SSR and hydration where you might hydrate part of the page and then stop rendering a stylesheet and React cannot know whether the unhydrated parts of the page still depend on that stylesheet. At the moment React does not do any stylesheet garbage collection though we do intend in the future to add this. However the removal of unused stylesheets won't be guaranteed to happen during the commit that no longer renders a particular stylesheet. So when you navigate from page A to page B we won't necessarily immediately remove the stylesheets that were loaded with page A only.

One thing you do get for this however is coordination of UI reveals with when stylesheets are loaded so Next.js can start rendering the next page and ensure all the stylesheets are ready while either staying on the prior screen or showing an instant loading state for the next screen.

It can feel maybe limiting that global styles are not unconditionally supported but that's the current reality and we have no plans to change this in the future. Have you explored using CSS Modules to style your applciation?

controversial commented 2 months ago

Hi @gnoff, thanks for the explainer!

I do already style my application using CSS Modules, but here’s my use case for why adding and removing global CSS from a page/layout is important (even while using CSS modules):

There are some CSS styles which are required to be set globally:
- In many browsers, the background-color or color-scheme set on the body element controls the color of the browser’s scroll bar, or the color of the browser chrome (in desktop Safari)
- The basis for the rem CSS unit can only be changed by setting a font-size on the :root element
My app has separate route groups within which different visual styles are applied. The layouts in these route groups need to be able to set different values for these “base” styles: the background-color and color of the body element, as well as some CSS variables that are applied globally, and the root font size.

This would be trivially achievable with proper support for importing global CSS from layouts (the layout for each route group could just import a separate theme.scss file)—except that in the current implementation, the global CSS files from the previous route group will never be removed.

I’m sure you can generalize this example to other cases where it would be useful to be able to have different global styles within different sections of an app.

I very much appreciate the deep dive on how & why React manages stylesheets, that context is genuinely super valuable for understanding the background behind this issue. I also now see the documentation of “caveats” on the documentation for React 19’s stylesheet support, which I hadn’t seen before.

It feels strange to me for the React reconciler to leave any elements in the DOM that are no longer part of the virtual DOM, but the edge case you mentioned

you might hydrate part of the page and then stop rendering a stylesheet and React cannot know whether the unhydrated parts of the page still depend on that stylesheet

is something I hadn’t considered.

You’re obviously much higher-context than I am on the constraints that exist within the React renderer / hydration process, but why couldn’t a mechanism exist like the following:

React assigns a stable (between client/server) internal ID to each <style>
During both SSR and CSR, React tracks, for each deduplicated/<head>-hoisted stylesheet in the DOM, the set of all <style> tag IDs that caused it to render. Each ID is added to the set at the point when React discovers + makes the decision decide to hoist that stylesheet, either on the client or the server. This could be as simple as a data-dependents attribute written on the <head> stylesheet, or it could be tracked using data structures in JS-land.
When a <style> tag is removed from a component in a hydrated part of the DOM, React removes its ID from the the set of dependents tracked for the stylesheet in the <head>
Remove a stylesheet from the <head> when its set of dependents becomes empty

Then, when React sees that a <style> is removed from a component in the hydrated part of the DOM, React would know whether an unhydrated part of the page depends on that stylesheet based on whether its set of dependent IDs is non-empty.

samcx commented 1 month ago

This would be trivially achievable with proper support for importing global CSS from layouts (the layout for each route group could just import a separate theme.scss file)—except that in the current implementation, the global CSS files from the previous route group will never be removed.

@controversial This shouldn't be the case with multiple Route Groups since it is supposed to a Full Page Reload (Mentioned in the Good to know at the end of the section). Could be a 🐛—can you confirm this is the case on the latest canary?

controversial commented 1 month ago

I’m not necessarily talking about top-level route groups with separate root layouts, I’m talking about route groups generally (including those that aren’t at the top level of the app)

lifeike commented 1 month ago

hi next, thx for your hard work on nextjs development, its super convenient as a full-stack framework. however, if you can make more convenient by following common sense would be better which each global css won't affect each other route group. just a kind suggestion.

I have (marketing) folder which import marketing.css, and the :root tailwind styling always appear on /admin layout which kind confused to me.

please consider scenario. thx

vercel / next.js