vercel / next.js

The React Framework
https://nextjs.org
MIT License
126.77k stars 26.95k forks source link

[NEXT-1308] Css is imported multiple times and out of order in /app dir #51030

Closed ssijak closed 2 months ago

ssijak commented 1 year ago

Verify canary release

Provide environment information

Operating System:
      Platform: darwin
      Arch: arm64
      Version: Darwin Kernel Version 22.5.0: Mon Apr 24 20:52:24 PDT 2023; root:xnu-8796.121.2~5/RELEASE_ARM64_T6000
    Binaries:
      Node: 18.12.1
      npm: 8.19.2
      Yarn: 1.22.19
      pnpm: 7.18.1
    Relevant packages:
      next: 13.4.5-canary.9
      eslint-config-next: N/A
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 5.1.3

Which area(s) of Next.js are affected? (leave empty if unsure)

App directory (appDir: true)

Link to the code that reproduces this issue or a replay of the bug

https://github.com/ssijak/next-css-issue-not-working-simple

To Reproduce

Just start the app and check the styling on the buttons. Styles are imported multiple times wherever Button was used (page and layout) and order is also not deterministic, so it can be imported in different order on different app runs.

This is another/same simple repro difference is just that it uses turbo and transpiles the UI lib, I started with that but figured that the issue is happening without it too https://github.com/ssijak/next-css-issue-not-working

Describe the Bug

-Same styles are imported multiple times -Order of imports is not deterministic

Screenshot: https://share.cleanshot.com/nq35j7vh

Expected Behavior

Same styles should be imported once. Starting the app multiple times should not produce different results (ordering of CSS, impacting specificity)

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

From SyncLinear.com | NEXT-1308

ssijak commented 1 year ago

This PR https://github.com/vercel/next.js/pull/50406 seems like it should have fixed this issue, but I see the problem in my repro repositories

ssijak commented 1 year ago

CSS modules seem to be working ok. Removed vanilla-extract from this repro, and imported the same CSS from CSS module in the page and layout, and set it on the <button>, and it imports the CSS once.

Edit: it works like that if both the page and layout are RSC. When I use use client directive on the page it imports the same CSS twice https://share.cleanshot.com/HjhMJhCy

igordanchenko commented 1 year ago

This issue is still present in the latest canary (13.4.7-canary.1)

1) Same styles are imported multiple times 2) The order of imports is not deterministic

Here is a minimal repro with easy-to-reproduce test cases.

https://codesandbox.io/p/sandbox/next-51030-hlj722?file=%2Fapp%2Fpage.tsx%3A1%2C1

tinleym commented 1 year ago

The current order of imports wreaks havoc when CSS Cascade Layers are being used. The defined layer precedence that belongs in the root layout file gets read after the application of layers in child components. This causes the defined layer precedence to be ignored in favor of something random (https://css-tricks.com/css-cascade-layers/#aa-establishing-a-layer-order), and can make routes unusable.

This issue surfaced for me when updating up from 13.4.4 to 13.4.5 or 13.4.6.

ddoan commented 1 year ago

One workaround I'm currently using is to make the children CSS more specific: button.button instead of .button or div.something instead of .something.

shuding commented 1 year ago

For https://codesandbox.io/p/sandbox/next-51030-hlj722?file=%2Fapp%2Fpage.tsx%3A1%2C1, it becomes very tricky if your CSS code itself has conflicts and some behaviors are undefined. For example when a dynamic component has loaded, its CSS might cause side effects to other parts of the page, which is expected.

For example, if you have a layout like this:

import style1 from 'a.module.css'
import style2 from 'b.module.css'

<button className={style1.btn + ' ' + style2.btn}>

The styles that imports later will override previous ones, so b.module.css will take precedence. However, if we have a page component that imports a.module.css again, it will take precedence over b.module.css again because page's styles have higher priority than the layout's. This is also a cause of duplicated styles.

In general, we recommend you to use CSS @layer, or try to make CSS selectors as pure and specific as possible.

igordanchenko commented 1 year ago

@shuding, I appreciate you taking the time to look into this example.

it becomes very tricky if your CSS code itself has conflicts and some behaviors are undefined.

I wouldn't call styles override a conflict. Overriding base component styles in a descendant component is a pretty common technique. Isn't that what "C" in CSS stands for?

For example when a dynamic component has loaded, its CSS might cause side effects to other parts of the page, which is expected.

The above example is a module-scoped CSS that doesn't have any global side-effects.

The styles that imports later will override previous ones, so b.module.css will take precedence.

Yes, you are absolutely correct, and that's the desired behavior that we are unfortunately not able to achieve under the app router.

However, if we have a page component that imports a.module.css again, it will take precedence over b.module.css again because page's styles have higher priority than the layout's. This is also a cause of duplicated styles.

In theory, yes. But it's not the case in the above example, isn't it?

tinleym commented 1 year ago

@shuding if you'll please read my comment below, @layer is not working.

The indeterministic order of imports wreaks havoc when CSS Cascade Layers are being used. The defined layer precedence that belongs in the root layout file gets read after the application of layers in child components. This causes the defined layer precedence to be ignored in favor of something random (https://css-tricks.com/css-cascade-layers/#aa-establishing-a-layer-order), and can make routes unusable.

This issue surfaced for me when updating up from 13.4.4 to 13.4.5 or 13.4.6.

shuding commented 1 year ago

Thanks! I'll go through all the reproductions and think about an improvement in the next couple of days!

I understand that the confusion caused by the new architecture. In general scoped CSS itself doesn't have conflicts, but the order and duplication might cause the styles to conflict (override each other). I'll give you an example:

layout:

boatilus commented 1 year ago

That said, we still have some ideas to improve it. Like changing the CSS modules hashing algorithm to make it base on the layer, or deduplicate styles that are already in its parent layer.

If I understand the latter suggestion correctly, this seems to me like the right approach. If a build system produces hashed rule/class names based on the rule contents (e.g., ._129ac00b) in layout.css, it'd be unnecessary to duplicate that rule in page.css, and that duplication causes the unexpected precedence fighting. The basic idea here would be if (selector in layout.css) strip from page.css. Although I say that with that big caveat that I don't know the implications when overriding a rule generated in page.css that's intentional and how to signify that.

To your point, if there's perfect purity, this won't be a problem, but it's also reasonably unrealistic that there wouldn't be things like rules applied to descendent selectors (and probably some non-negigible number of other scenarios) on class names that have a high likelihood of their precedence being trumped by the same properties in a rule duplicated in page.css.

sleepdotexe commented 1 year ago

I'm also encountering the same behaviour.

For my use case, I'm creating standard UI/layout components that I want to use across multiple pages and components – these components have *.module.css styles. This allows me to add default custom stylings to these UI components and only load them if the component is used on the page. I also allow an additional className to be added for additional styling at a per-component level.

The problem is, as mentioned above, changing between routes/different app runs can result in different CSS load orders in addition to a huge amount of duplicate CSS.

For example:

// components/layout/Section.tsx

import styles from '../../styles/layout/Section.module.css';

interface Props extends React.ComponentProps<'section'> {
    fullWidth?: boolean;
}

export default function Section(props: Props) {
    const { fullWidth, ...remainingProps } = props;
    return (
        <section
            {...remainingProps}
            className={[styles['section'], props.className].join(' ')}
            data-fullwidth={fullWidth}
        >
            {props.children}
        </section>
    );
}

Navigating from Page A, to Page B, back to Page A cause Page A to display differently the second time round. Refreshing manually causes Page A to revert to its original layout.

For now, as a partial fix, I have managed to use css @layer to lower the CSS specificity of all the UI components' styles, however it still results in large amounts of duplicate css being added to each page, especially if the UI component is used frequently (such as a custom button).

Would be interested to know if there is a better way to go about this. I can provide more detailed examples/reproductions if necessary.

jep-a commented 1 year ago

@tinleym

The current order of imports wreaks havoc when CSS Cascade Layers are being used. The defined layer precedence that belongs in the root layout file gets read after the application of layers in child components. This causes the defined layer precedence to be ignored in favor of something random (https://css-tricks.com/css-cascade-layers/#aa-establishing-a-layer-order), and can make routes unusable.

This issue surfaced for me when updating up from 13.4.4 to 13.4.5 or 13.4.6.

Have a quick hack fix for the css layer order here https://github.com/vercel/next.js/issues/16630#issuecomment-1644918877

katiasmet-93 commented 1 year ago

Is there already a solution for css that is loaded multiple times if you use the "use client" mode? I use css modules and have f.e. several buttons or inputs on 1 page. The css (modules) are loaded multiple times in version 13.4.19.

monolithed commented 1 year ago

Is there a way to consolidate all styles into one file, so that the minifier automatically removes duplicates? I can't estimate the scale of the issue in kilobytes, but I see that each of my components (a total of 31) duplicates styles three times, and this is in production.

Screenshot 2023-09-05 at 01 37 57
boatilus commented 1 year ago

I just ended up writing a simple Webpack plugin that removes duplicate rules by their class names (which isn't a good idea in pratice, but has worked well enough for my needs). I've only tested this with CSS generated with Vanilla Extract, and it doesn't support route groups, but someone may find it useful.

Stillonov commented 1 year ago

I have the same problems and it's awful. My app is very fragile. It happens when I navigate through pages containing common components.

Screenshot 2023-09-08 at 01 24 15
katiasmet-93 commented 1 year ago

Maybe not the best solution for now, but converting to the pages router fixed it for me.

monolithed commented 1 year ago

Maybe not the best solution for now, but converting to the pages router fixed it for me.

@katiasmet-93, your solution might be suitable for projects with a small codebase where there's no need for testing investments. But such thoughts really make one reconsider completely dropping Next.

nikita2090 commented 1 year ago

I have the same problem when using shared components in layouts and pages (next 13.4.19). It doesn't work correctly both when using CSS modules and when using tailwind.

olalonde commented 1 year ago

Same issue with mantine https://github.com/vercel/next.js/issues/16630#issuecomment-1718974809

gffuma commented 1 year ago

Same problem on 13.5.2 not the ideal solution but downgrade to 13.4.4 helped me.

monolithed commented 1 year ago

Same problem on 13.5.2 not the ideal solution but downgrade to 13.4.4 helped me.

Screenshot 2023-09-22 at 19 04 29

@gffuma, I tried downgrading to version 3.4, and it didn't help at all. If the component is not being reused, there are no issues.

gffuma commented 1 year ago

Is exact the 3.4.4 ?

monolithed commented 1 year ago

Is exact the 3.4.4 ?

@dgagn, yep! I tried that version. I don't think any magic should happen because this bug has been around for more than three years

ensidialabs commented 1 year ago

Most likely I have the same problem ((

270266163-017fb489-cd28-400c-b812-967444e140e0 270264894-b1b696ef-c3a4-46e4-b01c-2438ad7ed090

tinleym commented 1 year ago

The multiple imports also causes animation issues. If you want, say, a no-js fade in for a module on a dedicated route, a CSS module with an animate property could get reapplied during the animation and cause it to repeat.

maiconsanson commented 1 year ago

Using 14.0.1

Screenshot 2023-11-02 at 11 07 16
cpotey commented 11 months ago

Just chiming in to say I'm seeing the same as the above on 14.0.2 - hadn't noticed it previously when on 13.5.3, might look to revert back and see if it was the same

Screenshot 2023-11-15 at 13 17 29

From what I can tell, if there's a 'use client' somewhere in the imports of a page/component, then things start to go a bit whacky, and things start doubling up on CSS

sjahanmard commented 11 months ago

Let's assume that we are going to use @layer. When we are using a library, for example react-quill in my case, which has its own class names for styling, their class names will take precedence over our custom one and over ride them, there is no way to over ride them without !important. Is there a way out of this? @shuding

tomjvdps commented 10 months ago

For css from libraries, the only workaround I found is to rewrite it by encapsulating it in a @layer @sjahanmard

For defining @layer, I use this workaround: https://github.com/vercel/next.js/issues/51030#issuecomment-1644922285

After all this, despite the duplication of the CSS, everything is ok I'm using version 13.4.7

SkyGopnik commented 10 months ago

Found another case of this, I'm on the faq page, after that I go to the login page and return back using browser back button, after that i'm having multiple css files on my faq page, that don't apply to that.

image image

ssijak commented 9 months ago

@shuding is there an option to generate one big CSS file as an output, no splitting? without the solution for this, or this option, we can't upgrade to versions post 13.4.5

pvaladez commented 9 months ago

Global CSS is also sometimes being added after the module css(see below). This breaks the case where you are expecting a module class to override a class from your global css file.

I've forked the codepen example from @igordanchenko, updated next.js to 14.0.4, and added a global.css that sets the button color to green:

.button {
  background-color: green;
}

My expectation is that global CSS will always be added to the page before any components styles from css modules, so the css in Button.module.css will override the above .button selector, assuming the two selectors have the same specificity(ex. classes.root from Button.module.css).

Interestingly, the buttons on the home page will only be green if both global.css is the last file to be edited and its file contents have actually changed.

So it seems to be setting the order of the CSS based on which file was edited last!?

https://codesandbox.io/p/devbox/next-51030-forked-hj9qmz

I am also able to replicate this behavior locally, and I noticed that when I delete the .next folder and rebuild the app then it will reset to showing a random color(have seen red, blue & green) on the home page buttons.

https://github.com/pvaladez/nextjs-css-import-issue

AllenWilliamson commented 9 months ago

The current order of imports wreaks havoc when CSS Cascade Layers are being used. The defined layer precedence that belongs in the root layout file gets read after the application of layers in child components. This causes the defined layer precedence to be ignored in favor of something random (https://css-tricks.com/css-cascade-layers/#aa-establishing-a-layer-order), and can make routes unusable.

This issue surfaced for me when updating up from 13.4.4 to 13.4.5 or 13.4.6.

I solved the cascade layer issue by adding a webpack.BannerPlugin similar to the following:

const config = {
    webpack: (
        config,
        { buildId, dev, isServer, defaultLoaders, nextRuntime, webpack }
    ) => {
        config.plugins.push(
            new webpack.BannerPlugin({
                banner: '@layer one, two, three;\n\n',
                raw: true,
                entryOnly: true,
                include: /\.css$/,
            })
        );
        return config;
    },
};

It's not ideal but works for now.

AnastasiiaRoch commented 8 months ago

The same issue is in Next 14.1.0 (SCSS Modules). The problem happens when using generic reusable components in pages. Any updates, please?

This PR_50406 should have fixed this issue, but it still occurs.

Screenshot 2024-02-15 at 11 37 04
thexpand commented 8 months ago

I have the exactly same issue as @AnastasiiaRoch. Using SCSS Modules. And it's not consistent, so it's sometimes difficult to reproduce. The problem is that it happens on production as well.

I think that the following issue might be related, but it says it only happens on dev mode, which is not the case: https://github.com/vercel/next.js/issues/13092

scottsmith-1 commented 8 months ago

Using CSS Modules with and without compose and I've had all the same symptoms during route navigations. Duplicates lots of styles and throws off the cascade completely.

monolithed commented 8 months ago

I'm just curious why the project team hasn't provided any comments on such a critical issue for several years and hasn't prioritized this task in any way.

tinleym commented 8 months ago

@ztanner does this pr address this issue? https://github.com/vercel/next.js/pull/61428

MayheMatan commented 8 months ago

The current order of imports wreaks havoc when CSS Cascade Layers are being used. The defined layer precedence that belongs in the root layout file gets read after the application of layers in child components. This causes the defined layer precedence to be ignored in favor of something random (https://css-tricks.com/css-cascade-layers/#aa-establishing-a-layer-order), and can make routes unusable. This issue surfaced for me when updating up from 13.4.4 to 13.4.5 or 13.4.6.

I solved the cascade layer issue by adding a webpack.BannerPlugin similar to the following:

const config = {
    webpack: (
        config,
        { buildId, dev, isServer, defaultLoaders, nextRuntime, webpack }
    ) => {
        config.plugins.push(
            new webpack.BannerPlugin({
                banner: '@layer one, two, three;\n\n',
                raw: true,
                entryOnly: true,
                include: /\.css$/,
            })
        );
        return config;
    },
};

It's not ideal but works for now.

can you please provide a sample repo with such solution?

IhorKevin commented 8 months ago

It's another major version (14) and this pain still exists. I'm wondering why the issue is ignored by maintainers? Just look how much of CSS rules are useless because of the same component from project's UI kit is imported to different chunks: Screenshot 2024-03-05 at 12 05 30

samcx commented 7 months ago

We landed a :pr:https://github.com/vercel/next.js/pull/63157.

Can anybody verify if this has been fixed on v14.2.0-canary.28 or later?

otaviosoares commented 7 months ago

I need to run a few more tests, but it looks like it's fixed. 😃

EDIT: Confirmed! I don't see the style duplication anymore. The order seems correct as well.

Thank you very much everyone!!!

damnsamn commented 7 months ago

Absolutely huge news, great work guys. Thank you!

klonwar commented 7 months ago

This is awesome! Looking forward to the release

sannajammeh commented 7 months ago

@samcx This is still an issue for us even on the latest Canary. Only happning on build.

image
damnsamn commented 7 months ago

@samcx This is still an issue for us even on the latest Canary. Only happning on build.

@sannajammeh Your screenshot doesn’t look like it depicts this issue, which is related to duplicate styles which override specificity due to how they’re ordered. This fix as I understand it corrects the CSS chunking algorithm to prevent importing the same component styles more than once.

sannajammeh commented 7 months ago

@samcx This is still an issue for us even on the latest Canary. Only happning on build.

@sannajammeh

Your screenshot doesn’t look like it depicts this issue, which is related to duplicate styles which override specificity due to how they’re ordered. This fix as I understand it corrects the CSS chunking algorithm to prevent importing the same component styles more than once.

The issue mentions multiple problems. Double CSS is just one of them.

The fix also mentions CSS order and we're still experiencing issues.

samcx commented 7 months ago

@sannajammeh Can you share with us a simple :repro: showcasing your issue and a explanation in the :repro: (in a README.md) what we're supposed to see?

IhorKevin commented 7 months ago

I can not switch to canary and verify the issue is fixed (in my project I use Lingui SWC plugin which has an incompatibility with @swc/core used in canary branch of Next). But it's a big thing if it works. @samcx , thanks!