style-loader
style-loader copied to clipboard
webpack-contrib
1.1.2 injects the same stylesheet more than once
- Operating System: macOS Catalina
- Node Version: 12.13.1
- NPM Version: 6.12.1
- webpack Version: 4.41.4
- style-loader Version: 1.1.2
Expected Behavior
Each stylesheet gets injected only once
Actual Behavior
Stylesheet gets re-injected each time it's imported from another stylesheet
How Do We Reproduce?
https://github.com/Hypnosphi/style-loader-duplicates
yarn && yarn start
Open http://localhost:8080, inspect the HTML:
It doesn't reproduce with 1.0.2, see 1.0 branch
It is expected, you have two ./common.css imports:
- https://github.com/Hypnosphi/style-loader-duplicates/blob/master/a.css#L1
- https://github.com/Hypnosphi/style-loader-duplicates/blob/master/b.css#L1
It is required for right order, for example you can include on a.css or b.css with content:
.foo {
color: red;
}
Without double including we break css @import logic (described in css spec)
For example, you have:
index.html
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8" />
<meta name="description" content="" />
<meta name="viewport" content="width=device-width, initial-scale=1" />
<title>Webpack Playground</title>
<style>
@import url('./order-1.css');
@import url(http://example.com/style.css);
@import url('./order-2.css');
@import url(http://example.com/style.css);
@import url('./order-1.css');
@import url(http://example.com/style.css);
@import url('./order-2.css') screen and (min-width: 2000px);
@import url(http://example.com/style.css);
div {
width: 100%;
height: 200px;
}
</style>
</head>
<body>
<h1>Webpack Playground</h1>
<p>Text</p>
<div> Text of DIV </div>
</body>
</html>
order-1.css
div {
background: red;
}
order-2.css
div {
background: blue;
}
recreate that example and look in dev tools, each @import applies to style
Yes, it was changed because before it was invalid, downgrade loader with example above and you will see what color invalid.
I see. May there be a sense in keeping only the last include, as it overrides all the previous ones anyway?
UPD: I mean, last include for each unique pair of resource + media query
Actually, changed order is what led me to discovering this issue. E.g., if you add this to a.css
.bar {
color: red;
}
<div class="foo bar" /> will still be blue, because common.css comes once again from b.css import. But it's the same with native @import, so I got the point.
@Hypnosphi
May there be a sense in keeping only the last include, as it overrides all the previous ones anyway?
It is potential unsafe and broke how @import should works
will still be blue, because common.css comes once again from b.css import. But it's the same with native @import, so I got the point.
Right, it’s good that you found this problem, the same styles may seem like a mistake, but this is the correct insertion algorithm, I can spend time and provide you with examples when the previous algorithm broke the import logic, yes it increases the page size a bit, but we should use the correct logic instead of a small reduction and potential invalid order which again can lead to the wrong style
In my case it ruins everything. I am using css-modules/compose.
Something like that:
common.css:
.common {
background-color: yellow;
}
1.css:
.selector1 {
composes: common from 'common.css';
background-color: red;
}
2.css:
.selector2 {
composes: common from 'common.css';
background-color: navy;
}
import css1 from './1.css'
import css1 from './2.css'
css1.use();
css2.use();
const el1 = document.createElement('div');
document.body.appendChild(el1);
el1.className = styles1.locals.selector1
const el2 = document.createElement('div');
document.body.appendChild(el2);
el2.className = styles2.locals.selector2
And now we will have yellow background color for el1 instead of expected red.
Because 'composed' styles has been inserted twice.
<style>
.common { background-color: yellow; }
</style>
<style>
.selector1 { background-color: red; }
</style>
<style>
.common { background-color: yellow; }
</style>
<style>
.selector2 { background-color: navy; }
</style>
Second common style has override .selector1 background color.
@evilebottnawi I have the same issue due to using CSS Modules composes. I built a reproduction repo here: https://github.com/jeffijoe/webpack-css-dupe-bug
Problem what css @import have other order logic than css modules composes, and if we return previous behavior we break logic for css @import, but new behavior break logic for composes, maybe we can mark composes modules and do other logic
I'm asking because in our codebase, we mostly import a classname as @value before composing:
@value someClass, someColor, someFont from '../common.css';
.myClass {
composes: someClass;
color: someColor:
font: someFont;
}
I found solution, I can not be in time because of the holidays (but I will try to make it in time ), I really apologize
I'm not sure how the old behavior was incorrect, It looks wrong when you write normal css imports but that's not the expected behavior of modules or the dependencies in webpack. In general we usually assume that a required module is only imported the first time as is the case with JS. It's impossible now for dependency order to be correct for css files that depend on each other.
Consider this case without ~~css-modules~~ with light css-modules;
base-button.css
.btn {
height: 30px;
}
custom-button.css
.custom-btn {
height: 60px;
}
toolbar.css
@value btn from './base-button.css';
.toolbar > btn {
margin-left: 10px
}
CustomButton.js
import 'base-button.css'
import 'custom-button.css'
return <button className='btn custom-btn'>
Toolbar.js
import 'toolbar.css'
return <div className='toolbar'>{children}</div>
App.js
import CustomButton from './CustomButton'
import Toolbar from './Toolbar'
My expectation, and previous behavior is that CustomButton has overriden the base button height, b/c it was defined later. But not b/c SomeComponent imports after Custom Button it re imports the style. I know this has been covered but i wanted to make the case that this is NOT just a css-modules issue, it also vastly increases the number of styles that included in the app during dev :/
I guess all i'm saying is, trying to match the css import spec seems lik the wrong thing to do, at least as a default. I don't think it's possible for css to interoperate with js in the way webpack has defined without it treating css imports with the same rules as the rest of the dependency graph
@jquense Imports from JS seem to work as they did before: https://github.com/Hypnosphi/style-loader-duplicates/tree/import-from-js
Hmm interesting! I'm definitely not seeing that, maybe i can turn it into a repro,
O i see, yeah the trigger is css-modules, BUT in our case it's absolutely due to a @value import, so i'd add that they are intend a problem here as well not just composes
EDIT updated my example with breaking case
PS: @evilebottnawi this change also breaks my approach in the css-module-loader proof of concept, since it adds @imports to dependent css-modules to ensure the correct order, we could work around that tho by always handling :import {} in css-loader perhaps?
PPS: I also found a bug in the pre 1.1.0 code that also duplicated styles :/
@evilebottnawi does it make sense to have an additional concept in css-loader that marks things as dependencies but not css imports? TBH it seems strange that style-loader doesn't just insert the style for the single file it's loader, instead b/c css-loader returns it's css as well as any of it's dependencies, style-loader seems to assume that the js entry for a css import results in a complete, isolated css file.
I guess this is really highlighting that css-loader should really not do css-modules as well.
@jquense it is bug, i will fix it in near future, there was a vacation and I unfortunately did not have time, sorry
no worries, i've realized i'm in the same boat for old versions as well. Please don't rush, enjoy vacation.
Partial fixed, but some cases still broken (it was broken in previous versions too):
- same
@importin two difference files collapsed in one so order can be potential invalid, we should fix it in css-loader, maybe add 4th argument for all@import, it is allow to distinguish css modules import from@import - When you use HMR and remove
@importthen insert new@importit can create an invalid order (a rare use case)
Please test https://github.com/webpack-contrib/style-loader/releases/tag/v1.1.3 and put feedback here, should be work fine in 99% cases
My case https://github.com/webpack-contrib/style-loader/issues/450#issuecomment-569104644 looks like is solved. Will test it additionally. Thank U!
Thanks @evilebottnawi. It mostly works, but not in a case when B composes from A, and C composes from both A and B.
I updated https://github.com/Hypnosphi/style-loader-duplicates to reproduce this case. <div class="${baz}">baz</div> should be red, but it's blue
@Hypnosphi please update css-loader to ^3.4.2, we have some regression on css-loader side, thanks!