Since 3.3.1, CSS @import modules are not deduplicated (removed in https://github.com/webpack-contrib/css-loader/commit/1fb51340a7719b6f5b517cb71ea85ec5d45c1199, for example). Deduplication was reenabled later, but only for CSS modules (see https://github.com/webpack-contrib/css-loader/pull/1040#discussion_r369365652 and https://github.com/webpack-contrib/css-loader/pull/1044, which tried to reenable this for @import).
We'd like an option to turn back on deduplication for @import. For example:
module.exports = {
module: {
rules: [
{
test: /\.css$/i,
loader: "css-loader",
options: {
import: {
deduplication: true
},
},
},
],
},
};
We recognize in general this affects the order of css on the page, so it probably should default to false. However, providing this option can drastically improve applications that use lots of @import statements to include well-structured css with lots of duplicates.
In our case (JupyterLab), we have over 100 packages bundled together with webpack, many of which have css files. Each package uses @import to include the css of its dependencies in its main css file, but no package puts css on the page (i.e., no package imports its css into its javascript files). The top-level application is responsible for putting all the css on the page, and it accomplishes this with a single css file that @imports its direct dependency's css files. In essence, we have a dependency graph of css files which the top-level application is responsible for getting onto the page. For the application as a whole, we have many cases where a particular dependency css file is @imported dozens of times through this dependency graph of css files. This worked fine up through css-loader 3.3.1, since deduplication guaranteed that any given module's css file was only actually included once on the page, in a topographically correct order.
However, css-loader over version 3.3.1 does not do this deduplication, so we end up with thousands of DOM style tags, most of which are duplicates of others (for example, one dependency's CSS is on the page dozens of times). Our application slows to a crawl, and the startup time is at least 10x longer. More specifically, with css-loader 4.3.0, we end up with 23,151 style DOM elements on the page (without deduplication), but when I manually change the css-loader code to reenable deduplication for @import, this shrinks to 175 style DOM elements (and application load times are normal again). As it is, we won't be able to upgrade past css-loader 3.3.1 without either a major restructuring of our css (possibly losing the benefits of our approach) or abandoning or forking css-loader.
Essentially, we would love an option to effect this change:
diff --git a/src/plugins/postcss-import-parser.js b/src/plugins/postcss-import-parser.js
index b0698a6..8e73944 100644
--- a/src/plugins/postcss-import-parser.js
+++ b/src/plugins/postcss-import-parser.js
@@ -178,13 +178,13 @@ const plugin = (options = {}) => {
});
}
- options.api.push({ importName, media, index });
+ options.api.push({ importName, media, index, dedup: options.deduplicate });
// eslint-disable-next-line no-continue
continue;
}
- options.api.push({ url, media, index });
+ options.api.push({ url, media, index, dedup: options.deduplicate });
}
},
};
If you are okay with merging such an option, I can also work on an implementation, if that helps.
Sorry no, it is out of spec of official CSS, we want to move CSS pipeline into webpack core, so we will not support not official approaches, also it is not safe (in very many cases)
This worked fine up through css-loader 3.3.1, since deduplication guaranteed that any given module's css file was only actually included once on the page, in a topographically correct order.
It is not truth :smile:
Thanks for the very quick feedback. Do you happen to have any thoughts off the top of your head how to handle a complex dependency graph of css files @importing each other without bogging down with huge numbers of style tags?
This worked fine up through css-loader 3.3.1, since deduplication guaranteed that any given module's css file was only actually included once on the page, in a topographically correct order.
It is not truth 馃槃
That's good to know too. Since our imports were done in dependency order (i.e., we @import our dependency's css before our own), and the deduplication seemed to be structured such that the first time a module was seen was the time it was put on the page, it would be sorted correctly.
Thanks for the very quick feedback. Do you happen to have any thoughts off the top of your head how to handle a complex dependency graph of css files
@importing each other without bogging down with huge numbers of style tags?
To be clear, a typical situation for us is for css files A, B, and C to all import D, and A imports B and C. Without deduplication, D ends up on the page 3 times. Ideally, we'd like D to end up on the page first once, then B and C in either order, then A.
Thanks for the very quick feedback. Do you happen to have any thoughts off the top of your head how to handle a complex dependency graph of css files @importing each other without bogging down with huge numbers of style tags?
Honestly, it's hard for me to say this without any example, at least minimal
That's good to know too. Since our imports were done in dependency order (i.e., we @import our dependency's css before our own), and the deduplication seemed to be structured such that the first time a module was seen was the time it was put on the page, it would be sorted correctly.
Yep, you can do it in safe way If you put it in the right order, but it is still unsafe, we try to avoid to adding options which can lead to unsafe situations, when something is very specific we recommend to create own loaders/plugins
To be clear, a typical situation for us is for css files A, B, and C to all import D, and A imports B and C. Without deduplication, D ends up on the page 3 times. Ideally, we'd like D to end up on the page first once, then B and C in either order, then A.
This is how CSS works, you can experiment without webpack and you will that it is exactly what happens in the browser
Maybe you need simplify your graph and move some styles to global
Yep, you can do it in safe way If you put it in the right order, but it is still unsafe, we try to avoid to adding options which can lead to unsafe situations, when something is very specific we recommend to create own loaders/plugins
I think this is the crux of the issue. CSS in general is kind of a mess with a global namespace, but if you maintain structure things can be safe and efficient. It sounds like we should probably either do the dependency sort ourselves (perhaps using postcss to parse the css files?), or fork css-loader to create our own loader for our more structured situation.
Thanks for the feedback.
It sounds like we should probably either do the dependency sort ourselves (perhaps using postcss to parse the css files?), or fork css-loader to create our own loader for our more structured situation.
Honestly, you can try postcss-import and implement logic for merging CSS on own side and keep only necessary @imports or even without @import, it sounds not easy and not hard
fork css-loader to create our own loader for our more structured situation
It is also solutions, but in the future everything may change, so maintaining own fork is not the best idea for long term
Honestly, you can try
postcss-importand implement logic for merging CSS on own side and keep only necessary@importsor even without@import, it sounds not easy and not hard
Thanks for the tip on using postcss-import
It is also solutions, but in the future everything may change, so maintaining own fork is not the best idea for long term
In general, forking is a last resort, hence this issue asking about reenabling it in upstream first. As it is, we haven't been able to upgrade for a while past when the deduplication was removed, so that's also been a maintenance burden.
Thanks for the tip on using
postcss-import
From the docs, it looks like postcss-import actually skips duplicate css files by default, which looks like it may be exactly the default behavior we want: https://github.com/postcss/postcss-import#skipduplicates.
From the docs, it looks like postcss-import actually skips duplicate css files by default, which looks like it may be exactly the default behavior we want: https://github.com/postcss/postcss-import#skipduplicates.
https://github.com/postcss/postcss-import/blob/master/index.js#L227
not sure, you need to dedupe it between modules
not sure, you need to dedupe it between modules
At first glance, it looks to me like the scope there is referring to the media scope (e.g., @import file is root scope, but @import file print; is the different 'print' scope). So from that code (and the code below dealing with hashes), I would expect two modules containing @import url("~module/index.css") to have those imports deduplicated, and furthermore two modules importing completely separate filenames, but with the same content (e.g., a common normalization css file) to also be deduplicated unless that file contains imports itself.
In light of this default deduplicating behavior from postcss-import, showing that it is indeed a common thing, it would be great if you would reconsider. But I also understand if you don't want to support import deduplication, and we can work on our own preprocessor or loader for our situation.
In light of this default deduplicating behavior from postcss-import, showing that it is indeed a common thing
Sometimes developers do strange things and then looking for solutions to deal with these strange things :smile:
Honestly, I'd take a look at some kind of minimal working example, maybe this will allow you to see how it better to solve and improve docs about it, but we have one bug with deduplication in style-loader (very very very edge case), and to make sure it's not it
I posted a minimal toy working example at https://github.com/jasongrout/cssimports
we have one bug with deduplication in
style-loader(very very very edge case), and to make sure it's not it
Is there an open issue I can look at?
Edit: is it this issue? https://github.com/webpack-contrib/style-loader/issues/450#issuecomment-575667070
Thanks, I will look at this in near future (focus on webpack-dev-server and webpack-cli in this week)
Thanks, and thanks for all of your work in the webpack ecosystem!
@jasongrout And yes, we have 2 edge cases in link what you provide
@jasongrout Sorry for delay, still valid? I did not found example of HTML structure in repo...
Most helpful comment
I posted a minimal toy working example at https://github.com/jasongrout/cssimports