postcss / postcss-custom-properties

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

Duplicate Variables using `postcss-custom-properties` #114

Closed josephschmitt closed 6 years ago

josephschmitt commented 6 years ago

When mixing and matching variables you declare in your source files along with appendVariables and preserve, those vars get output twice: once from your source, and once appended to the bottom. Shouldn't appendVariables only be appending those that were passed in via the plugin's variables option? I don't understand the point of appending it again at the bottom. Am I missing something?

Test case showing the issue here: https://github.com/josephschmitt/custom-properties-append-vars-duplicates

Source setup:

:root {
  --myColor: red;
}

.my-component {
  color: var(--myColor);
  background: var(--myBackgroundColor);
}

postcss.config.js:

module.exports = {
  plugins: [
    require('postcss-custom-properties')({
      preserve: true,
      appendVariables: true,
      variables: {
        myBackgroundColor: 'lightgray'
      }
    })
  ]
};

Expected Result

Running through the postcss-custom-properties plugin with preserve and appendVariables produces an output file with the locally declared variables preserved, and the variables passed in via the plugin appended at the end of the file:

:root {
  --myColor: red;
}

.my-component {
  color: red;
  color: var(--myColor);
  background: lightgray;
  background: var(--myBackgroundColor);
}

:root {
  --myBackgroundColor: lightgray;
}

Actual Result

The output file correctly preserves the local variable and it appends the variable passed in via the config, but then it also outputs the local variable again appended to the bottom of the file. This is wasteful, and gets especially bad in large projects with lots of declared variables:

:root {
  --myColor: red;
}

.my-component {
  color: red;
  color: var(--myColor);
  background: lightgray;
  background: var(--myBackgroundColor);
}

:root {
  --myColor: red;
  --myBackgroundColor: lightgray;
}
jonathantneal commented 6 years ago

I agree with your concern, and I think the plugin should work as you expect.

While I didn’t write this functionality originally, I do think we should refresh this part of the plugin to not output CSS for things that are already in CSS. 😄

Due to some other commitments, there’s a high risk I couldn’t get to this before I forgot about it. I recommend pinging me in 2 weeks if you haven’t heard a response, which is likely.

And of course, as always, PRs are welcome. Thanks again for reporting this.

josephschmitt commented 6 years ago

@jonathantneal thanks for the quick reply as always. Now that I know it's not intentional, I might try and whip up a fix and send it your way. I'll post it here if I end up being able to. Thanks for your time!

josephschmitt commented 6 years ago

@jonathantneal took a crack at a PR, let me know what you think https://github.com/postcss/postcss-custom-properties/pull/116

josephschmitt commented 6 years ago

@jonathantneal any plans on publishing a new version soon?