css-loader icon indicating copy to clipboard operation
css-loader copied to clipboard

ICSS: Importing a composed class name produces wrong output

Open joepie91 opened this issue 5 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;
}

Selection_407

Actual Behavior

css-loader produces incorrect output:

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

Selection_406

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.

joepie91 avatar Nov 23 '19 23:11 joepie91

Thanks for issue i will look on this in near future

alexander-akait avatar Nov 25 '19 10:11 alexander-akait

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.

yakato avatar Dec 03 '19 15:12 yakato

@yakato please create reproducible test repo

alexander-akait avatar Dec 09 '19 16:12 alexander-akait

Might be related to https://github.com/webpack-contrib/css-loader/issues/561, which has a minimal reproduction.

apexskier avatar Feb 17 '20 10:02 apexskier

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

joepie91 avatar Feb 17 '20 18:02 joepie91

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.

OHUSAR avatar Apr 06 '20 09:04 OHUSAR

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

alexander-akait avatar Apr 06 '20 09:04 alexander-akait

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

OHUSAR avatar Apr 06 '20 09:04 OHUSAR

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

alexander-akait avatar Apr 06 '20 09:04 alexander-akait

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

Screenshot from 2020-04-06 11 22 27

Actual Behavior

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

Screenshot from 2020-04-06 11 22 52

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

OHUSAR avatar Apr 06 '20 09:04 OHUSAR

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

alexander-akait avatar Apr 06 '20 09:04 alexander-akait

@evilebottnawi any updates on this please? thank you

OHUSAR avatar May 07 '20 09:05 OHUSAR

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

alexander-akait avatar May 07 '20 11:05 alexander-akait

@OHUSAR Problem on mini-css-extract-plugin side, https://github.com/webpack-contrib/mini-css-extract-plugin/issues/555

alexander-akait avatar Jul 23 '20 12:07 alexander-akait

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

alexander-akait avatar Jul 23 '20 13:07 alexander-akait

Here big problem :disappointed: @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.

alexander-akait avatar Jul 23 '20 13:07 alexander-akait

I postpone it for v5, now it is the limitation

alexander-akait avatar Jul 23 '20 13:07 alexander-akait

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

alexander-akait avatar Jul 23 '20 14:07 alexander-akait

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?

joepie91 avatar Jul 23 '20 19:07 joepie91

Any workaround for this issue?

abhijithqb avatar Mar 18 '21 08:03 abhijithqb