Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicated content when @importing the same dependency from different CSS files #316

Closed
oleq opened this issue Nov 9, 2017 · 11 comments
Closed

Comments

@oleq
Copy link

oleq commented Nov 9, 2017

Setup

JS

js/
├── app.js
├── bar.js
└── foo.js
css/
├── bar.css
├── foo.css
└── shared.css

app.js

import './foo.js'
import './bar.js'

foo.js

import '../css/foo.css';

console.log( 'foo.js' );

bar.js

import '../css/foo.css'; // a duplicate -> see foo.js
import '../css/bar.css';

console.log( 'bar.js' );

CSS

foo.css

@import "./shared.css";

.foo {}

bar.css

@import "./shared.css";

.bar {}

shared.css

.shared {}

PostCSS, Webpack

postcss.config.js

'use strict';

module.exports = {
	plugins: [
		require( 'postcss-import' )()
	]
};

webpack.config.js

'use strict';

const path = require( 'path' );

module.exports = {
	entry: path.resolve( __dirname, 'js', 'app.js' ),

	output: {
		path: path.resolve( __dirname, 'build' ),
		filename: 'app.js',
		libraryTarget: 'umd',
		libraryExport: 'default',
		library: 'app'
	},

	module: {
		rules: [
			{
				test: /\.css$/,
				use: [
					{
						loader: 'style-loader',
						options: {
							singleton: true
						}
					},
					{
						loader: 'postcss-loader'
					},
				]
			}
		],
	}
};

Expected

<style type="text/css">
.shared {}
.foo {}
.bar {} 
</style>

Actual

The problem is that shared.css has been imported twice.

Note that the duplicate (import '../css/foo.css';) in bar.js has been detected and skipped correctly (single .foo{} in the output). It's all about @import "./shared.css";.

<style type="text/css">
.shared {}
.foo {}
.shared {}
.bar {} 
</style>
@RyanZim
Copy link
Collaborator

RyanZim commented Nov 9, 2017

I think this is webpack-related; @michael-ciniawsky would know better.

@michael-ciniawsky
Copy link

Remove postcss-import and let css-loader handle @import there is no way to dedupe it when using postcss-import since a loader's context is scoped to the file (module). It's in general better to avoid @import shared deps and require/import them directly into the entrypoint

|– src
| |– styles ('shared')
| | |– vendor.css
| |– entry.js
| |– a
| | |– styles ('scoped')
| | | |– a-1.css
| | |– a.css // @import './styles/a-1.css';
| | |– a.js
| |– b
| | |– styles ('scoped')
| | | |– b-1.css
| | |– b.css // @import './styles/b-1.css';
| | |– b.js

entry.js

import './styles/vendor.css'

import a from './a/a.js'
import b from './b/b.js'

...

a.js

import './a.css'

...

a.css

@import './styles/a-1.css';

...

Wont't fix for postcss-import, I'm afraid

@RyanZim
Copy link
Collaborator

RyanZim commented Nov 9, 2017

Closing as per @michael-ciniawsky

@RyanZim RyanZim closed this as completed Nov 9, 2017
@oleq
Copy link
Author

oleq commented Nov 10, 2017

Remove postcss-import and let css-loader handle @import

That's quite a problem because without postcss-import there's no way to use variables (postcss-custom-properties) or mixins (postcss-mixins) from @imported CSS files.

That's true, css-loader resolves duplicates but at the same time it makes the mentioned features useless.


Maybe if I explained the case, some things would become clearer:

The goal

CKEditor 5 is an extremely modular text editor. It consists of many packages which (among many other things) provide views (ES6 classes) to create the UI.

In other words: it's a framework. A developer can compile an editor which uses only:

  • ViewFooA from PackageFoo,
  • and ViewBarA, ViewBarB from PackageBar

while the packages (PackageFoo, PackageBar) provide many, many other views which are unnecessary from developer's (integration) point of view.

Each view brings some HTML+JS to the application. E.g. ButtonView allows inserting buttons in the UI and ToolbarView allows inserting toolbars, etc.. It also means that each view needs some CSS to be displayed.

Our goal is to make sure that when a view is imported in the application, it comes with the necessary CSS. And if the view is not used, the styles should not be in the webpack's output. It's all about making the builds of the editor optimal.

The problem

The ButtonView class imports its own styles

import '../../theme/components/button.css';

export default class ButtonView extends View {}

and so does ToolbarView

import '../../theme/components/toolbar.css';

export default class ToolbarView extends View {}

The problem is that both components use some pre–defined colors in variables (and mixins, but let's focus on colors to keep things simple):

colors.css

:root {
    --ck-color-background: white;
}

button.css:

@import "../helpers/colors";

.ck-button {
    background: var(--ck-color-background);
}

toolbar.css:

@import "../helpers/colors";

.ck-toolbar {
    background: var(--ck-color-background);
}

We decided to migrate the project from SASS to PostCSS because it totally failed when it came to handling imports. I.e. the ButtonView class can be imported in many packages, and with SASS the styles of the button got duplicated.

We figured out that postcss-import handles things just great. The variables and mixins from @imported CSS files simply work. And the style-loader+postcss-loader combo resolves possible CSS import duplicates in JS files just as great.

In fact, the problem we face here is the last one we have to finish the migration. What we expect is

:root {
    --ck-color-background: white;
}

.ck-button {
    background: var(--ck-color-background);
}

.ck-toolbar {
    background: var(--ck-color-background);
}

but what we see is

:root {
    --ck-color-background: white;
}

.ck-button {
    background: var(--ck-color-background);
}

:root {
    --ck-color-background: white;
}

.ck-toolbar {
    background: var(--ck-color-background);
}

and it will totally bloat our codebase because the project is huge and the will be tons of variable declarations like this repeated hundreds of times. We're developing a library and the size really matters.

See, using css-loader does resolve the duplicates but for whatever the reason, importing custom properties stop working.

Perhaps we missed something important or obvious here?

Maybe there's some way to aggregate/concatenate (???) styles on the fly with style-loader+css-loader so postcss-import is not required and postcss-custom-properties and postcss-mixins are able to detect variable/mixin definitions? Some sort of intermediate step before PostCSS actually starts processing the files?

It's a stalemate for us and any help would be appreciated.

@michael-ciniawsky
Copy link

That's quite a problem because without postcss-import there's no way to use variables (postcss-custom-properties) or mixins (postcss-mixins) from @imported CSS files.

That's true, css-loader resolves duplicates but at the same time it makes the mentioned features useless.

Via importLoaders the module request created by css-loader should run through postcss-loader

@import './file.css'; => require('!css-loader!postcss-loader!./file.css')

Theming is complicated with webpack in general, the best solution I found so far is enhancing the PostCSS AST with the variables from a JSON file instead (postcss-custom-properties)

Static

|– componentA
| | |– icon.svg
| | |– styles.css
| | |– index.js
| |– componentB
| | |– icon.svg
| | |– styles.css
| | |– index.js
|– variables.json

postcss.config.js

// All variables
// {
//  "--color": "red",
// }
const variables = require('./variables.json')

module.exports = ({ file, options, env }) => ({
   plugins: {
     'postcss-custom-properties': { variables }
   }
})

Dynamically (per file)

|– componentA
| | |– variables.json
| | |– icon.svg
| | |– styles.css
| | |– index.js
| |– componentB
| | |– variables.json
| | |– icon.svg
| | |– styles.css
| | |– index.js

postcss.config.js

module.exports = ({ file, options, env }) => ({
   plugins: {
     'postcss-custom-properties': { variables: require(`${file.dirname}/variables.json`) }
   }
})

I would personally go with the all-in-one file approach, since in the intermediate future (no IE support) you can serve a theme-x.css && theme-y.css file containing the vars only and ignore them during build time.

theme-x.css

:root {
   --bg-color: red;
}

theme-y.css

:root {
   --bg-color: blue;
}

ZauberNerd added a commit to xing/hops that referenced this issue Aug 22, 2018
Because this creates a lot of duplication when importing css variables
files in other css files which are then imported via js.
See:
- postcss/postcss-import#316
- postcss/postcss-import#294
- postcss/postcss-import#224
ZauberNerd added a commit to xing/hops that referenced this issue Aug 22, 2018
Because this creates a lot of duplication when importing css variables
files in other css files which are then imported via js.
See:
- postcss/postcss-import#316
- postcss/postcss-import#294
- postcss/postcss-import#224
@catamphetamine
Copy link

I'm going to try the postcss.config.js variables approach tomorrow, because I noticed today that DevTools had a dramatic drop in performance (the Elements tab is unusable) due to constants.css being overwritten by itself many times.
There're no duplicates in production CSS bundle, but during development there's a lot of duplication.
I'll update my comment with the results.

@trusktr
Copy link

trusktr commented Aug 21, 2020

It's been a while. Is this still an issue?

@TakCastel
Copy link

Yes, unfortunately it is still an issue as I've just encountered this problem today and can't find any solution.

@trusktr
Copy link

trusktr commented Mar 3, 2021

This is also a problem with plain CSS @import, which will have the same issue as the OP.

Given the OP's example,

CSS

foo.css

@import "./shared.css";

.foo {}

bar.css

@import "./shared.css";

.bar {}

shared.css

.shared {}

native CSS will behave exactly as

.shared {}
.foo {}
.shared {}
.bar {} 

This is not only bad because of duplication, but the second occurrence of .shared {} may override the attempt of .foo {} to override the first .shared {} styles, thus making the style output of foo.css be not as expected and appear to be incorrect.

For reference, this WICG thread describes the native CSS issue.

This prevents any intuitive sense of code re-use in CSS.

trusktr added a commit to lume/basicss that referenced this issue Mar 3, 2021
@1ewiswarren
Copy link

is there any solution to this? im close to giving up entirely with postcss

@RyanZim
Copy link
Collaborator

RyanZim commented Aug 18, 2022

This issue is a long thread of potentially unrelated problems; I'm locking this to prevent further noise. postcss-import will always follow native CSS behavior; if there's a problem otherwise, please open a new issue.

@postcss postcss locked as resolved and limited conversation to collaborators Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants