ICSS: Importing a composed class name produces wrong output

Question

ICSS: Importing a composed class name produces wrong output

Opened this issue 6 years ago · 20 comments

Operating System: Linux
Node Version: v10.17.0
NPM Version: n/a, Yarn 1.15.2
webpack Version: 4.41.2
css-loader Version: 3.2.0

Expected Behavior

css-loader should produce correct output:

.outer-box__outerBox--1rUd7j4- .inner-box__innerBox--19HrVC2M.shared__box--3DFGfIs1 {
	background-color: lime;
}

Actual Behavior

css-loader produces incorrect output:

.outer-box__outerBox--1rUd7j4- .inner-box__innerBox--19HrVC2M shared__box--3DFGfIs1 {
	background-color: lime;
}

Code

See below.

How Do We Reproduce?

Full repro case here: https://git.cryto.net/joepie91/icss-loader-test

Run yarn webpack to produce output with Webpack + css-loader (incorrect output), run yarn browserify-extract to produce output with Browserify + icssify (correct output).

Both output to the same files, so you can switch between them. Open test/index.html in a browser to see the result.

The bug is caused by the import handling using the JS/HTML-formatted (space-delimited) export, rather than a CSS-formatted (dot-delimited) export.

This means that when using a composed identifier (which consists of two space-delimited class names) as an imported identifier, the resulting selector rule will be broken (see code snippets above).

This can be fixed by replacing spaces with dots in the case of ICSS imports.

Answer 1 · 2019-11-25T10:47:23.000Z

Thanks for issue i will look on this in near future

Answer 2 · 2019-12-03T15:50:25.000Z

I have the same. After update all new react-scripts dependencies, also css-loader updated. With the latest version my all code based on CSS-Modules composes is broken.

Answer 3 · 2019-12-09T16:11:21.000Z

@yakato please create reproducible test repo

Answer 4 · 2020-02-17T10:16:16.000Z

Might be related to #561, which has a minimal reproduction.

Answer 5 · 2020-02-17T18:39:17.000Z

@apexskier That's a different issue, about imports not working at all. That has since been fixed, as far as I can tell.

The remaining issue is that specifically composed class names are not handled correctly, for which I've provided a reproduction in the initial report above.

Answer 6 · 2020-04-06T09:05:45.000Z

Hello.

We are also experiencing similar issue to this. In our case its an issue with using composes with dynamically imported css chunks.

I have created a minimal reproduction:

https://github.com/OHUSAR/css-modules-chunk-specificity-bug

@evilebottnawi is this an issue, you will be considering to look into, or should we use different way of solving our problems? Thank you for your answer!

The precise definition of the problem (pasted from README.md):

Let's consider having these source files:

a.module.css
```
.test {
    background-color: red;
}
```

b.module.css

.test2 {
    composes: test from "./a.module.css";
    background-color: blue;
}

c.module.css

.test3 {
    composes: test from "./a.module.css";
    background-color: green;
}

The output of these files would be:

a.module.css
```
.test {
    background-color: red;
}
```

b.module.css

.test {
    background-color: red;
}

.test2 {
    background-color: blue;
}

c.module.css

.test {
    background-color: red;
}

.test3 {
    background-color: green;
}

Now let's consider that our code dynamically imports chunk with output of a.module.css and c.module.css and than later imports chunk with b.module.css output.

The current applied css would be:

    .test {
        background-color: red;
    }

    .test {
        background-color: red;
    }

    .test3 {
        background-color: green;
    }

    .test {
        background-color: red;
    }

    .test2 {
        background-color: blue;
    }

When considering element using classnames from c.module.css,

    <div class="test_localname test3_localname" />

the applied background color of the C element would be red instad of green. The reason for this is that the class .test is included later in the css hierarchy and therefore is used for element style.

Answer 7 · 2020-04-06T09:13:40.000Z

@OHUSAR It should be fixed, can you create reproducible test repo, thanks

Answer 8 · 2020-04-06T09:16:04.000Z

Ok. I will create a separate issue, with full description and all thats needed.

Answer 9 · 2020-04-06T09:22:26.000Z

@OHUSAR You can put a problem here if it will be other problem I will create issue from comment

Answer 10 · 2020-04-06T09:42:40.000Z

Operating System: Ubuntu 16.04.5 LTS
Node Version: v10.19.0
NPM Version: 6.13.4
webpack Version: 4.42.1
css-loader Version: 3.4.1 (used in latest CRA https://github.com/facebook/create-react-app/blob/master/packages/react-scripts/package.json)

Expected Behavior

After dynamically importing component, which composes css class, that an already present component also composes, the existing component should preserve its styles.

Actual Behavior

After dynamically importing component B, components C styles get overriden.

Code

// index.module.css

.test {
    background-color: red;
    padding: 30px;
}

// b.module.css

.test2 {
    composes: test from '../index.module.css';
    background-color: blue;
}

// c.moodule.css

.test3 {
    composes: test from '../index.module.css';
    background-color: green;
}

Result:

<head>
    <link rel="stylesheet" type="text/css" href="chunk-2.css">
    <script charset="utf-8" src="2.main.js"></script>
    <link rel="stylesheet" type="text/css" href="chunk-1.css">
    <script charset="utf-8" src="1.main.js"></script>
</head>

// chunk-2.css

._2D6NmF3oNXamuyZ2z-z6-2 {
  background-color: red;
  padding: 30px;
}

._2vSpwQvfEWT-h3qiyUMFPO {
  background-color: green;
}

// chunk-1.css

._2D6NmF3oNXamuyZ2z-z6-2 {
  background-color: red;
  padding: 30px;
}

._3IkLTxcAE-k8TWXxjYQrik {
  background-color: blue;
}

Webpack config:

// webpack.config.js
...
    module: {
        rules: [
            {
                test: /\.module\.css$/i,
                use: [
                    { loader: MiniCssExtractPlugin.loader },
                    {
                        loader: 'css-loader',
                        options: {
                            modules: true,
                            importLoaders: 1,
                        },
                    },
                ],
            },
        ],
    },
...

How Do We Reproduce?

For reproduction, see this minimal repo:
https://github.com/OHUSAR/css-modules-chunk-specificity-bug

Answer 11 · 2020-04-06T09:43:33.000Z

@OHUSAR Thanks, I will look at that in near future

Answer 12 · 2020-05-07T09:05:18.000Z

@evilebottnawi any updates on this please? thank you

Answer 13 · 2020-05-07T11:00:30.000Z

@OHUSAR Looks like a limitation, I think it can't be fixed without other problems

Answer 14 · 2020-07-23T12:18:05.000Z

@OHUSAR Problem on mini-css-extract-plugin side, webpack/mini-css-extract-plugin#555

Answer 15 · 2020-07-23T13:21:54.000Z

Sorry for delay, WIP on the issue, maybe we need improve our architecture, also it is allow to validate existing names and throw an error on the build step

Answer 16 · 2020-07-23T13:52:38.000Z

Here big problem 😞 @joepie91 You example is broken too.

Example:

inner-box.css

@value shadow: 10px 5px 5px red;

.innerBox {
	composes: box from "./shared.css";
	width: 64px;
	height: 64px;
	background-color: red;
}

outer-box.css

:import("./inner-box.css") {
	importedInnerBox: innerBox;
}

@value shadow from './inner-box.css';

.outerBox {
	composes: box from "./shared.css";
	width: 100px;
	height: 100px;
	background-color: blue;
	box-shadow: shadow;
}

.outerBox .importedInnerBox {
	background-color: lime;
}

And look at bundle:

"shadow":"10px.5px.5px.red"

We replace spaces on ., but it is invalid.

Don't know how we can solve it, because we don't know this values on the build step and don't know context, what we know? Only value.

Answer 17 · 2020-07-23T13:58:40.000Z

I postpone it for v5, now it is the limitation

Answer 18 · 2020-07-23T14:01:36.000Z

Here other problem you can import selector from other file and it is not hashed, using :local(selector) { } provide invalid export. So supporting selector is very limited now, you can use it only in the current file, importing is not working as expected

Answer 19 · 2020-07-23T19:22:49.000Z

Don't know how we can solve it, because we don't know this values on the build step and don't know context, what we know? Only value.

This seems like a bug in icss-utils then, if it doesn't permit inserting a different value depending on the context. I've filed a bug: css-modules/icss-utils#70

Here other problem you can import selector from other file and it is not hashed, using :local(selector) { } provide invalid export. So supporting selector is very limited now, you can use it only in the current file, importing is not working as expected

I'm not sure what you mean with this. Can you give an example?

Answer 20 · 2021-03-18T08:50:28.000Z

Any workaround for this issue?