postcss / postcss-custom-properties

Use Custom Properties in CSS
https://postcss.github.io/postcss-custom-properties
MIT License
596 stars 77 forks source link

Duplication declaration with `preserve: true` and `strict: false` #109

Closed le0pard closed 5 years ago

le0pard commented 6 years ago

Hello. Plugin add always duplicate declarations with options preserve: true and strict: false. Example:

         require('postcss-cssnext')({
            browsers: [
              'last 2 version'
            ],
            features: {
              rem: false,
              customProperties: {
                strict: false,
                warnings: false,
                preserve: true
              }
            }
          }),

Input:

.form-field__input
  width: 99%
  font-size: 1.8rem
  color: var(--emphColor)
  border-radius: 6px
  border: 1px solid var(--commentColor)
  background-color: color(var(--headsColor) tint(4%))

Output:

.form-field__input {
  width: 99%;
  font-size: 1.8rem;
  color: var(--emphColor);
  color: var(--emphColor);
  border-radius: 6px;
  border: 1px solid var(--commentColor);
  border: 1px solid var(--commentColor);
  background-color: color(var(--headsColor) tint(4%));
  background-color: color(var(--headsColor) tint(4%)); }

123

Looks like it still trying to add fallback. Is it possible to disable such behaviour of plugin?

postcss-custom-properties version 6.2.0

oleersoy commented 6 years ago

I'm seeing something similar to this. For example with this as the input:

.u-font-family-ampersand {
    font-family: var(--font-family-ampersand) !important;

This is the output:

.u-font-family-ampersand {
    font-family: Ampersand,ff-more-web-pro,"More Pro",Georgia,Times,serif !important;
    font-family: var(--font-family-ampersand) !important;

I'm expecting only:

.u-font-family-ampersand {
    font-family: Ampersand,ff-more-web-pro,"More Pro",Georgia,Times,serif !important;

In my case I'm not specifying any configuration properties. Just using the defaults.

cfrank-rv commented 6 years ago

I am seeing this as well on 6.3.1. With almost the same settings as OP just without strict. This is definitely a bug since it's not a fallback when both rules are just duplicates of each other.

font-weight: var(--font-weight-light);

becomes

font-weight: var(--font-weight-light);
font-weight: var(--font-weight-light);
oleersoy commented 6 years ago

I don't have see any issue with version 6.3.1, the version prior to the 7.0.0 update.

jonathantneal commented 6 years ago

I’ll look into creating a test for this. Once verified, that will help me with a resolution. You can see the different tests I have made here, if anyone feels up to contributing one today:

https://github.com/jonathantneal/postcss-tests

cfrank-rv commented 6 years ago

Hey @jonathantneal I'll create a test for this. Should be ready in an hour or so

cfrank-rv commented 6 years ago

This issue is probably caused by something else since the test I ran show postcss-custom-properties returning the correct results using the settings in the OP.

https://github.com/cfrank-rv/postcss-tests/tree/master/postcss-custom-properties-test-01

I ran the test on 6.3.1 and postcss v6.0.21 which is where I was seeing the bug.

Here is my config, where I am seeing the bug. If this helps at all.

module.exports = {
  plugins: {
    'postcss-import': {},
    'postcss-for': {},
    'postcss-nested': {},
    'postcss-cssnext': {
      browsers: [
        'last 2 versions',
        '> 2%',
      ],
      features: {
        customProperties: {
          preserve: true,
          warnings: false,
        },
      },
    },
  },
};

I might create another test using these settings to just confirm it's caused by that. But that will have to happen a little later today.

jonathantneal commented 6 years ago

Okay, let me know if you discover an issue with this plugin, as that would help me write a fix. If your test shows the plugin operating correctly and we haven’t had anything further to debug, next weekend I will close this issue.

pldg commented 6 years ago

I had the same problem with v7.0.0. I'm using Webpack 4 with postcss-loader.

Adding postcss-import as first plugin in the list solve the problem.

le0pard commented 6 years ago

@pldg I have exactly this issue, where postcss-import is first and postcss-custom-properties@7.0.0, webpack@4.6.0:

{
    loader: 'postcss-loader',
    options: {
      sourceMap: true,
      plugins: function() {
        return [
          require('postcss-import')({
            addDependencyTo: webpack
          }),
          require('postcss-url')(),
          require('lost')({
            flexbox: 'flex'
          }),
          require('rucksack-css')(),
          require('postcss-cssnext')({
            browsers: [
              'last 2 version'
            ],
            features: {
              rem: false,
              customProperties: {
                strict: false,
                warnings: false,
                preserve: true
              }
            }
          }),
          require('postcss-browser-reporter')(),
          require('postcss-reporter')()
        ];
      }
    }
  }
pldg commented 6 years ago

@le0pard

addDependencyTo, what's this option? It's not even in postcss-import options list.

Maybe the problem is postcss-cssnext?

Try postcss-custom-properties stand alone version.

My config:

{
  loader: 'style-loader',
  options: {
    sourceMap: true
  }
},
{
  loader: 'css-loader',
  options: {
    sourceMap: true
  }
},
{
  loader: 'postcss-loader',
  options: {
    sourceMap: true,
    plugins: () => ([
      require('postcss-import'),
      require('postcss-custom-properties')({
        preserve: false
      }),
      require('autoprefixer')
    ])
  }
}

Also with preserve: true works fine.

le0pard commented 6 years ago

@pldg addDependencyTo do nothing right now - no influence. cssnext just number of postcss packages - https://github.com/MoOx/postcss-cssnext/blob/master/package.json (including this one).

itaditya commented 6 years ago

I am having the same issue

:root {
  --primary-color: orange;
}

.Button {
  background: var(--primary-color);
}

results in

:root {
  --primary-color: orange;
}

.Button {
  background: orange;
  background: orange;
  background: var(--primary-color);
}

the postcss.config.js file ->

module.exports = {
  plugins: {
    'autoprefixer': {
      'browsers': ['> 1%', 'last 2 versions']
    },
    'postcss-nested': {},
    'postcss-custom-properties': {}
  }
}
pldg commented 6 years ago

As I said in the comment above, adding postcss-import as first plugin solve the problem. The problem seems related to how postcss-custom-properties handle @import statements. I've made a test repository test-postcss-custom-properties.

le0pard commented 6 years ago

@pldg project with postcss-import and this plugin - issue still present: https://github.com/le0pard/mpw.js

itaditya commented 6 years ago

@pldg really sorry for not thoroughly checking the above comments before commenting. Thanks for responding quickly.

I tried your solution, but the duplication is still there. One important thing I'm using rollup

PFight commented 4 years ago

Still getting dublication on 9.1.1 using rollup:

background: var(--checked-pin-color, #fff);

=>

background: #fff;
background: #fff;
background: var(--checked-pin-color, #fff);