1.1.2 injects the same stylesheet more than once #450

Hypnosphi · 2019-12-25T17:36:21Z

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

The text was updated successfully, but these errors were encountered:

alexander-akait · 2019-12-25T17:45:04Z

It is expected, you have two ./common.css imports:

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)

alexander-akait · 2019-12-25T17:47:31Z

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

alexander-akait · 2019-12-25T17:48:24Z

Yes, it was changed because before it was invalid, downgrade loader with example above and you will see what color invalid.

Hypnosphi · 2019-12-25T19:21:38Z

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

Hypnosphi · 2019-12-25T19:29:15Z

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.

alexander-akait · 2019-12-26T09:34:23Z

@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

quebits · 2019-12-26T18:06:26Z

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.

jeffijoe · 2019-12-27T10:17:00Z

@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

alexander-akait · 2019-12-27T10:23:29Z

Yep, it is bug, looking for the solution

alexander-akait · 2019-12-27T10:33:28Z

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

jeffijoe · 2019-12-27T10:34:31Z

I think that makes sense. 😄

Hypnosphi · 2019-12-27T13:04:21Z

What about @value? Should it behave as @import, or as composes does?

alexander-akait · 2019-12-27T13:21:12Z

@value like @import and should works fine

Hypnosphi · 2019-12-27T14:44:07Z

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;
}

pretty bug Need to wait until this issue is resolved before updating this: webpack-contrib/style-loader#450 Not a big deal, only took me 2 hours to figure out what's going wrong here. The day we completely move to `styled-components` can't come any sooner...

* Upgrade deps. * Add `request` dependency explicitly. Not sure how this used to work, but now webpack is throwing that it can't find this dependency so we're installing it explicitly. * Fix ambiguous composition in css modules. Not sure how this used to work, but now it throws an error. * Update `css-loader` options. * Import `upload` function only in Electron environment. Reverts commit fbfb355. Previously, I thought it was some library that's requiring the `fs` module, and it turns out it was me after all. Oops. * On second thought, rever the `style-loader` dependency as it has a pretty bug Need to wait until this issue is resolved before updating this: webpack-contrib/style-loader#450 Not a big deal, only took me 2 hours to figure out what's going wrong here. The day we completely move to `styled-components` can't come any sooner... * Update deprecated Electron APIs. * Fix prettier lint errors. * Add a temporary fix for overriding default button styles. * Import Electron `screen` API from the main process.

alexander-akait · 2019-12-30T14:25:35Z

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

jeffijoe · 2019-12-30T14:26:34Z

@evilebottnawi thank you for your hard work! Enjoy the holidays!

jquense · 2020-01-08T15:26:02Z

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 :/

jquense · 2020-01-08T15:29:27Z

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

Hypnosphi · 2020-01-08T15:45:43Z

@jquense Imports from JS seem to work as they did before: https://github.com/Hypnosphi/style-loader-duplicates/tree/import-from-js

jquense · 2020-01-08T16:01:03Z

Hmm interesting! I'm definitely not seeing that, maybe i can turn it into a repro,

jquense · 2020-01-08T16:03:22Z

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 :/

jquense · 2020-01-08T17:11:59Z

@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.

alexander-akait · 2020-01-09T09:50:09Z

@jquense it is bug, i will fix it in near future, there was a vacation and I unfortunately did not have time, sorry

jquense · 2020-01-09T15:49:58Z

no worries, i've realized i'm in the same boat for old versions as well. Please don't rush, enjoy vacation.

alexander-akait · 2020-01-17T15:16:26Z

Partial fixed, but some cases still broken (it was broken in previous versions too):

same @import in 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 @import then insert new @import it can create an invalid order (a rare use case)

alexander-akait · 2020-01-17T15:18:44Z

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

quebits · 2020-01-17T15:26:17Z

My case #450 (comment) looks like is solved. Will test it additionally. Thank U!

Hypnosphi · 2020-01-27T21:32:22Z

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

alexander-akait · 2020-01-28T14:22:29Z

@Hypnosphi please update css-loader to ^3.4.2, we have some regression on css-loader side, thanks!

Hypnosphi · 2020-01-28T16:16:39Z

I can confirm that it's fixed with [email protected] and [email protected]

alexander-akait · 2020-01-28T16:18:41Z

Let's keep open for some other edge case (#450 (comment)), I think I will fix it in next week, anyway thanks for the issue

nicomfe · 2020-06-08T04:57:39Z

Im using scss modules, getting duplicate inline css with these versions

"style-loader": "1.2.1" or "1.1.3"
"css-loader": "3.5.3" or "3.4.2"
"sass-loader": "8.0.2"

Repo for reproducing the issue here: https://github.com/nicomfe/ultimate-react-server-rendering/tree/with-redux-and-scss-modules

alexander-akait · 2024-04-08T16:33:31Z

All problems should be resolved, if you faced with the proble - please open a new issue and I will help

alexander-akait mentioned this issue Jan 17, 2020

fix: injection algorithm #456

Merged

6 tasks

Hypnosphi closed this as completed Jan 28, 2020

alexander-akait reopened this Jan 28, 2020

This was referenced Nov 20, 2020

Option to turn on deduplication for @import webpack-contrib/css-loader#1233

Closed

Update css-loader, style-loader and mini-css-extract-plugin jupyterlab/jupyterlab#9364

Closed

alexander-akait closed this as completed Apr 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

1.1.2 injects the same stylesheet more than once #450

1.1.2 injects the same stylesheet more than once #450

Hypnosphi commented Dec 25, 2019

alexander-akait commented Dec 25, 2019 •

edited

Loading

alexander-akait commented Dec 25, 2019

alexander-akait commented Dec 25, 2019 •

edited

Loading

Hypnosphi commented Dec 25, 2019 •

edited

Loading

Hypnosphi commented Dec 25, 2019

alexander-akait commented Dec 26, 2019

quebits commented Dec 26, 2019

jeffijoe commented Dec 27, 2019

alexander-akait commented Dec 27, 2019 •

edited

Loading

alexander-akait commented Dec 27, 2019

jeffijoe commented Dec 27, 2019

Hypnosphi commented Dec 27, 2019 •

edited

Loading

alexander-akait commented Dec 27, 2019 •

edited

Loading

Hypnosphi commented Dec 27, 2019

alexander-akait commented Dec 30, 2019

jeffijoe commented Dec 30, 2019

jquense commented Jan 8, 2020 •

edited

Loading

jquense commented Jan 8, 2020

Hypnosphi commented Jan 8, 2020

jquense commented Jan 8, 2020

jquense commented Jan 8, 2020 •

edited

Loading

jquense commented Jan 8, 2020

alexander-akait commented Jan 9, 2020

jquense commented Jan 9, 2020

alexander-akait commented Jan 17, 2020

alexander-akait commented Jan 17, 2020

quebits commented Jan 17, 2020

Hypnosphi commented Jan 27, 2020 •

edited

Loading

alexander-akait commented Jan 28, 2020

Hypnosphi commented Jan 28, 2020

alexander-akait commented Jan 28, 2020

nicomfe commented Jun 8, 2020 •

edited

Loading

alexander-akait commented Apr 8, 2024

1.1.2 injects the same stylesheet more than once #450

1.1.2 injects the same stylesheet more than once #450

Comments

Hypnosphi commented Dec 25, 2019

Expected Behavior

Actual Behavior

How Do We Reproduce?

alexander-akait commented Dec 25, 2019 • edited Loading

alexander-akait commented Dec 25, 2019

alexander-akait commented Dec 25, 2019 • edited Loading

Hypnosphi commented Dec 25, 2019 • edited Loading

Hypnosphi commented Dec 25, 2019

alexander-akait commented Dec 26, 2019

quebits commented Dec 26, 2019

jeffijoe commented Dec 27, 2019

alexander-akait commented Dec 27, 2019 • edited Loading

alexander-akait commented Dec 27, 2019

jeffijoe commented Dec 27, 2019

Hypnosphi commented Dec 27, 2019 • edited Loading

alexander-akait commented Dec 27, 2019 • edited Loading

Hypnosphi commented Dec 27, 2019

alexander-akait commented Dec 30, 2019

jeffijoe commented Dec 30, 2019

jquense commented Jan 8, 2020 • edited Loading

jquense commented Jan 8, 2020

Hypnosphi commented Jan 8, 2020

jquense commented Jan 8, 2020

jquense commented Jan 8, 2020 • edited Loading

jquense commented Jan 8, 2020

alexander-akait commented Jan 9, 2020

jquense commented Jan 9, 2020

alexander-akait commented Jan 17, 2020

alexander-akait commented Jan 17, 2020

quebits commented Jan 17, 2020

Hypnosphi commented Jan 27, 2020 • edited Loading

alexander-akait commented Jan 28, 2020

Hypnosphi commented Jan 28, 2020

alexander-akait commented Jan 28, 2020

nicomfe commented Jun 8, 2020 • edited Loading

alexander-akait commented Apr 8, 2024

alexander-akait commented Dec 25, 2019 •

edited

Loading

alexander-akait commented Dec 25, 2019 •

edited

Loading

Hypnosphi commented Dec 25, 2019 •

edited

Loading

alexander-akait commented Dec 27, 2019 •

edited

Loading

Hypnosphi commented Dec 27, 2019 •

edited

Loading

alexander-akait commented Dec 27, 2019 •

edited

Loading

jquense commented Jan 8, 2020 •

edited

Loading

jquense commented Jan 8, 2020 •

edited

Loading

Hypnosphi commented Jan 27, 2020 •

edited

Loading

nicomfe commented Jun 8, 2020 •

edited

Loading