MadLittleMods / postcss-css-variables

PostCSS plugin to transform CSS Custom Properties(CSS variables) syntax into a static representation
https://madlittlemods.github.io/postcss-css-variables/playground/
Other
536 stars 62 forks source link

Displaying readable errors #22

Closed vincentorback closed 7 years ago

vincentorback commented 8 years ago

For now it looks like this great plugin doesn’t log any errors at all. If I for example have a setup like this:

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

.btn {
  color: var(--other-color);
}

All I get is a postcss error that looks something like this: TypeError: Cannot read property 'indexOf' of undefined]

Would be nice if this plugin could show what the error is, which file and which linenumber. And maybe using postcss-reporter.

MadLittleMods commented 8 years ago

Hey @vincentorback,

I have also run into this before but I am having trouble reproducing it now even with your example. We also have a tests for this kind of situation and they are passing.

It should be adding to the result warnings but it seems to get caught up in some code and error out before we can gracefully pass on the result.

vincentorback commented 8 years ago

Okey... I tried to have a look but was hard to track down the issue. How is it set up now? It just ignores errors and displays undefined, or should the plugin crash, or something else?

MadLittleMods commented 8 years ago

It should set undefined as the value; that will invalidate the declaration in the browser to potentially fallback to other values. It will also emit a warning(about the variable being undefined) on the result object which postcss-reporter inspects and displays.

Just to be sure, what version of postcss-css-variables are you using?

Any luck on a test/reproduction case?

vincentorback commented 8 years ago

I’m using the 0.5.1 version. And also in running PostCSS with npm script.

After lots of testing with and without other plugins and moving around files it boiled down it not working with autoprefixer:

postcss -u postcss-css-variables -u autoprefixer -u postcss-reporter ./style.css

which is really weird but perhaps it picks up error differently and does something new? I’m going to keep looking for what’s wrong but this is just my last finding.

vincentorback commented 8 years ago

An idea which I'm not sure is true, but could it be so that postcss-css-variables sets the property to undefined and not "undefined", a string, so that when autoprefixer picks up the trail it fails?

tbremer commented 8 years ago

I am having the same issue here, it's not a big deal to move autoprefixer to a second build, however I am not seeing any warnings or reports generated when an incorrect variable declaration is being passed in.

Thoughts?

versions:

"postcss": "^5.0.14"
"postcss-css-variables": "^0.5.1"
MadLittleMods commented 8 years ago

@tbremer Create a separate issue as this seems unrelated.

tbremer commented 8 years ago

@MadLittleMods, sounds good. I will try to get a testable situation written up as well! https://github.com/MadLittleMods/postcss-css-variables/issues/31

vincentorback commented 8 years ago

Still not sure if this is a postcss-css-variables-issue but I did some more tests:

.foo {
  color: var(--bar);
}

-u postcss-css-variables style.css Works fine, rendering .foo {color:undefined}.

-u postcss-css-variables -u autoprefixer style.css Breaks showing error.

-u postcss-css-variables -u cssnano style.css Breaks showing error.

So the issue is when other plugins reads the result of a missing variables which is undefined. But if you just write:

.blue {
  color: undefined
}

It works fine... Not trying to rush a fix, I don’t think it’s a huge problem, but just to let you know.

MadLittleMods commented 8 years ago

No worries @vincentorback, I really appreciate the barebones test samples and results!

If the undefined value is messing things up, we can probably just check and convert it to a string either here or here. Feel free to PR with tests so it doesn't regress.

MadLittleMods commented 7 years ago

Released in v0.6.0 :tada:. Thanks to @vincentorback for the contribution ❤️