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

CSS Order Differs Between Development & Production Modes when Treeshaking. #7094

Open
brycehill opened this issue Apr 22, 2018 · 44 comments · May be fixed by #18302
Open

CSS Order Differs Between Development & Production Modes when Treeshaking. #7094

brycehill opened this issue Apr 22, 2018 · 44 comments · May be fixed by #18302

Comments

@brycehill
Copy link

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
CSS seems to be out of order in "development" vs "production" mode. It appears somehow related to sideEffects and tree shaking.

If the current behavior is a bug, please provide the steps to reproduce.

Here's a repo demonstrating the issue: https://github.com/brycehill/webpack-css-order.

There are instructions in the readme. Basically, if you build in development mode, the outputted CSS is different from production mode.

What is the expected behavior?
CSS order is consistent from one environment to another.

I don't want to depend on a specific CSS order, but I do expect it to be consistent from when I develop to when I build for production.

If this is a feature request, what is motivation or use case for changing the behavior?
N/A

Please mention other relevant information such as the browser version, Node.js version, webpack version, and Operating System.

Node: v8.9.1
Webpack: v4.6.0
OS: MacOS High Sierra

@sokra
Copy link
Member

sokra commented Apr 23, 2018

Yeah we could improve this. (But technically using sideEffects you say order doesn't matter).

Note to myself: for "index" we use dependency order, but for harmony execution harmony initi oder should be respected.

@brycehill
Copy link
Author

Thanks for the reply.

using sideEffects you say order doesn't matter

I'm curious why this is the case. Is it a technical limitation? For me, the final order doesn't matter, but what does matter is a consistent order between development and production environments

@TheLarkInn
Copy link
Member

@brycehill are you saying that unused CSS which causes the development mode to look different from production mode is getting treeshaken?

@brycehill
Copy link
Author

@TheLarkInn That's what I thought was happening at first.

However, if you take a look at my example repo, none of the CSS is eliminated, but setting sideEffects: true fixes the problem. To reiterate from my previous comment:

For me, the final order doesn't matter, but what does matter is a consistent order between development and production environments

@sokra
Copy link
Member

sokra commented Jun 24, 2018

Should be fixed by #7507 in 4.12.1

@AdrienLemaire
Copy link

AdrienLemaire commented Sep 10, 2018

Not sure if this is related, but after upgrading babel to v7 (already on webpack 4.17.2), my staging/production webpack builds are missing any css including through import "./style.less" calls.
After some tedious debugging, I found out that adding the cli option -p or the config mode: "production", (or both) will break the build (which means that dev build works without any problem).

Since I had "sideEffects": false, in my packages.json, I changed it to "sideEffects": ["*.css", "*.less"],, and it seems to have fixed my build.

Can someone explain me what this sideEffects is doing, why it only affects production builds, and why it was working with Babel v6 and not with babel v7 ?

@cvle
Copy link

cvle commented Dec 17, 2018

@sokra I just hit this issue in 4.27.1. It was also reproducable in https://github.com/brycehill/webpack-css-order with the upgraded webpack version.

Because of this I can't set "sideEffects": false nor "sideEffects": ["*.css"] in my package.json, otherwise it affects the CSS-order which causes styling issues.

@zamb3zi
Copy link

zamb3zi commented Jan 4, 2019

I'm also seeing this in 4.27.1 and 4.28.3.

@FuzzyTree
Copy link

FuzzyTree commented Jan 23, 2019

I'm seeing this in 4.29. When I remove the 'sideEffects' property from one my vendor libraries the css order between production and dev are the same but when the property is included they are not.

@brycehill
Copy link
Author

Looks like this was fixed in v4.12.1 as described by @sokra, however it broke again in the next minor release (v4.13.0).

I pushed an update to my test repo showing this.

@bz2
Copy link

bz2 commented May 9, 2019

@sokra Did this get deliberately reverted, or can the same fix just be reapplied to master?

@7iomka
Copy link

7iomka commented May 15, 2019

v4.30.0 - broken css order when you import your css files in js
v4.31.0 - seems css order is as expected!

@7iomka
Copy link

7iomka commented May 25, 2019

Attention, as before, when switching from one mode to another, for some strange reason, order gets lost!
To fix this you need to do npm cache clear --force
Why?

@Javey
Copy link

Javey commented Jul 4, 2019

When import file from node_modules which has declared "sideEffects": [".*css"] in package.json, the css order is unexpected, but it's ok when import file from root directory.

I have created a test repo: https://github.com/Javey/webpack-tree-shaking-demo

@webpack-bot
Copy link
Contributor

This issue had no activity for at least three months.

It's subject to automatic issue closing if there is no activity in the next 15 days.

@darylteo
Copy link

@webpack-bot please don't. This is an important issue and I'm disappointed we still haven't been able to solve without copious amounts of "!important" rules.

@alexander-akait
Copy link
Member

@darylteo don't worry we check all PRs what bot closed

@LoserAntbear
Copy link

It's pretty obvious, but might help someone.
TL;DR:
For multiple entries configuration. This might be caused by scripts order in your index.html.

Hi guys. I'm not sure, that what i'm going to say is fully applicable to covered situation, but i had the same issue.
In my React project i had 3 bundled .js files (vendors.bundle.js, first.bundle.js, second.bundle.js). We've been using style modules (imported styles).
When i was working in devenv, everything was cool. But after building for production styles become messy. Some was overwritten by another ones and so on.

The reason was obvious and i am a bit shy about not figuring out this in first minutes of investigation:
Scripts load order. Which was:

...
<script src="./vendors.bundle.js" type="text/javascript"></script>
<script src="./second.bundle.js" type="text/javascript"></script>
<script src="./first.bundle.js" type="text/javascript"></script>
...

All i had to do is switch order for last two guys.
Hope it might help someone like me. Cheers.

satazor added a commit to moxystudio/next-with-moxy that referenced this issue Dec 21, 2019
The "sideEffects" property in package.json improves webpack treeshaking. However that causes the out-of-order CSS we were seeing in production builds. We may enable again once the bug is fixed.
For more context see webpack/webpack#7094 and webpack-contrib/mini-css-extract-plugin#202.
satazor added a commit to moxystudio/next-with-moxy that referenced this issue Dec 22, 2019
The "sideEffects" property in package.json improves webpack treeshaking. However that causes the out-of-order CSS we were seeing in production builds. We may enable again once the bug is fixed.
For more context see webpack/webpack#7094 and webpack-contrib/mini-css-extract-plugin#202.
@Hypnosphi
Copy link
Contributor

Hypnosphi commented Feb 19, 2020

using sideEffects you say order doesn't matter

Does it mean that I have to mark all JS files that import CSS directly or indirectly as having side effects?

@vankop
Copy link
Member

vankop commented Sep 15, 2021

@alexander-akait is it still an issue for webpack@5?

@alexander-akait
Copy link
Member

@vankop need tests, I think no

@thoughtspile
Copy link

thoughtspile commented Sep 29, 2021

Hi friends! I just made a detailed repro with webpack@5.55: https://github.com/thoughtspile/mini-css-extract-plugin-order-repro

Dropping sideEffects from package disables tree-shaking altogether, which is unfortunate.

@alexander-akait

@thoughtspile
Copy link

In case you're curious, I have apparently worked around the issue with a Babel transform that traverses the dependencies and adds transitive CSS imports to every JS file in depth-first order: VKCOM/VKUI#1933

Not sure if it's a stable fix though.

@sokra
Copy link
Member

sokra commented Oct 18, 2021

Here is an explanation why it behaves that way. Using the example here: https://github.com/thoughtspile/mini-css-extract-plugin-order-repro

If the order of some modules matter they are not side-effect-free. In this example Text.js is not side-effect-free, since it has the side-effect of adding some css styles to the document. Actually that's very often the case and we still flag these modules as side-effect-free, but the real question you want to ask yourself when flagging something as side-effect-free is: Are these side-effects visible form outside or are they only internally used?
In this case side-effects of Text.js are visible from outside, because the css class is combined with another css class on the same element.

In addition to that mini-css-plugin makes no strong guarantee about the order of independent CSS files. It tries its best to keep the importing order, but there are some cases where this is not followed, e. g. because modules are split into multiple css bundles.

I would recommend to not rely on CSS order between multiple files in general. If you really want to, make sure to @import the styles that should be overriden in the CSS file. That makes a bit stronger guarantees.

So best don't mix class names from different origins on the same element. You can use composition/nesting instead.


Nonetheless I think we can change the order in the case where it's directly imported in a certain order...

@thoughtspile
Copy link

Here is an explanation why it behaves that way.

Thanks for the response! I've spent quite some time on a task to end up facing this issue, so I'd like to discuss a bit.

Use case background first. Our component library, VKUI, has ~100 components, but an average app only uses ~30. Eliminating unused CSS seemed like an easy win. We also rely on <Item className="Context__item"> that works like a charm without tree-shaking, and I thought I could avoid a large-scale refactoring.

If the order of some modules matter they are not side-effect-free.

I see that the whole idea of tree-shaking CSS is shaky, because it's trying to be both a side-effect (the module that directly imports it relies on it implicitly) and non-side-effect (because you can remove it when you don't have a direct import in used modules). Could you please explain how sideEffects propagate?

I'd like to avoid nesting (if you mean having .Context .Item selectors in Item.css) for several reasons:

  • Item is exposed to the internals of every component that can contain it. <Item className="Context__item"> is not perfect either, but at least it only leaks Item's top-level box, which tends to be more stable.
  • If we pick child component conditionally, say createElement(href ? Link : Button, { className: 'Context__action' }, we now have to add overrides in both Link.css and Button.css

Shipping with @import in CSS seems easy to misconfigure on user side — add a postcss-import and hello duplicate css. Also, we now have to import child components in both JS and CSS, which is unpleasant and can't be automated away.

I am arriving at the conclusion that a UI kit without CSS-in-js or an advanced CSS variable system is not possible, but just wanted to try one last time.

@alexander-akait
Copy link
Member

Shipping with @import in CSS seems easy to misconfigure on user side — add a postcss-import and hello duplicate css. Also, we now have to import child components in both JS and CSS, which is unpleasant and can't be automated away.

css-loader handles @import by default with valid order, you can avoid using postcss-import

@marr
Copy link

marr commented Nov 21, 2022

Well, if you need something fast and dirty you may apply webpack+4.46.0.patch.txt via patch-package... The patch most probably breaks something somewhere since it makes HarmonyImportSideEffectDependency to behave exactly as HarmonyImportDependency and removes dependencyReference hook call in Compilation.js. But it makes development and production CSS order to be consistent for me.

Note, that completely broke my build. Console error:

TypeError: (void 0) is not a function

I was trying this as a workaround suggested here: vuetifyjs/vuetify#13732

@marr
Copy link

marr commented Nov 21, 2022

Fwiw, I had been using treeShake: false in my nuxtjs-vuetify module settings due to a separate issue I was experiencing before. Removing that fixed the CSS order issue. To fix the other issue, I had to import a component I was rendering conditionally with import { VCard } from 'vuetify/lib'.

@william-will-angi
Copy link

william-will-angi commented Mar 21, 2023

I got around this issue by adding sideEffects: true to all of my webpack loader configs. Not the most ideal as it disables tree-shaking appwide. Hoping someone may have found a better solution.

Edit: Without modifying sideEffects, it actually seems to be producing class names in the exact opposite order that I would expect.

@kosciolek
Copy link

kosciolek commented Jan 6, 2024

I created an issue in mini-css-extract-plugin's repo. It might or might not be related: webpack-contrib/mini-css-extract-plugin#1070

CSS files are concatenated in the order they're imported. Webpack seems to reorder imports of side effect free modules. We're supposed to counteract it with the sideEffects field in package.json.

However, it seems like "sideEffects": ["*.css"] does not work properly for external packages (i.e. from NPM, or other packages from your monorepo).

  1. It works for your local files. If declared inside the package.json of your app, then CSS files under your src will not be tree shaken out, and they also will be included in the correct order.
  2. BUT, if you import a package which has "sideEffects": ["*.css"] in its package.json, then its CSS will not be tree shaken out, but it might be loaded in the wrong order, as webpack reorders imports by the time mini-css-extract-plugin runs!

Here is a simple reproduction with a description.

@hlyst21
Copy link

hlyst21 commented Jan 15, 2024

@alexander-akait Greetings! Is there currently any way to solve the problem with the order of styles when using tree shaking?
I saw that the task has been moved to high priority in webpack 5/6, but if this can somehow be solved now, that would be just great.

My case:
On the react project.js using webpack 5.
Styling via css modules from under scss.
That is, each component has its own css module, for example:

component
  Component.js
  Component.module.scss
// Component.js
import React from 'react'

import styles from './Component.module.scss'

const Component = () => {}

When you enable tree shaking in webpack (specify sideEffects: ["/.css", "/.scss"]), problems with css formation begin, namely, the order of modules entering the final css file ceases to be predictable and logical.
For example, there are two react components.js, Title and Content. In the Content component, we want to reuse the Title component, and pass other styles to it via className prop:

components
  title
    Title.js
    Title.module.scss
  content
    Content.js
    Content.module.scss
// Title.js
import React from 'react'
import cx from 'classnames'

import styles from './Title.module.scss'

const Title = ({ className }) => (
  <h1 className={cx(styles.title, className)}>Title</h1>
)

// Title.module.scss
.title {
  background: red;
}

// Content.js
import React from 'react'

import Title from '../title/Title'

import styles from './Content.module.scss'

const Content = ({ className }) => (
  <Title className={styles.title}></Title>
)

// Content.module.scss
.title {
  background: blue;
}

With tree shaking turned off, we will have the following final css file:

.components_title_title {background: red;} 
.components_content_title {background: blue;} 

That is, everything will work as expected, first come the styles of the Title component, then the styles of the Content component. The passed class from Content to Title will be below, so styles can be easily redefined.

However, when tree shaking is active, the style order will be the opposite of what we expect, i.e.:

.components_content_title {background: blue;}
.components_title_title {background: red;} 

In this case, the styles from the Content will not be able to override the styles of the Title, respectively.
This is very demotivating, because I want to take advantage of tree shaking, but because of this problem with styles, I can’t do it.

@alexander-akait
Copy link
Member

Yeah, it is a bug inside webpack, I don't look deeply at this right now, so no solution right now, you can try to solve it on the CSS side, i.e. use css specificity, I know it is not a perfect solution, also you can try to use CSS modules

@kosciolek
Copy link

kosciolek commented Mar 18, 2024

@alexander-akait I'd like to fix it myself if possible, would you accept a PR? I haven't done any contributions to webpack yet though, what do you think is the scope of this issue? Any suggestions as to where I should start investigating?

const processQueue = () => {
It looks like webpack traverses modules here?

@alexander-akait
Copy link
Member

@kosciolek I'm afraid this is not an easy task, you can start with adding failed test cases and then we will look how to fix it

@kosciolek
Copy link

One of the commits in 5.90.2 seems to have fixed it, at least for the reproduction I made above. However, our larger project still has this problem, but it's likely it's something on our side.

@alexander-akait
Copy link
Member

@kosciolek Yeah, we can also have some edge cases, so, if you can provide an example I will investigate it

One of the commits in 5.90.2 seems to have fixed it

Release will be today/tomorrow, need to finish one small thing

@kosciolek
Copy link

kosciolek commented Apr 2, 2024

This is the commit in 5.90.2 that fixed the issue in the reproduction:

a2ce375

commit a2ce375d968a9ef5e8b9ee59a30268d3b35c864a
Merge: 2a063f88d 1673ee5d1
Author: Alexander Akait <4567934+alexander-akait@users.noreply.github.com>
Date:   Mon Feb 12 13:37:05 2024 +0300

    fix: use new runtime to reconsider skipped connections activeState

Adding these 3 removed lines back breaks the repro again:

a2ce375#diff-926fcfc6c89e458cc222fa6b30ac4150fca4140e70522c08fa661b005da45b48L102

@kosciolek
Copy link

kosciolek commented Apr 8, 2024

@alexander-akait I made a PR with a failed test case

As stated above, after 5.90.2, the reproduction I linked above is no longer applicable (apparently it's fixed), but now there's another problem when optimizing reexports of modules that import side-effectful modules.

Commenting this line out

moduleGraph.updateModule(dep, target.module);
"solves" the issue, but I don't know where's the real problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Priority - High
Development

Successfully merging a pull request may close this issue.