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

Back slashes added? #11

Open
kevinSuttle opened this issue Aug 23, 2016 · 27 comments
Open

Back slashes added? #11

kevinSuttle opened this issue Aug 23, 2016 · 27 comments

Comments

@kevinSuttle
Copy link

kevinSuttle commented Aug 23, 2016

Any ideas on what could be causing this? I'm not sure it's this plugin, to be honest.

screen shot 2016-08-23 at 12 03 22 pm

 postcss: function () {
      return [
        require('postcss-import')({
          addDependencyTo: webpack,
        }),
        customProperties({
          preserve: true,
        }),
        cssApply,
        autoprefixer,
      ]
    },
@kevinSuttle kevinSuttle changed the title Forward slashes added? Back slashes added? Aug 23, 2016
@kevinSuttle
Copy link
Author

I even removed :root and still the back slashes were added.

screen shot 2016-08-23 at 12 30 51 pm

@kevinSuttle
Copy link
Author

kevinSuttle commented Aug 23, 2016

There seems to be some issue with colons as well. When I don't add them after the "mixins" declared under :root (e.g. --ThemeAnimationStyles), the rules below :root are not eval'd.

@pascalduez
Copy link
Owner

Hi,

are you sure the @apply plugin is executed actually?
Because property sets declarations are supposed to be removed from final output.

Or most probably it's because the slash is added to the declaration before the plugin runs, which breaks its "detection", so nothing happens.

I would advise to always add colons at the end of the declaration.
I will add more tests for this.

Can you set-up a small repo to be able to reproduce/debug?

@kevinSuttle
Copy link
Author

The plugin is running, because the styles are being applied. Let me see if I can set up a small test repo.

@kevinSuttle
Copy link
Author

@pascalduez Here you go: https://cloudup.com/cLsnx9RtW82 See README for repro steps.

@kevinSuttle
Copy link
Author

The plugin is running, because the styles are being applied.

Odd. I noticed that they're being applied in Chrome, but not Safari.
screen shot 2016-08-24 at 2 24 54 pm

@kevinSuttle
Copy link
Author

Yeah, something's up for sure, because changing the buttons.css file doesn't change anything in Chrome, even though Webpack is "rebuilding".

@kevinSuttle
Copy link
Author

kevinSuttle commented Aug 24, 2016

Maybe it's this?

postcss/postcss-import#223

@pascalduez
Copy link
Owner

@kevinSuttle Is this issue still relevant?

@pascalduez
Copy link
Owner

I tried fiddling with the provided repo for an hour or so, I could not get any progress.
Looks like the postcss plugins are not even applied/called.
By default storybook will use its own webpack config, so no surprise the plugins aren't used.
I added a custom webpack config for storybook but same result.

Next step would be to try without storybook, just "plain" webpack and an index.html.

I'm using cssnext on a projet with webpack n' stuff and have no issue BTW.

@kevinSuttle
Copy link
Author

@pascalduez I haven't seen it reproduced. Thanks for checking into it. I appreciate you taking the time.

@kevinSuttle
Copy link
Author

kevinSuttle commented Sep 20, 2016

Seeing this again now.
I'm now using:

Button.css

:root {
  \--ThemeButtonStyles: {
    @apply --ThemeAnimationStyles;
...
}

webpack.config.js

postcss: function() {
      return [
        atImport({
           addDependencyTo: webpack,
           path: './src/Index.css',
           plugins: [
              customProperties({preserve: true}),
              cssApply,
              autoprefixer
          ]
        }),
      ]
    },

@pascalduez
Copy link
Owner

This is concerning enough, even if Webpack 2 is still in beta. Re-opening.

@pascalduez pascalduez reopened this Sep 20, 2016
@kevinSuttle
Copy link
Author

kevinSuttle commented Sep 20, 2016

Let me elaborate on the structure:

./Index.css

@import "./Themes/Vars.css";
@import "./Themes/Colors/Kratos.css";
@import "./Themes/Colors/Prometheus.css";
@import "./Themes/Mixins.css";

Each component has it's own CSS file. e.g. Button.

Buttons/

  • Buttons.js
  • Buttons.css

Buttons.css is the only component that I am writing a mixin for locally, so maybe that's the repro.

:root {
  --ThemeButtonStyles: {
    @apply --ThemeAnimationStyles; /* From "../Themes/Mixins.css" */
    ...
 }

.btn,
.primaryBtn,
.secondaryBtn,
.pageBtn,
.navBtn {
  @apply --ThemeButtonStyles;

  ... 
  }
}
WARNING in ./~/css-loader!./~/postcss-loader!./src/Index.css
postcss-custom-properties: /Users/kevinSuttle/Code/platform-ui-primitives/src/Themes/Mixins.css:15:5: variable '--PrimaryThemeColor' is undefined and used without a fallback

[repeats]

This is inaccurate. --PrimaryThemeColor is defined in the 2 color files listed before Mixins.css.
BUT, something is adding these slashes before they can get there.

@kevinSuttle
Copy link
Author

kevinSuttle commented Sep 20, 2016

Confirmed. Removing that :root declaration in Button/Buttons.css removed that \, but the warnings still appear. Separate issue it seems.

Or maybe not. I've got another :root declaration in Themes/Mixins.css as well, but I also have them in every top-level file that gets imported into ./Index.css.

:root {
  --ThemeAnimationStyles: {
    transition-duration: .2s;
    transition-timing-function: cubic-bezier(.075, .82, .165, 1);
  };

@kevinSuttle
Copy link
Author

kevinSuttle commented Sep 20, 2016

Hm. The top-level styles are getting added last, but the component is still getting the correct style applied, which makes this even more confusing.

screen shot 2016-09-20 at 3 50 52 pm

@kevinSuttle
Copy link
Author

kevinSuttle commented Sep 20, 2016

I added the imports in my top-level npm module export:

require("./Themes/Vars.css");
require("./Themes/Colors/Kratos.css");
require("./Themes/Colors/Prometheus.css");
require("./Themes/Mixins.css");

And got the slashes back on top-level :root declarations.
I removed the smart-import plugin and the slashes went away.

I still get the warnings from postcss-custom-properties, but the styles work.

WARNING in ./~/css-loader!./~/postcss-loader!./src/Toggles/Toggles.css
postcss-custom-properties: /Users/kevinSuttle/Code/platform-ui-primitives/src/Toggles/Toggles.css:37:3: variable '--PrimaryThemeColor' is undefined and used without a fallback

Edit: nope. @MoOx, this appears to be an issue with postcss-custom-properties, as it still appears even when I remove postcss-apply.

Will open there.

@Memoyr
Copy link

Memoyr commented Feb 3, 2017

Hi , I got the same issue with the plugin, it was adding backslashes when I try to import custom property sets from an external CSS file.
I aim to keep all of my sets in separate file as my project is build with multiple CSS files (file per component).
I succeed to import my file and use custom property sets by using postcss-apply in conjonction of postcss-import.
There's 2 downside.

  • I have to import the file into my current file anytime I want to use a custom property set.
  • A waring is triggered saying "No custom properties set declared " , even though the styles are actually being applied.

I there a better way to achieve this, without those two downside ?
Thx

part of my webpack ("webpack": "1.13.3") config :
``
var pcssNesting = require('postcss-nesting')
var pcssNext = require('postcss-cssnext')
var cssApply = require('postcss-apply')
var pcssCustomProps = require('postcss-custom-properties')
var pcssReporter = require('postcss-reporter')
var atImport = require("postcss-import")

postcss: [
pcssCustomProps({preserve: 'computed', strict: false}),
pcssNext,
atImport(),
cssApply,
pcssNesting,
pcssReporter({clearMessages: true})
]`

@pascalduez
Copy link
Owner

@Memoyr Just a quick remark on what you posted above, postcss-custom-properties and postcss-apply are already included within postcss-cssnext. So apparently you're running them twice.
Also the plugins order is important, postcss-import should come first.

@pascalduez
Copy link
Owner

I have to import the file into my current file anytime I want to use a custom property set.

That's how postcss-import and most preprocessors works.
Only alt solution I can see for now is waiting for #15.

@ericsaboia
Copy link

Same is happening here. apply can't do its job because something is adding a \ before the variables. My best guess is that it's css-loader doing it, but I can't understand why.

For now to get unblocked I've created this postcss plugin to clean the extra backslash.

@pascalduez
Copy link
Owner

pascalduez commented Feb 19, 2018

@ericsaboia Basically postcss-loader should comes before the css-loader so the apply plugin should already have done it's job.
Edit: I don't know what I'm talking about :)

I once tried to debug, but the thing is quite dense, there's so many actors involed...
Re-opening anyway.

@pascalduez pascalduez reopened this Feb 19, 2018
@ericsaboia
Copy link

thanks for looking into this, even though it doesn't seem to be an issue caused by postcss-apply, @pascalduez.

I'm executing postcss-apply as a plugin using extract-text-webpack-plugin + optimize-css-assets-webpack-plugin, so it's only called after all the loaders are executed.

When I execute it inside the loaders it works well, meaning the \ isn't there yet. That's why I believe it might be happening inside css-loader.

I can set up a minimal reproducible environment to help debug this.

@yebrahim
Copy link

yebrahim commented Mar 8, 2018

Also hitting this issue, and also because of css-loader. If I switch to the simple string-loader it works (although imports break), so it seems css-loader does some kind of validation while loading the CSS, and automatically comments invalid CSS out. I can't find a way to turn this off though.

@pascalduez
Copy link
Owner

Okay, I spotted the culprit. This happens when used together with CSS modules.
It's css-selector-tokenizer stringify function.

@yebrahim
Copy link

@pascalduez no way around this then?

@pascalduez
Copy link
Owner

pascalduez commented Apr 16, 2018

@yebrahim One solution, PR that lib to prevent the escaping. But since custom property sets are not going anywhere, it might be more difficult to get the change accepted, dunno. I'll try, but feel free to.

Currently custom property sets don't have any browser support and will most likely won't get any.
So it does not make much sense to use the preserve option, so they basically won't get escaped (problem solved).
That might be mitigated by the order the plugin are run though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants